Skip to content

add Unwrap()#6

Merged
irees merged 1 commit into
mainfrom
fix-sse
May 9, 2026
Merged

add Unwrap()#6
irees merged 1 commit into
mainfrom
fix-sse

Conversation

@irees

@irees irees commented May 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds Unwrap() http.ResponseWriter to the logging middleware's response wrapper so http.ResponseController can navigate past it to find Flusher, Hijacker, and other interfaces on the underlying writer. Without this, any SSE/streaming handler downstream of DurationLoggingMiddleware cannot flush and either errors out or silently truncates.

Background

DurationLoggingMiddleware wraps http.ResponseWriter in *responseWriter to capture the status code and response size for logging. The wrapper embeds http.ResponseWriter but the embedded interface doesn't expose Flush/Hijack/Push, so a downstream w.(http.Flusher) type assertion fails. The standard-library remedy is for wrappers to implement Unwrap() http.ResponseWriter, which http.ResponseController follows.

Discovered via

A new SSE endpoint (GET /jobs/queues/{queue}/jobs/{jobId}/watch) in transitland-lib's jobserver returned an empty body when mounted in tlv2's middleware chain — the chain includes this logging middleware, which was breaking Flush().

Test plan

  • Unit: existing lmw_test.go continues to pass.
  • Manual: in tlv2 with --enable-jobs-api, confirm curl -N $BASE/queues/$Q/jobs/$JOB_ID/watch against an already-terminal job emits the terminal event and event: end sentinel before the connection closes. Before this change, the body was empty.

Copilot AI review requested due to automatic review settings May 9, 2026 01:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds an Unwrap() method to the logging responseWriter wrapper so net/http’s http.ResponseController can traverse through the wrapper to reach capabilities (e.g., flushing/hijacking) provided by the underlying http.ResponseWriter.

Changes:

  • Implement (*responseWriter).Unwrap() http.ResponseWriter in lmw.go.
  • Document why the wrapper needs to be unwrap-able for downstream streaming use-cases.

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

Comment thread lmw.go
@irees irees changed the title add Unrwap() add Unwrap() May 9, 2026
@irees irees merged commit c913c7a into main May 9, 2026
9 checks passed
@irees irees deleted the fix-sse branch May 9, 2026 01:48
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.

2 participants