Skip to content

fix(network): gracefully handle expired peer in peer_connection_attempt#15390

Merged
darioush merged 2 commits into
near:masterfrom
darioush:log-003
Mar 17, 2026
Merged

fix(network): gracefully handle expired peer in peer_connection_attempt#15390
darioush merged 2 commits into
near:masterfrom
darioush:log-003

Conversation

@darioush

Copy link
Copy Markdown
Contributor
  • During sync, outbound connection attempts can take a long time (e.g., TCP timeout to unreachable peers). Meanwhile, remove_expired can delete the peer from the store. When the connection attempt finally completes with a failure, peer_connection_attempt would bail with an error logged at ERROR level.
  • Instead, if the connection failed and the peer is gone from the store, return Ok silently (with a debug log). We were only going to mark the peer as Unknown, and it's already deleted.
  • Keep the bail for the unexpected case where the connection succeeded but the peer is missing.

@codecov

codecov Bot commented Mar 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.10%. Comparing base (62db14e) to head (7497b7e).
⚠️ Report is 9 commits behind head on master.

Files with missing lines Patch % Lines
chain/network/src/peer_manager/peer_store/mod.rs 72.72% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #15390      +/-   ##
==========================================
+ Coverage   69.00%   69.10%   +0.09%     
==========================================
  Files         934      934              
  Lines      209206   209728     +522     
  Branches   209206   209728     +522     
==========================================
+ Hits       144360   144923     +563     
+ Misses      58861    58814      -47     
- Partials     5985     5991       +6     
Flag Coverage Δ
pytests-nightly 1.16% <0.00%> (-0.01%) ⬇️
unittests 68.33% <82.35%> (+0.07%) ⬆️
unittests-nightly 68.52% <82.35%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@darioush darioush requested a review from shreyan-gupta March 17, 2026 13:51
@darioush darioush marked this pull request as ready for review March 17, 2026 13:52
@darioush darioush requested a review from a team as a code owner March 17, 2026 13:52
Copilot AI review requested due to automatic review settings March 17, 2026 13:52

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

Adjusts PeerStore::peer_connection_attempt to avoid emitting errors when an outbound connection attempt fails after the peer has already been removed (e.g., expired) from the peer store—reducing log noise during sync while preserving an error for unexpected success cases.

Changes:

  • Treat “peer missing + connection failed” as a benign race: log at DEBUG and return Ok(()).
  • Preserve the existing error (bail!) when “peer missing + connection succeeded”.
  • Refactor the function to use a let Some(..) = .. else { .. } guard for clearer control flow.

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

@darioush darioush added this pull request to the merge queue Mar 17, 2026
Merged via the queue into near:master with commit fd43055 Mar 17, 2026
32 checks passed
@darioush darioush deleted the log-003 branch March 17, 2026 20:39
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