Skip to content

Commit

Permalink
contrib/internal/httputil: only set status once with WriteHeader (#629)
Browse files Browse the repository at this point in the history
The net/http only respects the first call to WriteHeader, ignoring others.
This commit changes our responseWriter wrapper to do the same so we will
get accurate reporting of status codes and errors when users call
WriteHeader multiple times.

Fixes #623
  • Loading branch information
knusbaum authored and gbbr committed Apr 13, 2020
1 parent fa73cfc commit 9b02ec4
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 0 deletions.
3 changes: 3 additions & 0 deletions contrib/internal/httputil/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ func (w *responseWriter) Write(b []byte) (int, error) {
// WriteHeader sends an HTTP response header with status code.
// It also sets the status code to the span.
func (w *responseWriter) WriteHeader(status int) {
if w.status != 0 {
return
}
w.ResponseWriter.WriteHeader(status)
w.status = status
w.span.SetTag(ext.HTTPCode, strconv.Itoa(status))
Expand Down
19 changes: 19 additions & 0 deletions contrib/internal/httputil/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,23 @@ func TestTraceAndServe(t *testing.T) {
assert.True(called)
assert.Equal(c.ParentID(), p.SpanID())
})

t.Run("doubleStatus", func(t *testing.T) {
mt := mocktracer.Start()
assert := assert.New(t)
defer mt.Stop()

handler := func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusOK)
w.WriteHeader(http.StatusInternalServerError)
}
r, err := http.NewRequest("GET", "/", nil)
assert.NoError(err)
w := httptest.NewRecorder()
TraceAndServe(http.HandlerFunc(handler), w, r, "service", "resource", nil)

spans := mt.FinishedSpans()
assert.Len(spans, 1)
assert.Equal("200", spans[0].Tag(ext.HTTPCode))
})
}

0 comments on commit 9b02ec4

Please sign in to comment.