fix: use json.Marshal in WriteJSONResponse to avoid trailing newline#3466
fix: use json.Marshal in WriteJSONResponse to avoid trailing newline#3466
Conversation
json.Encoder.Encode appends a trailing newline character after the JSON output. This caused 8 proxy tests to fail after commit f9f875b refactored manual w.Write calls to use WriteJSONResponse. Switch from json.NewEncoder(w).Encode(body) to json.Marshal(body) + w.Write(data), which produces clean JSON without a trailing newline. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes proxy test failures by changing httputil.WriteJSONResponse to emit JSON without the trailing newline produced by json.Encoder.Encode.
Changes:
- Replace
json.NewEncoder(w).Encode(body)withjson.Marshal(body)followed byw.Write(data)to avoid a trailing newline. - Add early-return handling for marshal failures.
Show a summary per file
| File | Description |
|---|---|
internal/httputil/httputil.go |
Updates JSON response writing to avoid trailing newline in HTTP responses. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 1
| w.Header().Set("Content-Type", "application/json") | ||
| w.WriteHeader(statusCode) | ||
| json.NewEncoder(w).Encode(body) | ||
| data, err := json.Marshal(body) | ||
| if err != nil { | ||
| return |
There was a problem hiding this comment.
json.Marshal is done after WriteHeader(statusCode). If marshaling fails, this returns after headers/status have already been sent, leaving an empty body with the original status code (often 200) and no indication of the server error. Consider marshaling first, and on error writing an explicit 500 (or at least the correct error status) before sending headers; also consider handling/logging the w.Write error.
See below for a potential fix:
data, err := json.Marshal(body)
if err != nil {
http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError)
return
}
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(statusCode)
if _, err := w.Write(data); err != nil {
return
}
Problem
Commit f9f875b refactored two sites in
internal/proxy/handler.goto usehttputil.WriteJSONResponseinstead of manualw.Writecalls. However,WriteJSONResponseusedjson.NewEncoder(w).Encode(body), which always appends a trailing newline. This caused 8 proxy tests to fail:TestWriteEmptyResponse(6 subtests)TestHandleWithDIFC_LabelResponseError_CoarseBlockedTestHandleWithDIFC_NoFineGrainedLabels_CoarseBlockedAll failures showed the same pattern: expected
"[]"but got"[]\n".Fix
Switch
WriteJSONResponsefromjson.NewEncoder(w).Encode(body)tojson.Marshal(body)+w.Write(data), which produces clean JSON without a trailing newline. This is the standard approach for HTTP JSON responses.Verification
make agent-finishedpasses — all unit and integration tests green.