server/writer: properly handle result encoding errors #7114
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Stephan Renatus <[email protected]>
…te content-type headers Signed-off-by: Stephan Renatus <[email protected]>
srenatus
commented
Oct 11, 2024
| w.WriteHeader(status) | ||
| _, _ = w.Write(append(err.Bytes(), byte('\n'))) | ||
| } | ||
|
|
||
| // JSON writes a response with the specified status code and object. The object | ||
| // will be JSON serialized. | ||
| // Deprecated: This method is problematic when using a non-200 status `code`: if | ||
| // encoding the payload fails, it'll print "superfluous call to WriteHeader()" | ||
| // logs. | ||
| func JSON(w http.ResponseWriter, code int, v interface{}, pretty bool) { |
There was a problem hiding this comment.
There is only a single caller to this method, and it's the health check logging an error. The payloads involved with that are so simple that this problem shouldn't really apply -- but we could refactor this nonetheless.
ashutosh-narkar
approved these changes
Oct 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If the encoding of a result of some sort -- any payload passed to
writer.JSON()orwriter.JSONOK()-- failed, we'd emit a log line likeand nobody likes these. Also, the encoding error would be the response payload, but the status code had still been 200.
Now, errors encoding the payload properly lead to 500 statuses, without extra logs.
Also headers should be set using Set(), not Add(), to avoid response headers like