Skip to content

Conversation

@Neurostep
Copy link
Contributor

@Neurostep Neurostep commented Oct 30, 2025

Description

In this PR we make the logging Response Writer wrapper SSE compatible by adding the Flush method to it.

Testing considerations

  • Use in API Gateway and see if middleware works as expected with SSE enabled for GraphQL

Checklist

  • Prefixed the PR title with the JIRA ticket code
  • Performed simple, atomic commits with good commit messages
  • Verified that the commit history is linear and commits are squashed as necessary
  • Thoroughly tested the changes in development and/or staging
  • Updated the README.md as necessary

Related links

Copilot AI review requested due to automatic review settings October 30, 2025 12:48
@Neurostep Neurostep requested a review from a team as a code owner October 30, 2025 12:48
@Neurostep Neurostep requested a review from Xanderwot October 30, 2025 12:48
Copy link

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 adds HTTP Flusher support to the loggingResponseWriter by implementing the Flush() method. This allows the wrapped response writer to properly support streaming responses like Server-Sent Events (SSE) or chunked transfer encoding.

Key Changes

  • Implements Flush() method on loggingResponseWriter that delegates to the underlying http.ResponseWriter if it implements http.Flusher

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

Comment on lines +127 to +131
func (lrw *loggingResponseWriter) Flush() {
if f, ok := lrw.ResponseWriter.(http.Flusher); ok {
f.Flush()
}
}
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

The Flush method should include a comment documenting its purpose, similar to the WriteHeader method above it. This is especially important since it implements the http.Flusher interface and its silent failure behavior (when the underlying writer doesn't support flushing) should be documented.

Copilot uses AI. Check for mistakes.
Copy link
Member

@terranisu terranisu left a comment

Choose a reason for hiding this comment

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

Looks flushed to me

@Neurostep Neurostep merged commit 6f68834 into main Oct 30, 2025
14 checks passed
@Neurostep Neurostep deleted the maksimt/DEVPLAT-6026/sse-middleware-support branch October 30, 2025 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants