Improve errors

- Add request metadata (method, path)
- Sentry improvement: custom types for common errors

internal/api/api.go

@@ -11,6 +11,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/badgateway"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 
 	"github.com/dgrijalva/jwt-go"
 )
@@ -147,13 +148,13 @@ func (api *API) PreAuthorizeHandler(h HandleFunc, suffix string) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		authReq, err := api.newRequest(r, nil, suffix)
 		if err != nil {
-			helper.Fail500(w, fmt.Errorf("preAuthorizeHandler: newUpstreamRequest: %v", err))
+			helper.Fail500(w, requesterror.New("preAuthorizeHandler", r, "newUpstreamRequest: %v", err))
 			return
 		}
 
 		authResponse, err := api.Client.Do(authReq)
 		if err != nil {
-			helper.Fail500(w, fmt.Errorf("preAuthorizeHandler: do %v: %v", authReq.URL.Path, err))
+			helper.Fail500(w, requesterror.New("preAuthorizeHandler", r, "do request: %v", err))
 			return
 		}
 		defer authResponse.Body.Close()
@@ -173,7 +174,7 @@ func (api *API) PreAuthorizeHandler(h HandleFunc, suffix string) http.Handler {
 		}
 
 		if contentType := authResponse.Header.Get("Content-Type"); contentType != ResponseContentType {
-			helper.Fail500(w, fmt.Errorf("preAuthorizeHandler: API responded with wrong content type: %v", contentType))
+			helper.Fail500(w, requesterror.New("preAuthorizeHandler", r, "API responded with wrong content type: %v", contentType))
 			return
 		}
 
@@ -182,7 +183,7 @@ func (api *API) PreAuthorizeHandler(h HandleFunc, suffix string) http.Handler {
 		// request metadata. We must extract this information from the auth
 		// response body.
 		if err := json.NewDecoder(authResponse.Body).Decode(a); err != nil {
-			helper.Fail500(w, fmt.Errorf("preAuthorizeHandler: decode authorization response: %v", err))
+			helper.Fail500(w, requesterror.New("preAuthorizeHandler", r, "decode authorization response: %v", err))
 			return
 		}
 		// Don't hog a TCP connection in CLOSE_WAIT, we can already close it now

internal/api/block.go

 package api
 
 import (
-	"fmt"
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 )
 
 // Prevent internal API responses intended for gitlab-workhorse from
 // leaking to the end user
 func Block(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		rw := &blocker{rw: w}
+		rw := &blocker{rw: w, r: r}
 		defer rw.Flush()
 		h.ServeHTTP(rw, r)
 	})
