Skip to content

*: fix QUIC false error#4585

Merged
KaloyanTanev merged 2 commits into
mainfrom
kalo/quic-false-error
Jul 6, 2026
Merged

*: fix QUIC false error#4585
KaloyanTanev merged 2 commits into
mainfrom
kalo/quic-false-error

Conversation

@KaloyanTanev

Copy link
Copy Markdown
Collaborator

No need to error on closing the write if the stream was already canceled.
Change was provoked by this failing unit test:
https://github.com/ObolNetwork/charon/actions/runs/28664948834/job/85014276978?pr=4575

category: misc
ticket: none

@KaloyanTanev KaloyanTanev requested a review from pinebit July 6, 2026 14:29
@KaloyanTanev KaloyanTanev self-assigned this Jul 6, 2026
@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 25.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.29%. Comparing base (087db8b) to head (b486119).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
p2p/sender.go 25.00% 6 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4585   +/-   ##
=======================================
  Coverage   57.29%   57.29%           
=======================================
  Files         245      245           
  Lines       33537    33559   +22     
=======================================
+ Hits        19215    19228   +13     
- Misses      11882    11892   +10     
+ Partials     2440     2439    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents benign QUIC “canceled stream” errors from failing request/response flows by treating that specific close error as non-fatal (especially when the peer has already reset the send direction), while still surfacing other close failures.

Changes:

  • Treat “canceled stream” errors from CloseWrite() in SendReceive as benign (debug log + continue).
  • Replace repeated string checks with a shared helper isCanceledStreamErr.
  • Add unit tests covering isCanceledStreamErr behavior (including wrapped errors).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
p2p/sender.go Ignores benign canceled-stream errors on close paths (incl. CloseWrite) and centralizes detection logic in a helper.
p2p/sender_internal_test.go Adds focused tests to lock in the helper’s behavior for nil/other/canceled/wrapped errors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@KaloyanTanev KaloyanTanev merged commit 31ad8cb into main Jul 6, 2026
12 checks passed
@KaloyanTanev KaloyanTanev deleted the kalo/quic-false-error branch July 6, 2026 17:42
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.

3 participants