Skip to content

Add warning when writing response body multiple times (fixes #4477) #4483

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
raju-mechatronics wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add warning when writing response body multiple times (fixes #4477) #4483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8eb015d
fix(context): added warning for multiple writes to response body
raju-mechatronics Dec 28, 2025
dc128b7
test(context): add tests for multiple JSON and SSE writes in debug mode
raju-mechatronics Dec 28, 2025
4d32cf9
Merge branch 'master' into master
raju-mechatronics Dec 28, 2025
efdb77e
Merge branch 'master' into master
appleboy Jan 2, 2026
bc46a09
Merge branch 'master' into master
appleboy Feb 28, 2026
7b5727c
Merge branch 'master' into master
appleboy Feb 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1158,6 +1158,15 @@ func (c *Context) Render(code int, r render.Render) {
return
}

if c.Writer.Written() && IsDebugging() {
// Skip warning for SSE and streaming responses (status code -1)
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

The inline comment says “Skip warning for SSE and streaming responses (status code -1)”, but code == -1 is also used for non-streaming renders like render.Redirect, and the logic also has a separate sse.Event type check. Please clarify the comment (and/or consolidate the conditions) so future readers understand why -1 and sse.Event are treated specially.

Suggested change
// Skip warning for SSE and streaming responses (status code -1)
// Skip this warning when:
// - status code is -1, which is used for special/streaming-style renders (including some
// non-streaming helpers like redirects) where multiple writes are expected, and
// - the renderer is an SSE event, since Server-Sent Events are sent as a stream.

Copilot uses AI. Check for mistakes.
if code != -1 {
if _, ok := r.(sse.Event); !ok {
debugPrint("[WARNING] Response body already written. Attempting to write again with status code %d", code)
}
Comment on lines +1161 to +1166
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

c.Writer.Written() becomes true as soon as headers are written (Size()==0), not only after body bytes are written. This can emit a false warning on the first body write if a handler/middleware flushes headers early (e.g., calls c.Writer.WriteHeaderNow()/Flush()) and then renders JSON. To specifically detect multiple body writes (as the message says), consider checking c.Writer.Size() > 0 (or equivalent) instead of Written().

Copilot uses AI. Check for mistakes.
}
}

if err := r.Render(c.Writer); err != nil {
// Pushing error to c.Errors
_ = c.Error(err)
Expand Down
37 changes: 37 additions & 0 deletions context_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1424,6 +1424,43 @@ func TestContextRenderNoContentData(t *testing.T) {
assert.Equal(t, "text/csv", w.Header().Get("Content-Type"))
}

// Test multiple JSON writes in debug mode
func TestContextRenderMultipleJSON(t *testing.T) {
w := httptest.NewRecorder()
c, _ := CreateTestContext(w)

oldMode := os.Getenv("GIN_MODE")
defer os.Setenv("GIN_MODE", oldMode)
SetMode(DebugMode)
Comment on lines +1432 to +1434
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

oldMode := os.Getenv("GIN_MODE") / defer os.Setenv(...) is redundant here because the test never modifies the environment variable, and SetMode(...) does not read GIN_MODE after init. Consider removing the env save/restore and instead restoring the previous gin mode via Mode() + SetMode.

Copilot uses AI. Check for mistakes.

Comment on lines +1432 to +1435
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

These tests call SetMode(DebugMode) but never restore the previous gin mode, which can leak global state into later tests (the suite defaults to TestMode). Capture the prior Mode() (or assume TestMode) and defer SetMode(...) to restore it after the test.

Copilot uses AI. Check for mistakes.
output := captureOutput(t, func() {
c.JSON(http.StatusOK, H{"foo": "bar"})
c.JSON(http.StatusOK, H{"baz": "qux"})
})

assert.Equal(t, http.StatusOK, w.Code)
assert.Contains(t, output, "[WARNING] Response body already written")
assert.Contains(t, output, "status code 200")
}

// Test multiple SSE writes in debug mode
func TestContextRenderMultipleSSE(t *testing.T) {
w := httptest.NewRecorder()
c, _ := CreateTestContext(w)

oldMode := os.Getenv("GIN_MODE")
defer os.Setenv("GIN_MODE", oldMode)
SetMode(DebugMode)
Comment on lines +1451 to +1453
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

As in the previous test, the GIN_MODE env save/restore is unused because the test never changes GIN_MODE, and SetMode does not consult the env var after startup. Prefer removing this and restoring the prior gin mode via Mode()/SetMode instead.

Copilot uses AI. Check for mistakes.

Comment on lines +1451 to +1454
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

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

This test calls SetMode(DebugMode) but never restores the previous gin mode, which can leak global state into later tests. Please defer SetMode(...) to restore the mode (usually TestMode) after the assertions complete.

Copilot uses AI. Check for mistakes.
output := captureOutput(t, func() {
c.SSEvent("message", "test1")
c.SSEvent("message", "test2")
})

assert.Equal(t, http.StatusOK, w.Code)
assert.NotContains(t, output, "[WARNING] Response body already written")
}

func TestContextRenderSSE(t *testing.T) {
w := httptest.NewRecorder()
c, _ := CreateTestContext(w)
Expand Down