Skip to content

mcp: use http.ResponseController to ensure writes are flushed#870

Merged
maciej-kisiel merged 1 commit intomodelcontextprotocol:mainfrom
entalas:better-response-flushing
Mar 31, 2026
Merged

mcp: use http.ResponseController to ensure writes are flushed#870
maciej-kisiel merged 1 commit intomodelcontextprotocol:mainfrom
entalas:better-response-flushing

Conversation

@toofishes
Copy link
Copy Markdown
Contributor

If the ResponseWriter is a wrapped one (which is quite common when using middleware to log things like response sizes), the direct cast to http.Flusher doesn't find the flush capability. Instead, use http.ResponseController as introduced in go1.20 which handles unwrapping to find the base response writer.

@maciej-kisiel
Copy link
Copy Markdown
Contributor

This assumes that the wrapper implements Unwrap, right? Is that a common practice?

@toofishes
Copy link
Copy Markdown
Contributor Author

This assumes that the wrapper implements Unwrap, right? Is that a common practice?

The wrapper can implement Unwrap, implement http.Flusher directly, or a FlushError() method that returns an error, and it will continue to work. https://go.dev/src/net/http/responsecontroller.go shows the relatively straightforward switch on interface type.

It seems like it is still not as well known as it should be, but it isn't uncommon:

Note that several of these also implement Flush, either because they need to do additional work, or predate the ResponseController Unwrap interface existing.

An additional change I plan to propose (in a future PR) is also calling SetWriteDeadline(time.Time{}) on the response once a connection has entered the event stream phase, which also benefits from being exposed on the response controller. This would prevent a http.Server WriteTimeout from closing SSE connections. I currently need to install this middleware on the MCP route otherwise:

func DisableWriteDeadlineMiddleware(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		rc := http.NewResponseController(w)
		if err := rc.SetWriteDeadline(time.Time{}); err != nil {
			logging.ContextLogger(r.Context()).Warn("unable to disable write deadline", zap.Error(err))
		}
		next.ServeHTTP(w, r)
	})
}

Copy link
Copy Markdown
Contributor

@maciej-kisiel maciej-kisiel left a comment

Choose a reason for hiding this comment

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

Please merge main.

If the ResponseWriter is a wrapped one (which is quite common when using
middleware to log things like response sizes), the direct cast to
`http.Flusher` doesn't find the flush capability. Instead, use
`http.ResponseController` as introduced in go1.20 which handles unwrapping to
find the base response writer.
@toofishes toofishes force-pushed the better-response-flushing branch from 8d30ffe to 5be9a54 Compare March 31, 2026 10:49
@toofishes
Copy link
Copy Markdown
Contributor Author

Added the suggested comments and rebased on main.

@maciej-kisiel maciej-kisiel merged commit d3fd25b into modelcontextprotocol:main Mar 31, 2026
9 checks passed
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