@@ -19,6 +19,7 @@ func Block(h http.Handler) http.Handler {
 
 type blocker struct {
 	rw       http.ResponseWriter
+	r        *http.Request
 	hijacked bool
 	status   int
 }
@@ -47,7 +48,7 @@ func (b *blocker) WriteHeader(status int) {
 		b.status = 500
 		b.Header().Del("Content-Length")
 		b.hijacked = true
-		helper.Fail500(b.rw, fmt.Errorf("api.blocker: forbidden content-type: %q", ResponseContentType))
+		helper.Fail500(b.rw, requesterror.New("api.blocker", b.r, "forbidden content-type: %q", ResponseContentType))
 		return
 	}
 

internal/artifacts/artifacts_upload.go

 package artifacts
 
 import (
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"mime/multipart"
@@ -12,6 +11,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/upload"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/zipartifacts"
 )
@@ -75,7 +75,7 @@ func (a *artifactsUploadProcessor) Cleanup() {
 func UploadArtifacts(myAPI *api.API, h http.Handler) http.Handler {
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		if a.TempPath == "" {
-			helper.Fail500(w, errors.New("UploadArtifacts: TempPath is empty"))
+			helper.Fail500(w, requesterror.New("UploadArtifacts", r, "TempPath is empty"))
 			return
 		}
 

internal/artifacts/entry.go

@@ -2,7 +2,6 @@ package artifacts
 
 import (
 	"bufio"
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -15,6 +14,7 @@ import (
 	"syscall"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/zipartifacts"
 )
@@ -28,14 +28,14 @@ var SendEntry = &entry{"artifacts-entry:"}
 func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	var params entryParams
 	if err := e.Unpack(&params, sendData); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendEntry: unpack sendData: %v", err))
+		helper.Fail500(w, requesterror.New("SendEntry", r, "unpack sendData: %v", err))
 		return
 	}
 
 	log.Printf("SendEntry: sending %q from %q for %q", params.Entry, params.Archive, r.URL.Path)
 
 	if params.Archive == "" || params.Entry == "" {
-		helper.Fail500(w, errors.New("SendEntry: Archive or Entry is empty"))
+		helper.Fail500(w, requesterror.New("SendEntry", r, "Archive or Entry is empty"))
 		return
 	}
 
@@ -44,7 +44,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 	if os.IsNotExist(err) {
 		http.NotFound(w, r)
 	} else if err != nil {
-		helper.Fail500(w, fmt.Errorf("SendEntry: %v", err))
+		helper.Fail500(w, requesterror.New("SendEntry", r, "%v", err))
 	}
 }
 

internal/badgateway/roundtripper.go

@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 )
 
 // Values from http.DefaultTransport
@@ -25,7 +26,7 @@ var DefaultTransport = &http.Transport{
 }
 
 // Custom error for pretty Sentry 'issues'
-type Error502 error
+type Error struct{ error }
 
 type RoundTripper struct {
 	Transport *http.Transport
@@ -81,9 +82,9 @@ func (t *RoundTripper) RoundTrip(r *http.Request) (res *http.Response, err error
 	// instead of 500s we catch the RoundTrip error here and inject a
 	// 502 response.
 	if err != nil {
-		helper.LogError(Error502(fmt.Errorf("badgateway: %s %q failed after %.3fs: %v",
-			r.Method, r.RequestURI, time.Since(start).Seconds(), err,
-		)))
+		helper.LogError(&Error{
+			requesterror.New("badgateway", r, "failed after %.3fs: %v", time.Since(start).Seconds(), err),
+		})
 
 		res = &http.Response{
 			StatusCode: http.StatusBadGateway,

internal/git/archive.go

@@ -18,6 +18,7 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 )
 
@@ -34,7 +35,7 @@ var SendArchive = &archive{"git-archive:"}
 func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	var params archiveParams
 	if err := a.Unpack(&params, sendData); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendArchive: unpack sendData: %v", err))
+		helper.Fail500(w, requesterror.New("SendArchive", r, "unpack sendData: %v", err))
 		return
 	}
 
@@ -42,7 +43,7 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string
 	urlPath := r.URL.Path
 	format, ok := parseBasename(filepath.Base(urlPath))
 	if !ok {
-		helper.Fail500(w, fmt.Errorf("handleGetArchive: invalid format: %s", urlPath))
+		helper.Fail500(w, requesterror.New("SendArchive", r, "invalid format: %s", urlPath))
 		return
 	}
 
@@ -65,7 +66,7 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string
 	// to finalize the cached archive.
 	tempFile, err := prepareArchiveTempfile(path.Dir(params.ArchivePath), archiveFilename)
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("handleGetArchive: create tempfile: %v", err))
+		helper.Fail500(w, requesterror.New("SendArchive", r, "create tempfile: %v", err))
 		return
 	}
 	defer tempFile.Close()
@@ -76,12 +77,12 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string
 	archiveCmd := gitCommand("", "git", "--git-dir="+params.RepoPath, "archive", "--format="+archiveFormat, "--prefix="+params.ArchivePrefix+"/", params.CommitId)
 	archiveStdout, err := archiveCmd.StdoutPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("handleGetArchive: archive stdout: %v", err))
+		helper.Fail500(w, requesterror.New("SendArchive", r, "archive stdout: %v", err))
 		return
 	}
 	defer archiveStdout.Close()
 	if err := archiveCmd.Start(); err != nil {
-		helper.Fail500(w, fmt.Errorf("handleGetArchive: start %v: %v", archiveCmd.Args, err))
+		helper.Fail500(w, requesterror.New("SendArchive", r, "start %v: %v", archiveCmd.Args, err))
 		return
 	}
 	defer helper.CleanUpProcessGroup(archiveCmd) // Ensure brute force subprocess clean-up
@@ -95,13 +96,13 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string
 
 		stdout, err = compressCmd.StdoutPipe()
 		if err != nil {
-			helper.Fail500(w, fmt.Errorf("handleGetArchive: compress stdout: %v", err))
+			helper.Fail500(w, requesterror.New("SendArchive", r, "compress stdout: %v", err))
 			return
 		}
 		defer stdout.Close()
 
 		if err := compressCmd.Start(); err != nil {
-			helper.Fail500(w, fmt.Errorf("handleGetArchive: start %v: %v", compressCmd.Args, err))
+			helper.Fail500(w, requesterror.New("SendArchive", r, "start %v: %v", compressCmd.Args, err))
 			return
 		}
 		defer helper.CleanUpProcessGroup(compressCmd)
@@ -116,22 +117,24 @@ func (a *archive) Inject(w http.ResponseWriter, r *http.Request, sendData string
 	setArchiveHeaders(w, format, archiveFilename)
 	w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just return
 	if _, err := io.Copy(w, archiveReader); err != nil {
-		helper.LogError(fmt.Errorf("handleGetArchive: copy 'git archive' output: %v", err))
+		helper.LogError(&copyError{
+			requesterror.New("SendArchive", r, "copy 'git archive' output: %v", err),
+		})
 		return
 	}
 	if err := archiveCmd.Wait(); err != nil {
-		helper.LogError(fmt.Errorf("handleGetArchive: archiveCmd: %v", err))
+		helper.LogError(requesterror.New("SendArchive", r, "archiveCmd: %v", err))
 		return
 	}
 	if compressCmd != nil {
 		if err := compressCmd.Wait(); err != nil {
-			helper.LogError(fmt.Errorf("handleGetArchive: compressCmd: %v", err))
+			helper.LogError(requesterror.New("SendArchive", r, "compressCmd: %v", err))
 			return
 		}
 	}
 
 	if err := finalizeCachedArchive(tempFile, params.ArchivePath); err != nil {
-		helper.LogError(fmt.Errorf("handleGetArchive: finalize cached archive: %v", err))
+		helper.LogError(requesterror.New("SendArchive", r, "finalize cached archive: %v", err))
 		return
 	}
 }

internal/git/blob.go

 package git
 
 import (
-	"fmt"
 	"io"
 	"log"
 	"net/http"
 	"strings"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 )
 
@@ -19,7 +19,7 @@ var SendBlob = &blob{"git-blob:"}
 func (b *blob) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	var params blobParams
 	if err := b.Unpack(&params, sendData); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendBlob: unpack sendData: %v", err))
+		helper.Fail500(w, requesterror.New("SendBlob", r, "unpack sendData: %v", err))
 		return
 	}
 
@@ -27,29 +27,29 @@ func (b *blob) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 
 	sizeOutput, err := gitCommand("", "git", "--git-dir="+params.RepoPath, "cat-file", "-s", params.BlobId).Output()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("SendBlob: get blob size: %v", err))
+		helper.Fail500(w, requesterror.New("SendBlob", r, "get blob size: %v", err))
 		return
 	}
 
 	gitShowCmd := gitCommand("", "git", "--git-dir="+params.RepoPath, "cat-file", "blob", params.BlobId)
 	stdout, err := gitShowCmd.StdoutPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("SendBlob: git cat-file stdout: %v", err))
+		helper.Fail500(w, requesterror.New("SendBlob", r, "git cat-file stdout: %v", err))
 		return
 	}
 	if err := gitShowCmd.Start(); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendBlob: start %v: %v", gitShowCmd, err))
