Skip to content

Extract BaseResponseWriter to httputil to eliminate duplicate status-capture code#6106

Merged
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-http-response-writer
May 20, 2026
Merged

Extract BaseResponseWriter to httputil to eliminate duplicate status-capture code#6106
lpcox merged 3 commits into
mainfrom
copilot/duplicate-code-http-response-writer

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 20, 2026

internal/server/response_writer.go and internal/tracing/http.go each independently implemented an http.ResponseWriter wrapper with identical WriteHeader/Write/Unwrap status-capture logic — and had already diverged (implicit-200 handling present in tracing but absent in server).

Changes

  • internal/httputil/response_writer.go — new BaseResponseWriter embedding http.ResponseWriter with:

    • WriteHeader: captures status code and forwards
    • Write: captures implicit 200 on first write if no prior WriteHeader
    • Unwrap: exposes underlying writer for http.ResponseController interface delegation (http.Flusher, etc.)
  • internal/tracing/http.gostatusResponseWriter now just embeds httputil.BaseResponseWriter; its own WriteHeader, Write, and Unwrap implementations removed (all promoted from base)

  • internal/server/response_writer.goresponseWriter embeds httputil.BaseResponseWriter instead of raw http.ResponseWriter; retains its own WriteHeader/Write only for logging and body-capture, delegating the status logic to the base type

// Before: duplicated across two packages
type statusResponseWriter struct {
    http.ResponseWriter
    statusCode int
}
func (rw *statusResponseWriter) WriteHeader(code int) { rw.statusCode = code; rw.ResponseWriter.WriteHeader(code) }
func (rw *statusResponseWriter) Write(b []byte) (int, error) { /* implicit-200 logic */ }
func (rw *statusResponseWriter) Unwrap() http.ResponseWriter { return rw.ResponseWriter }

// After: single definition in httputil, embedded where needed
type statusResponseWriter struct {
    httputil.BaseResponseWriter
}
  • internal/httputil/response_writer_test.go — full test coverage for BaseResponseWriter
  • internal/tracing/http_test.go — updated to use promoted StatusCode field

Copilot AI changed the title [WIP] Refactor duplicate HTTP response writer status-code wrappers Extract BaseResponseWriter to httputil to eliminate duplicate status-capture code May 20, 2026
Copilot finished work on behalf of lpcox May 20, 2026 17:06
Copilot AI requested a review from lpcox May 20, 2026 17:06
@lpcox lpcox marked this pull request as ready for review May 20, 2026 18:21
Copilot AI review requested due to automatic review settings May 20, 2026 18:21
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR centralizes HTTP status-code capture logic by introducing a shared httputil.BaseResponseWriter and reusing it from both tracing and server response-writer wrappers, eliminating duplicated (and previously diverged) implementations.

Changes:

  • Added internal/httputil/BaseResponseWriter to capture status codes (including implicit 200) and support http.ResponseController via Unwrap.
  • Refactored tracing’s statusResponseWriter to embed the shared base wrapper and updated call sites/tests accordingly.
  • Updated the server responseWriter to embed the shared base wrapper while retaining body capture + logging.
Show a summary per file
File Description
internal/httputil/response_writer.go Introduces shared BaseResponseWriter implementation for status capture + Unwrap.
internal/httputil/response_writer_test.go Adds unit tests for the new base response-writer behavior.
internal/tracing/http.go Refactors tracing response-writer wrapper to embed the shared base type.
internal/tracing/http_test.go Updates tests to validate status capture via the promoted StatusCode field.
internal/server/response_writer.go Updates server response-writer to embed BaseResponseWriter and delegate status logic while keeping body buffering/logging.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 5/5 changed files
  • Comments generated: 2

Comment on lines +14 to +29
// StatusCode holds the captured HTTP status code. It is set by WriteHeader
// and, if still zero when Write is first called, defaults to http.StatusOK.
StatusCode int
}

// WriteHeader captures the status code and forwards it to the underlying writer.
func (w *BaseResponseWriter) WriteHeader(code int) {
w.StatusCode = code
w.ResponseWriter.WriteHeader(code)
}

// Write captures an implicit 200 status when called without a prior WriteHeader,
// then delegates to the underlying writer.
func (w *BaseResponseWriter) Write(b []byte) (int, error) {
if w.StatusCode == 0 {
w.StatusCode = http.StatusOK
Comment thread internal/httputil/response_writer.go Outdated
Comment on lines +25 to +31
// Write captures an implicit 200 status when called without a prior WriteHeader,
// then delegates to the underlying writer.
func (w *BaseResponseWriter) Write(b []byte) (int, error) {
if w.StatusCode == 0 {
w.StatusCode = http.StatusOK
}
return w.ResponseWriter.Write(b)
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 20, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 20, 2026

@copilot address review feedback

Fixed in the latest commit. BaseResponseWriter now tracks a private wroteHeader flag:

  • WriteHeader: only captures StatusCode on the first call; subsequent calls are forwarded to the underlying writer without mutating the captured status.
  • Write: sets wroteHeader = true alongside the implicit-200 capture, so a later WriteHeader can no longer overwrite it.

Two new tests cover both scenarios (TestBaseResponseWriter_WriteHeader_FirstCallWins and TestBaseResponseWriter_Write_LaterWriteHeaderDoesNotOverrideImplicit200).

Copilot finished work on behalf of lpcox May 20, 2026 18:31
@lpcox lpcox merged commit b3d3252 into main May 20, 2026
14 checks passed
@lpcox lpcox deleted the copilot/duplicate-code-http-response-writer branch May 20, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[duplicate-code] Duplicate Code Pattern: HTTP Response Writer Status-Code Wrapper in server and tracing packages

3 participants