Skip to content

fix(go): tear down only the connection a request failed on - #3986

Open
hubcio wants to merge 1 commit into
masterfrom
fix/go-sdk-stale-reconnect-teardown
Open

fix(go): tear down only the connection a request failed on#3986
hubcio wants to merge 1 commit into
masterfrom
fix/go-sdk-stale-reconnect-teardown

Conversation

@hubcio

@hubcio hubcio commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Two requests failing at once are one reconnect, but the
teardown that precedes it went by transport state alone.
Connect marks the client connected before it signs in, so
the second caller -- parked on c.mtx for the length of that
sign-in -- woke to a healthy-looking state and closed the
socket the first caller's reconnect had just established.
That left the first caller replaying over nothing, failing
it with ErrNotConnected, and started a second attempt for a
reconnect that had already happened.

Stamp every installed connection with a generation and carry
it out of the attempt that ran on it, so the teardown is a
no-op once the connection has been replaced. The leader
redirect took the same stale teardown and now drops a
redirect decided on a connection that is gone.

Surfaced as a flaky
TestConnect_ConcurrentReconnectsThroughExchangeShareOneAttempt.

Two requests failing at once are one reconnect, but the
teardown that precedes it went by transport state alone.
Connect marks the client connected before it signs in, so
the second caller -- parked on c.mtx for the length of that
sign-in -- woke to a healthy-looking state and closed the
socket the first caller's reconnect had just established.
That left the first caller replaying over nothing, failing
it with ErrNotConnected, and started a second attempt for a
reconnect that had already happened.

Stamp every installed connection with a generation and carry
it out of the attempt that ran on it, so the teardown is a
no-op once the connection has been replaced. The leader
redirect took the same stale teardown and now drops a
redirect decided on a connection that is gone.

Surfaced as a flaky
TestConnect_ConcurrentReconnectsThroughExchangeShareOneAttempt.
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 28, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.67347% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.26%. Comparing base (0fdf9d6) to head (1ac46f1).

Files with missing lines Patch % Lines
foreign/go/client/tcp/tcp_core.go 88.23% 3 Missing and 1 partial ⚠️
foreign/go/client/tcp/tcp_session_management.go 73.33% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3986   +/-   ##
=========================================
  Coverage     84.26%   84.26%           
  Complexity     1398     1398           
=========================================
  Files          1219     1219           
  Lines        173644   173670   +26     
  Branches     140742   140742           
=========================================
+ Hits         146317   146340   +23     
- Misses        23369    23371    +2     
- Partials       3958     3959    +1     
Components Coverage Δ
Rust Core 85.08% <ø> (ø)
Java SDK 67.23% <ø> (ø)
C# SDK 75.39% <ø> (ø)
Python SDK 90.06% <ø> (ø)
PHP SDK 85.65% <ø> (ø)
Node SDK 96.07% <ø> (ø)
Go SDK 69.17% <83.67%> (+0.07%) ⬆️
Files with missing lines Coverage Δ
foreign/go/client/tcp/tcp_core.go 87.42% <88.23%> (+0.26%) ⬆️
foreign/go/client/tcp/tcp_session_management.go 85.62% <73.33%> (-0.87%) ⬇️
🚀 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.

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.

2 participants