+		helper.Fail500(w, requesterror.New("SendBlob", r, "start %v: %v", gitShowCmd, err))
 		return
 	}
 	defer helper.CleanUpProcessGroup(gitShowCmd)
 
 	w.Header().Set("Content-Length", strings.TrimSpace(string(sizeOutput)))
 	if _, err := io.Copy(w, stdout); err != nil {
-		helper.LogError(fmt.Errorf("SendBlob: copy git cat-file stdout: %v", err))
+		helper.LogError(&copyError{requesterror.New("SendBlob", r, "copy git cat-file stdout: %v", err)})
 		return
 	}
 	if err := gitShowCmd.Wait(); err != nil {
-		helper.LogError(fmt.Errorf("SendBlob: wait for git cat-file: %v", err))
+		helper.LogError(requesterror.New("SendBlob", r, "wait for git cat-file: %v", err))
 		return
 	}
 }

internal/git/diff.go

 package git
 
 import (
-	"fmt"
 	"io"
 	"log"
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 )
 
@@ -22,7 +22,7 @@ var SendDiff = &diff{"git-diff:"}
 func (d *diff) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	var params diffParams
 	if err := d.Unpack(&params, sendData); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendDiff: unpack sendData: %v", err))
+		helper.Fail500(w, requesterror.New("SendDiff", r, "unpack sendData: %v", err))
 		return
 	}
 
@@ -31,23 +31,25 @@ func (d *diff) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	gitDiffCmd := gitCommand("", "git", "--git-dir="+params.RepoPath, "diff", params.ShaFrom, params.ShaTo)
 	stdout, err := gitDiffCmd.StdoutPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("SendDiff: create stdout pipe: %v", err))
+		helper.Fail500(w, requesterror.New("SendDiff", r, "create stdout pipe: %v", err))
 		return
 	}
 
 	if err := gitDiffCmd.Start(); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendDiff: start %v: %v", gitDiffCmd, err))
+		helper.Fail500(w, requesterror.New("SendDiff", r, "start %v: %v", gitDiffCmd.Args, err))
 		return
 	}
 	defer helper.CleanUpProcessGroup(gitDiffCmd)
 
 	w.Header().Del("Content-Length")
 	if _, err := io.Copy(w, stdout); err != nil {
-		helper.LogError(fmt.Errorf("SendDiff: copy %v stdout: %v", gitDiffCmd, err))
+		helper.LogError(&copyError{
+			requesterror.New("SendDiff", r, "copy %v stdout: %v", gitDiffCmd.Args, err),
+		})
 		return
 	}
 	if err := gitDiffCmd.Wait(); err != nil {
-		helper.LogError(fmt.Errorf("SendDiff: wait for %v: %v", gitDiffCmd, err))
+		helper.LogError(requesterror.New("SendDiff", r, "wait for %v: %v", gitDiffCmd.Args, err))
 		return
 	}
 }

internal/git/error.go

+package git
+
+// For cosmetic purposes in Sentry
+type copyError struct{ error }

internal/git/format-patch.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/senddata"
 )
 
@@ -22,7 +23,7 @@ var SendPatch = &patch{"git-format-patch:"}
 func (p *patch) Inject(w http.ResponseWriter, r *http.Request, sendData string) {
 	var params patchParams
 	if err := p.Unpack(&params, sendData); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendPatch: unpack sendData: %v", err))
+		helper.Fail500(w, requesterror.New("SendPatch", r, "unpack sendData: %v", err))
 		return
 	}
 
@@ -33,23 +34,23 @@ func (p *patch) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 
 	stdout, err := gitPatchCmd.StdoutPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("SendPatch: create stdout pipe: %v", err))
+		helper.Fail500(w, requesterror.New("SendPatch", r, "create stdout pipe: %v", err))
 		return
 	}
 
 	if err := gitPatchCmd.Start(); err != nil {
-		helper.Fail500(w, fmt.Errorf("SendPatch: start %v: %v", gitPatchCmd, err))
+		helper.Fail500(w, requesterror.New("SendPatch", r, "start %v: %v", gitPatchCmd.Args, err))
 		return
 	}
 	defer helper.CleanUpProcessGroup(gitPatchCmd)
 
 	w.Header().Del("Content-Length")
 	if _, err := io.Copy(w, stdout); err != nil {
-		helper.LogError(fmt.Errorf("SendPatch: copy %v stdout: %v", gitPatchCmd, err))
+		helper.LogError(&copyError{requesterror.New("SendPatch", r, "copy %v stdout: %v", gitPatchCmd.Args, err)})
 		return
 	}
 	if err := gitPatchCmd.Wait(); err != nil {
-		helper.LogError(fmt.Errorf("SendPatch: wait for %v: %v", gitPatchCmd, err))
+		helper.LogError(requesterror.New("SendPatch", r, "wait for %v: %v", gitPatchCmd.Args, err))
 		return
 	}
 }

internal/git/git-http.go

@@ -5,7 +5,6 @@ In this file we handle the Git 'smart HTTP' protocol
 package git
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -17,6 +16,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 )
 
 func GetInfoRefs(a *api.API) http.Handler {
@@ -40,7 +40,7 @@ func looksLikeRepo(p string) bool {
 func repoPreAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Handler {
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		if a.RepoPath == "" {
-			helper.Fail500(w, errors.New("repoPreAuthorizeHandler: RepoPath empty"))
+			helper.Fail500(w, requesterror.New("repoPreAuthorizeHandler", r, "RepoPath empty"))
 			return
 		}
 
@@ -65,12 +65,12 @@ func handleGetInfoRefs(w http.ResponseWriter, r *http.Request, a *api.Response)
 	cmd := gitCommand(a.GL_ID, "git", subCommand(rpc), "--stateless-rpc", "--advertise-refs", a.RepoPath)
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("handleGetInfoRefs: stdout: %v", err))
+		helper.Fail500(w, requesterror.New("handleGetInfoRefs", r, "stdout: %v", err))
 		return
 	}
 	defer stdout.Close()
 	if err := cmd.Start(); err != nil {
-		helper.Fail500(w, fmt.Errorf("handleGetInfoRefs: start %v: %v", cmd.Args, err))
+		helper.Fail500(w, requesterror.New("handleGetInfoRefs", r, "start %v: %v", cmd.Args, err))
 		return
 	}
 	defer helper.CleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up
@@ -80,19 +80,21 @@ func handleGetInfoRefs(w http.ResponseWriter, r *http.Request, a *api.Response)
 	w.Header().Add("Cache-Control", "no-cache")
 	w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just return
 	if err := pktLine(w, fmt.Sprintf("# service=%s\n", rpc)); err != nil {
-		helper.LogError(fmt.Errorf("handleGetInfoRefs: pktLine: %v", err))
+		helper.LogError(requesterror.New("handleGetInfoRefs", r, "pktLine: %v", err))
 		return
 	}
 	if err := pktFlush(w); err != nil {
-		helper.LogError(fmt.Errorf("handleGetInfoRefs: pktFlush: %v", err))
+		helper.LogError(requesterror.New("handleGetInfoRefs", r, "pktFlush: %v", err))
 		return
 	}
 	if _, err := io.Copy(w, stdout); err != nil {
-		helper.LogError(fmt.Errorf("handleGetInfoRefs: copy output of %v: %v", cmd.Args, err))
+		helper.LogError(&copyError{
+			requesterror.New("handleGetInfoRefs", r, "copy output of %v: %v", cmd.Args, err),
+		})
 		return
 	}
 	if err := cmd.Wait(); err != nil {
-		helper.LogError(fmt.Errorf("handleGetInfoRefs: wait for %v: %v", cmd.Args, err))
+		helper.LogError(requesterror.New("handleGetInfoRefs", r, "wait for %v: %v", cmd.Args, err))
 		return
 	}
 }
@@ -104,7 +106,7 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	action := filepath.Base(r.URL.Path)
 	if !(action == "git-upload-pack" || action == "git-receive-pack") {
 		// The 'dumb' Git HTTP protocol is not supported
-		helper.Fail500(w, fmt.Errorf("handlePostRPC: unsupported action: %s", r.URL.Path))
+		helper.Fail500(w, requesterror.New("handlePostRPC", r, "unsupported action: %s", r.URL.Path))
 		return
 	}
 
@@ -112,25 +114,25 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 	cmd := gitCommand(a.GL_ID, "git", subCommand(action), "--stateless-rpc", a.RepoPath)
 	stdout, err := cmd.StdoutPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("handlePostRPC: stdout: %v", err))
