Skip to content

fix: prevent superfluous response.WriteHeader call in EncodeJSONResponse#537

Open
Kubudak90 wants to merge 1 commit into
coinbase:masterfrom
Kubudak90:fix-superfluous-writeheader
Open

fix: prevent superfluous response.WriteHeader call in EncodeJSONResponse#537
Kubudak90 wants to merge 1 commit into
coinbase:masterfrom
Kubudak90:fix-superfluous-writeheader

Conversation

@Kubudak90

Copy link
Copy Markdown

Description

Fixes #407

The EncodeJSONResponse function was calling w.WriteHeader(status) before encoding the JSON response. If json.NewEncoder(w).Encode(i) failed, it would call http.Error() which internally calls WriteHeader again, causing the "http: superfluous response.WriteHeader call" error when running rosetta-cli check:data with high concurrency.

Changes

  • Encode JSON to a buffer first
  • Only write the header if encoding succeeds
  • Use http.Error() only on encoding failure (before any header is written)

This follows the Go HTTP best practice of validating content before writing headers to avoid the superfluous WriteHeader error described in Go Forum.

Testing

The fix ensures that:

  1. On successful JSON encoding: Content-Type header is set, status is written, and JSON body is written
  2. On encoding failure: http.Error() handles the response with proper error message and 500 status

No duplicate WriteHeader calls can occur with this fix.

The EncodeJSONResponse function was calling w.WriteHeader(status) before
encoding the JSON response. If json.NewEncoder(w).Encode(i) failed, it would
call http.Error() which internally calls WriteHeader again, causing the
"http: superfluous response.WriteHeader call" error.

This fix:
1. Encodes the JSON to a buffer first
2. Only writes the header if encoding succeeds
3. Uses http.Error() only on encoding failure (before any header is written)

Fixes coinbase#407
@cb-heimdall

Copy link
Copy Markdown

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

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.

EncodeJSONResponse - http: superfluous response.WriteHeader call

2 participants