Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress aborthandler #21479

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

ianseyer
Copy link

@ianseyer ianseyer commented Feb 3, 2025

Fixes #18087, #15751, #10813, #12011, #16556, #20148

As per golang/go#28239, it is the expectation that middleware using net/http is responsible for handling http.AbortHandler panics that are automatically recovered.

In the current situation, any network connectivity blip, despite recovering successfully, results in a panic being written to the logs.

I have tested this build in our staging environment, and the panic message is indeed suppressed. We do see a proxy connection interrupted log message, which is to be expected (we have a reliable source of network connectivity blips).

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@ianseyer ianseyer requested a review from a team as a code owner February 3, 2025 17:06
@Vad1mo Vad1mo added the release-note/update Update or Fix label Feb 3, 2025
@Vad1mo
Copy link
Member

Vad1mo commented Feb 5, 2025

Thank you for your contribution, this is a quite common issue, we have also seen ourself.

@ianseyer
Copy link
Author

@reasonerjt what is the follow-up required?

@ianseyer ianseyer force-pushed the suppress-aborthandler branch 2 times, most recently from 8e1f7c9 to 4fb09f4 Compare February 19, 2025 15:44
dependabot bot and others added 2 commits February 19, 2025 09:44
…src (goharbor#21162)

Bumps [go.opentelemetry.io/otel](https://github.com/open-telemetry/opentelemetry-go) from 1.31.0 to 1.32.0.
- [Release notes](https://github.com/open-telemetry/opentelemetry-go/releases)
- [Changelog](https://github.com/open-telemetry/opentelemetry-go/blob/main/CHANGELOG.md)
- [Commits](open-telemetry/opentelemetry-go@v1.31.0...v1.32.0)

---
updated-dependencies:
- dependency-name: go.opentelemetry.io/otel
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: miner <yminer@vmware.com>
Signed-off-by: ianseyer <iseyer@cloudflare.com>
Signed-off-by: ianseyer <iseyer@cloudflare.com>
@ianseyer ianseyer force-pushed the suppress-aborthandler branch from 4fb09f4 to 6962389 Compare February 19, 2025 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handler crashed with error net/http: abort Handler
6 participants