+		helper.Fail500(w, requesterror.New("handlePostRPC", r, "stdout: %v", err))
 		return
 	}
 	defer stdout.Close()
 	stdin, err := cmd.StdinPipe()
 	if err != nil {
-		helper.Fail500(w, fmt.Errorf("handlePostRPC: stdin: %v", err))
+		helper.Fail500(w, requesterror.New("handlePostRPC", r, "stdin: %v", err))
 		return
 	}
 	defer stdin.Close()
 	if err := cmd.Start(); err != nil {
-		helper.Fail500(w, fmt.Errorf("handlePostRPC: start %v: %v", cmd.Args, err))
+		helper.Fail500(w, requesterror.New("handlePostRPC", r, "start %v: %v", cmd.Args, err))
 		return
 	}
 	defer helper.CleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up
 
 	// Write the client request body to Git's standard input
 	if _, err := io.Copy(stdin, r.Body); err != nil {
-		helper.Fail500(w, fmt.Errorf("handlePostRPC write to %v: %v", cmd.Args, err))
+		helper.Fail500(w, requesterror.New("handlePostRPC", r, "write to %v: %v", cmd.Args, err))
 		return
 	}
 	// Signal to the Git subprocess that no more data is coming
@@ -147,11 +149,13 @@ func handlePostRPC(w http.ResponseWriter, r *http.Request, a *api.Response) {
 
 	// This io.Copy may take a long time, both for Git push and pull.
 	if _, err := io.Copy(w, stdout); err != nil {
-		helper.LogError(fmt.Errorf("handlePostRPC copy output of %v: %v", cmd.Args, err))
+		helper.LogError(&copyError{
+			requesterror.New("handlePostRPC", r, "copy output of %v: %v", cmd.Args, err),
+		})
 		return
 	}
 	if err := cmd.Wait(); err != nil {
-		helper.LogError(fmt.Errorf("handlePostRPC wait for %v: %v", cmd.Args, err))
+		helper.LogError(requesterror.New("handlePostRPC", r, "wait for %v: %v", cmd.Args, err))
 		return
 	}
 }

internal/lfs/lfs.go

