Skip to content

fix(sdk): Restart the links on keepalive after a cancelled connect - #4782

Open
TomPlanche wants to merge 1 commit into
apache:masterfrom
TomPlanche:fix/sdk-keepalive-after-cancelled-connect
Open

TomPlanche wants to merge 1 commit into
apache:masterfrom
TomPlanche:fix/sdk-keepalive-after-cancelled-connect

Conversation

@TomPlanche

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

Closes #4638

Rationale

A client that loses the caller of its connect must still come back, like a client that is Disconnected. Without this fix, only an explicit connect() recovers it, and nothing in the SDK calls one.

What changed?

When a caller dropped a connect() while the reconnect sweep waited between two passes, the client stayed in Connecting with no slots. keepalive() restarts the links only for Disconnected or for a slot without a task, so the heartbeat never restarted them, and a request never dials.

Now keepalive() also restarts the links when the abandoned flag of the ConnectCoordinator is set, as @numinnex suggested in the review of #4569. The new connect() sees the flag, and its sweep disconnects the half-done state before it sweeps again. The keepalive_restarts() guard does not change, so an explicit disconnect or a logout stays final. The test from the issue reproduces the bug and now passes.

The cancel inside resume_or_sign_in, after install_bootstrap, is a different path and is not part of this PR.

Local Execution

  • Passed: cargo fmt --all and cargo clippy -p iggy --all-features --all-targets -- -D warnings.

  • Passed: the test from the issue, which failed on master with left: Connecting:

    $ cargo test -p iggy --lib -- given_a_sweep_owner_cancelled
    test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 760 filtered out
    
  • Passed: cargo test -p iggy --lib, with 761 tests.

  • Passed: pre-commit hooks with prek run.

AI Usage

  1. Claude Code (Claude Opus 5.5) checked that no PRs opened/drafted was fixing this.

@github-actions

github-actions Bot commented Nov 8, 2026

Copy link
Copy Markdown

Thanks for the PR. It is labeled S-waiting-on-review and queued for review.

If AI assistance was used to create this PR, you are required to run the team-review skill before opening it.

Slash commands (own line, regular comment) move it around the queue:

  • /ready - back to S-waiting-on-review after addressing feedback
  • /author - flip to S-waiting-on-author while you finish changes
  • /request-review @user-or-team - request a reviewer
  • /pin - exempt the PR from the stale bot, /unpin to undo

See CONTRIBUTING.md for details.

@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Nov 8, 2026
@codecov

codecov Bot commented Nov 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.81%. Comparing base (689436f) to head (f0cdbdb).

❌ Your patch check has failed because the patch coverage (0.00%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@              Coverage Diff              @@
##             master    #4782       +/-   ##
=============================================
- Coverage     90.89%   84.81%    -6.09%     
+ Complexity     3878     3876        -2     
=============================================
  Files          1564     1504       -60     
  Lines        407450   297379   -110071     
  Branches     341308   231237   -110071     
=============================================
- Hits         370368   252229   -118139     
- Misses        29706    37402     +7696     
- Partials       7376     7748      +372     
Components Coverage Δ
Rust Core 84.63% <100.00%> (-7.73%) ⬇️
Java SDK 79.38% <ø> (ø)
C# SDK 84.37% <ø> (+5.98%) ⬆️
Python SDK 86.52% <ø> (ø)
PHP SDK 92.23% <ø> (ø)
Node SDK 96.75% <ø> (ø)
Go SDK 71.93% <ø> (+0.12%) ⬆️
C++ SDK 86.84% <ø> (ø)
Files with missing lines Coverage Δ
core/sdk/src/links/connect.rs 94.16% <100.00%> (+0.06%) ⬆️
core/sdk/src/links/replicas.rs 96.73% <100.00%> (+0.15%) ⬆️

... and 467 files with indirect coverage changes

🚀 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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review PR is waiting on a reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rust SDK: a connect cancelled in the reconnect pause leaves the client in Connecting

1 participant