Fix: Use WriteHeader in json method for consistent status handling#2866
Fix: Use WriteHeader in json method for consistent status handling#2866raju-mechatronics wants to merge 1 commit intolabstack:masterfrom
Conversation
|
@aldas please take a look when you get a chance. |
|
Hi, this is a "feature" that Echo uses. I really do not want to touch this part as it could introduce subtle bugs for people that rely on this "feature". |
|
Hi @aldas, Thank you very much for taking the time to review and for the clear explanation — I really appreciate your insight! I proposed this change because Minimal example showing the difference: // /multiple-json → no warnings
e.GET("/multiple", func(c echo.Context) error {
responses := []Response{
{ID: 1, Name: "first"},
{ID: 2, Name: "second"},
{ID: 3, Name: "third"},
}
c.JSON(200, responses[0])
c.JSON(200, responses[1])
c.JSON(200, responses[2])
return nil
})
// /multiple-xml → warns twice
e.GET("/multiple_xml", func(c echo.Context) error {
responses := []Response{
{ID: 1, Name: "first"},
{ID: 2, Name: "second"},
{ID: 3, Name: "third"},
}
c.XML(200, responses[0])
c.XML(200, responses[1])
c.XML(200, responses[2])
return nil
})the logs: I thought aligning JSON() with the other methods would help catch accidental multiple-response mistakes more consistently. |
|
Seems that in Lines 454 to 458 in 096ce41 For history sake: I am not going to touch this part in |
|
okay. Thanks for the clarification. |
|
Hey @raju-mechatronics, thanks for taking the initiative here. Even though this PR didn’t get merged, it’s great to have people digging into the library’s internals and reviewing how things work. Thanks! |
Bug Fix
Problem
The
json()method incontext.go(line 504) was inconsistent with other response methods in how it sets the HTTP status code.Current behavior:
This approach directly sets the
Statusfield instead of properly callingWriteHeader(), which bypasses header commitment and prevents warnings about header modifications after the status is set.Solution
Updated the method to use
c.response.WriteHeader(code)for consistency with other response methods:All other similar response methods already use
WriteHeader():jsonPBlob()- line 489:c.response.WriteHeader(code)xml()- line 543:c.response.WriteHeader(code)Blob()- line 578:c.response.WriteHeader(code)JSONPBlob()- line 530:c.response.WriteHeader(code)