@@ -8,8 +8,6 @@ import (
 	"bytes"
 	"crypto/sha256"
 	"encoding/hex"
-	"errors"
-	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
@@ -18,6 +16,7 @@ import (
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/api"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 )
 
 func PutStore(a *api.API, h http.Handler) http.Handler {
@@ -28,17 +27,17 @@ func lfsAuthorizeHandler(myAPI *api.API, handleFunc api.HandleFunc) http.Handler
 	return myAPI.PreAuthorizeHandler(func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 
 		if a.StoreLFSPath == "" {
-			helper.Fail500(w, errors.New("lfsAuthorizeHandler: StoreLFSPath empty"))
+			helper.Fail500(w, requesterror.New("lfsAuthorizeHandler", r, "StoreLFSPath empty"))
 			return
 		}
 
 		if a.LfsOid == "" {
-			helper.Fail500(w, errors.New("lfsAuthorizeHandler: LfsOid empty"))
+			helper.Fail500(w, requesterror.New("lfsAuthorizeHandler", r, "LfsOid empty"))
 			return
 		}
 
 		if err := os.MkdirAll(a.StoreLFSPath, 0700); err != nil {
-			helper.Fail500(w, fmt.Errorf("lfsAuthorizeHandler: mkdir StoreLFSPath: %v", err))
+			helper.Fail500(w, requesterror.New("lfsAuthorizeHandler", r, "mkdir StoreLFSPath: %v", err))
 			return
 		}
 
@@ -50,7 +49,7 @@ func handleStoreLfsObject(h http.Handler) api.HandleFunc {
 	return func(w http.ResponseWriter, r *http.Request, a *api.Response) {
 		file, err := ioutil.TempFile(a.StoreLFSPath, a.LfsOid)
 		if err != nil {
-			helper.Fail500(w, fmt.Errorf("handleStoreLfsObject: create tempfile: %v", err))
+			helper.Fail500(w, requesterror.New("handleStoreLfsObject", r, "create tempfile: %v", err))
 			return
 		}
 		defer os.Remove(file.Name())
@@ -61,19 +60,19 @@ func handleStoreLfsObject(h http.Handler) api.HandleFunc {
 
 		written, err := io.Copy(hw, r.Body)
 		if err != nil {
-			helper.Fail500(w, fmt.Errorf("handleStoreLfsObject: copy body to tempfile: %v", err))
+			helper.Fail500(w, requesterror.New("handleStoreLfsObject", r, "copy body to tempfile: %v", err))
 			return
 		}
 		file.Close()
 
 		if written != a.LfsSize {
-			helper.Fail500(w, fmt.Errorf("handleStoreLfsObject: expected size %d, wrote %d", a.LfsSize, written))
+			helper.Fail500(w, requesterror.New("handleStoreLfsObject", r, "expected size %d, wrote %d", a.LfsSize, written))
 			return
 		}
 
 		shaStr := hex.EncodeToString(hash.Sum(nil))
 		if shaStr != a.LfsOid {
-			helper.Fail500(w, fmt.Errorf("handleStoreLfsObject: expected sha256 %s, got %s", a.LfsOid, shaStr))
+			helper.Fail500(w, requesterror.New("handleStoreLfsObject", r, "expected sha256 %s, got %s", a.LfsOid, shaStr))
 			return
 		}
 

internal/requesterror/requesterror.go

+package requesterror
+
+import (
+	"fmt"
+	"net/http"
+)
+
+// For errors that occur while handling an HTTP request it is often
+// relevant what the request was for. This helper lets us consistently
+// embed request metadata in the error message.
+func New(context string, r *http.Request, format string, a ...interface{}) error {
+	return fmt.Errorf("%s: %s %q: %s", context, r.Method, r.RequestURI, fmt.Sprintf(format, a...))
+}

internal/staticpages/servefile.go

@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/urlprefix"
 )
 
@@ -28,11 +29,11 @@ func (s *Static) ServeExisting(prefix urlprefix.Prefix, cache CacheMode, notFoun
 
 		// The filepath.Join does Clean traversing directories up
 		if !strings.HasPrefix(file, s.DocumentRoot) {
-			helper.Fail500(w, &os.PathError{
+			helper.Fail500(w, requesterror.New("ServeExisting", r, "%v", &os.PathError{
 				Op:   "open",
 				Path: file,
 				Err:  os.ErrInvalid,
-			})
+			}))
 			return
 		}
 

internal/upload/uploads.go

@@ -10,6 +10,7 @@ import (
 	"os"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 )
 
 type MultipartFormProcessor interface {
@@ -104,7 +105,7 @@ func rewriteFormFilesFromMultipart(r *http.Request, writer *multipart.Writer, te
 
 func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, tempPath string, filter MultipartFormProcessor) {
 	if tempPath == "" {
-		helper.Fail500(w, fmt.Errorf("handleFileUploads: tempPath empty"))
+		helper.Fail500(w, requesterror.New("handleFileUploads", r, "tempPath empty"))
 		return
 	}
 
@@ -118,7 +119,7 @@ func HandleFileUploads(w http.ResponseWriter, r *http.Request, h http.Handler, t
 		if err == http.ErrNotMultipart {
 			h.ServeHTTP(w, r)
 		} else {
-			helper.Fail500(w, fmt.Errorf("handleFileUploads: extract files from multipart: %v", err))
+			helper.Fail500(w, requesterror.New("handleFileUploads", r, "extract files from multipart: %v", err))
 		}
 		return
 	}

internal/upstream/handlers.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 
 	"gitlab.com/gitlab-org/gitlab-workhorse/internal/helper"
+	"gitlab.com/gitlab-org/gitlab-workhorse/internal/requesterror"
 )
 
 func contentEncodingHandler(h http.Handler) http.Handler {
@@ -26,7 +27,7 @@ func contentEncodingHandler(h http.Handler) http.Handler {
 		}
 
 		if err != nil {
-			helper.Fail500(w, fmt.Errorf("contentEncodingHandler: %v", err))
+			helper.Fail500(w, requesterror.New("contentEncodingHandler", r, "%v", err))
 			return
 		}
 		defer body.Close()