Commit 1c34f8a8 authored by Jacob Vosmaer's avatar Jacob Vosmaer

Provide context for logged errors

Change in behavior: we stop panicking in the middle of writing a
response body; instead we do a 'clean' return from the handler.
parent 9ed11af6
Pipeline #521 failed with stage
...@@ -52,7 +52,7 @@ func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ...@@ -52,7 +52,7 @@ func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
var env gitEnv var env gitEnv
var g gitService var g gitService
log.Print(r.Method, " ", r.URL) log.Printf("%s %q", r.Method, r.URL)
// Look for a matching Git service // Look for a matching Git service
foundService := false foundService := false
...@@ -73,7 +73,7 @@ func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ...@@ -73,7 +73,7 @@ func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// user ID (GL_ID) is. // user ID (GL_ID) is.
authResponse, err := h.doAuthRequest(r) authResponse, err := h.doAuthRequest(r)
if err != nil { if err != nil {
fail500(w, err) fail500(w, "doAuthRequest", err)
return return
} }
defer authResponse.Body.Close() defer authResponse.Body.Close()
...@@ -97,7 +97,7 @@ func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { ...@@ -97,7 +97,7 @@ func (h *gitHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
// information from the auth response body. // information from the auth response body.
dec := json.NewDecoder(authResponse.Body) dec := json.NewDecoder(authResponse.Body)
if err := dec.Decode(&env); err != nil { if err := dec.Decode(&env); err != nil {
fail500(w, err) fail500(w, "decode JSON GL_ID", err)
return return
} }
// Don't hog a TCP connection in CLOSE_WAIT, we can already close it now // Don't hog a TCP connection in CLOSE_WAIT, we can already close it now
...@@ -155,12 +155,12 @@ func handleGetInfoRefs(env gitEnv, _ string, path string, w http.ResponseWriter, ...@@ -155,12 +155,12 @@ func handleGetInfoRefs(env gitEnv, _ string, path string, w http.ResponseWriter,
cmd := gitCommand(env, "git", subCommand(rpc), "--stateless-rpc", "--advertise-refs", path) cmd := gitCommand(env, "git", subCommand(rpc), "--stateless-rpc", "--advertise-refs", path)
stdout, err := cmd.StdoutPipe() stdout, err := cmd.StdoutPipe()
if err != nil { if err != nil {
fail500(w, err) fail500(w, "handleGetInfoRefs", err)
return return
} }
defer stdout.Close() defer stdout.Close()
if err := cmd.Start(); err != nil { if err := cmd.Start(); err != nil {
fail500(w, err) fail500(w, "handleGetInfoRefs", err)
return return
} }
defer cleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up defer cleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up
...@@ -168,18 +168,22 @@ func handleGetInfoRefs(env gitEnv, _ string, path string, w http.ResponseWriter, ...@@ -168,18 +168,22 @@ func handleGetInfoRefs(env gitEnv, _ string, path string, w http.ResponseWriter,
// Start writing the response // Start writing the response
w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-advertisement", rpc)) w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-advertisement", rpc))
w.Header().Add("Cache-Control", "no-cache") w.Header().Add("Cache-Control", "no-cache")
w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just panic 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 { if err := pktLine(w, fmt.Sprintf("# service=%s\n", rpc)); err != nil {
panic(err) logContext("handleGetInfoRefs response", err)
return
} }
if err := pktFlush(w); err != nil { if err := pktFlush(w); err != nil {
panic(err) logContext("handleGetInfoRefs response", err)
return
} }
if _, err := io.Copy(w, stdout); err != nil { if _, err := io.Copy(w, stdout); err != nil {
panic(err) logContext("handleGetInfoRefs read from subprocess", err)
return
} }
if err := cmd.Wait(); err != nil { if err := cmd.Wait(); err != nil {
panic(err) logContext("handleGetInfoRefs wait for subprocess", err)
return
} }
} }
...@@ -191,7 +195,7 @@ func handlePostRPC(env gitEnv, rpc string, path string, w http.ResponseWriter, r ...@@ -191,7 +195,7 @@ func handlePostRPC(env gitEnv, rpc string, path string, w http.ResponseWriter, r
if r.Header.Get("Content-Encoding") == "gzip" { if r.Header.Get("Content-Encoding") == "gzip" {
body, err = gzip.NewReader(r.Body) body, err = gzip.NewReader(r.Body)
if err != nil { if err != nil {
fail500(w, err) fail500(w, "handlePostRPC", err)
return return
} }
} else { } else {
...@@ -202,25 +206,25 @@ func handlePostRPC(env gitEnv, rpc string, path string, w http.ResponseWriter, r ...@@ -202,25 +206,25 @@ func handlePostRPC(env gitEnv, rpc string, path string, w http.ResponseWriter, r
cmd := gitCommand(env, "git", subCommand(rpc), "--stateless-rpc", path) cmd := gitCommand(env, "git", subCommand(rpc), "--stateless-rpc", path)
stdout, err := cmd.StdoutPipe() stdout, err := cmd.StdoutPipe()
if err != nil { if err != nil {
fail500(w, err) fail500(w, "handlePostRPC", err)
return return
} }
defer stdout.Close() defer stdout.Close()
stdin, err := cmd.StdinPipe() stdin, err := cmd.StdinPipe()
if err != nil { if err != nil {
fail500(w, err) fail500(w, "handlePostRPC", err)
return return
} }
defer stdin.Close() defer stdin.Close()
if err := cmd.Start(); err != nil { if err := cmd.Start(); err != nil {
fail500(w, err) fail500(w, "handlePostRPC", err)
return return
} }
defer cleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up defer cleanUpProcessGroup(cmd) // Ensure brute force subprocess clean-up
// Write the client request body to Git's standard input // Write the client request body to Git's standard input
if _, err := io.Copy(stdin, body); err != nil { if _, err := io.Copy(stdin, body); err != nil {
fail500(w, err) fail500(w, "handlePostRPC write to subprocess", err)
return return
} }
stdin.Close() stdin.Close()
...@@ -228,18 +232,24 @@ func handlePostRPC(env gitEnv, rpc string, path string, w http.ResponseWriter, r ...@@ -228,18 +232,24 @@ func handlePostRPC(env gitEnv, rpc string, path string, w http.ResponseWriter, r
// Start writing the response // Start writing the response
w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-result", rpc)) w.Header().Add("Content-Type", fmt.Sprintf("application/x-%s-result", rpc))
w.Header().Add("Cache-Control", "no-cache") w.Header().Add("Cache-Control", "no-cache")
w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just panic w.WriteHeader(200) // Don't bother with HTTP 500 from this point on, just return
if _, err := io.Copy(w, stdout); err != nil { if _, err := io.Copy(w, stdout); err != nil {
panic(err) logContext("handlePostRPC read from subprocess", err)
return
} }
if err := cmd.Wait(); err != nil { if err := cmd.Wait(); err != nil {
panic(err) logContext("handlePostRPC wait for subprocess", err)
return
} }
} }
func fail500(w http.ResponseWriter, err error) { func fail500(w http.ResponseWriter, context string, err error) {
http.Error(w, "Internal server error", 500) http.Error(w, "Internal server error", 500)
log.Print(err) logContext(context, err)
}
func logContext(context string, err error) {
log.Printf("%s: %v", context, err)
} }
// Git subprocess helpers // Git subprocess helpers
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment