From 86081156a1148da78d741cc5c150e7294a66113e Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Tue, 31 Jul 2018 13:24:17 -0500 Subject: [PATCH] tq: display a warning message upon HTTP 422 --- t/t-content-type.sh | 21 +++++++++++++++++++++ tq/basic_upload.go | 11 +++++++++++ tq/transfer_queue.go | 32 ++++++++++++++++++++++++++++++-- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/t/t-content-type.sh b/t/t-content-type.sh index 6eab4020a0..0100d0b97d 100755 --- a/t/t-content-type.sh +++ b/t/t-content-type.sh @@ -44,3 +44,24 @@ begin_test "content-type: is disabled by configuration" [ 1 -eq "$(grep -c "Content-Type: application/zip" push.log)" ] ) end_test + +begin_test "content-type: warning message" +( + set -e + + reponame="content-type-warning-message" + setup_remote_repo "$reponame" + clone_repo "$reponame" "$reponame" + + git lfs track "*.txt" + printf "status-storage-422" > a.txt + + git add .gitattributes a.txt + git commit -m "initial commit" + git push origin master 2>&1 | tee push.log + + grep "info: Uploading failed due to unsupported Content-Type header(s)." push.log + grep "info: Consider disabling Content-Type detection with:" push.log + grep "info: $ git config lfs.contenttype false" push.log +) +end_test diff --git a/tq/basic_upload.go b/tq/basic_upload.go index e5de3fd237..85575654c5 100644 --- a/tq/basic_upload.go +++ b/tq/basic_upload.go @@ -102,6 +102,17 @@ func (a *basicUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Progres req = a.apiClient.LogRequest(req, "lfs.data.upload") res, err := a.doHTTP(t, req) if err != nil { + if errors.IsUnprocessableEntityError(err) { + // If we got an HTTP 422, we do _not_ want to retry the + // request later below, because it is likely that the + // implementing server does not support non-standard + // Content-Type headers. + // + // Instead, return immediately and wait for the + // *tq.TransferQueue to report an error message. + return err + } + // We're about to return a retriable error, meaning that this // transfer will either be retried, or it will fail. // diff --git a/tq/transfer_queue.go b/tq/transfer_queue.go index 81367ceb77..eba9f67707 100644 --- a/tq/transfer_queue.go +++ b/tq/transfer_queue.go @@ -1,6 +1,7 @@ package tq import ( + "fmt" "os" "sort" "sync" @@ -127,6 +128,11 @@ type TransferQueue struct { wait sync.WaitGroup manifest *Manifest rc *retryCounter + + // unsupportedContentType indicates whether the transfer queue ever saw + // an HTTP 422 response indicating that their upload destination does + // not support Content-Type detection. + unsupportedContentType bool } // objects holds a set of objects. @@ -651,8 +657,13 @@ func (q *TransferQueue) handleTransferResult( // If the error wasn't retriable, OR the object has // exceeded its retry budget, it will be NOT be sent to // the retry channel, and the error will be reported - // immediately. - q.errorc <- res.Error + // immediately (unless the error is in response to a + // HTTP 422). + if errors.IsUnprocessableEntityError(res.Error) { + q.unsupportedContentType = true + } else { + q.errorc <- res.Error + } q.wait.Done() } } else { @@ -763,6 +774,17 @@ func (q *TransferQueue) toAdapterCfg(e lfsapi.Endpoint) AdapterConfig { } } +var ( + // contentTypeWarning is the message printed when a server returns an + // HTTP 422 at the end of a push. + contentTypeWarning = []string{ + "Uploading failed due to unsupported Content-Type header(s).", + "Consider disabling Content-Type detection with:", + "", + " $ git config lfs.contenttype false", + } +) + // Wait waits for the queue to finish processing all transfers. Once Wait is // called, Add will no longer add transfers to the queue. Any failed // transfers will be automatically retried once. @@ -781,6 +803,12 @@ func (q *TransferQueue) Wait() { q.meter.Flush() q.errorwait.Wait() + + if q.unsupportedContentType { + for _, line := range contentTypeWarning { + fmt.Fprintf(os.Stderr, "info: %s\n", line) + } + } } // Watch returns a channel where the queue will write the value of each transfer