Skip to content

Make HTTPErrorHandler idempotent on committed responses#4243

Merged
reinkrul merged 1 commit into
masterfrom
worktree-echo-errors-idempotent
May 11, 2026
Merged

Make HTTPErrorHandler idempotent on committed responses#4243
reinkrul merged 1 commit into
masterfrom
worktree-echo-errors-idempotent

Conversation

@stevenvegt
Copy link
Copy Markdown
Member

@stevenvegt stevenvegt commented May 11, 2026

Summary

Make core.CreateHTTPErrorHandler short-circuit when ctx.Response().Committed is true, mirroring echo.DefaultHTTPErrorHandler.

Why

Echo's BodyDump middleware (used when http.log: metadata-and-body) calls c.Error(err) on its way out and still returns the error, so Echo's server loop invokes HTTPErrorHandler a second time for the same request. The first invocation wrote the response correctly; the second logged the operation error a second time and warned

WARN: Unable to send error back to client, response already committed

echo.DefaultHTTPErrorHandler already short-circuits on Committed == true. This change brings ours in line.

Extracted from

This commit was originally part of #4220 but was unrelated to that PR's scope; pulled out per @reinkrul's review comment.

Test plan

  • CI passes
  • core/echo_errors_test.go covers the double-invocation case

Echo's BodyDump middleware (used when http.log: metadata-and-body) calls
c.Error(err) on its way out and still returns the err, so echo's server
loop invokes HTTPErrorHandler a second time for the same request. The
first invocation wrote the response correctly; the second logged the
operation error a second time and warned "Unable to send error back to
client, response already committed" — visible noise without a real
problem. echo.DefaultHTTPErrorHandler short-circuits on Committed=true;
ours did not. Mirror that behavior.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qltysh
Copy link
Copy Markdown
Contributor

qltysh Bot commented May 11, 2026

0 new issues

Tool Category Rule Count

@qltysh
Copy link
Copy Markdown
Contributor

qltysh Bot commented May 11, 2026

Qlty


Coverage Impact

⬆️ Merging this pull request will increase total coverage on master by 0.03%.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: B Coverage rating: A
core/echo_errors.go93.3%93
Total93.3%
🤖 Increase coverage with AI coding...
In the `worktree-echo-errors-idempotent` branch, add test coverage for this new code:

- `core/echo_errors.go` -- Line 93

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@reinkrul reinkrul merged commit 180c2e4 into master May 11, 2026
11 of 12 checks passed
@reinkrul reinkrul deleted the worktree-echo-errors-idempotent branch May 11, 2026 11:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants