Skip to content

fix(go): complete TCP client teardown when conn.Close fails#3670

Merged
spetz merged 2 commits into
apache:masterfrom
chengxilo:fix-go-tcp-teardown
Jul 14, 2026
Merged

fix(go): complete TCP client teardown when conn.Close fails#3670
spetz merged 2 commits into
apache:masterfrom
chengxilo:fix-go-tcp-teardown

Conversation

@chengxilo

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

Relates to #3652 (comment)
Relates to #3621 (comment)

Rationale

Golang SDK TCP client return the error directly when c.conn.Close() return an error. This stop it from finishing the disconnect and shutdown process.

What changed?

  1. instead of return err directly, we keep the error, finish the process of shutdown/disconnect, return the error
  2. when close the connection, set the c.conn to nil
  3. In disconnect method, now we return nil directly when the status is shutdown too.(or caller can use disconnect->connect to revive a client that was shutdown before)

Local Execution

  • Passed
  • Pre-commit hooks ran

AI Usage

  1. Claude Opus 4.8
  2. implementaiton
  3. Reviewed it
  4. yes

@github-actions

Copy link
Copy Markdown

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

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

See CONTRIBUTING.md for details.

@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.50%. Comparing base (02c5e4f) to head (5d4d5a3).

Files with missing lines Patch % Lines
foreign/go/client/tcp/tcp_core.go 66.66% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3670      +/-   ##
============================================
- Coverage     73.59%   73.50%   -0.09%     
  Complexity      937      937              
============================================
  Files          1297     1291       -6     
  Lines        143769   141555    -2214     
  Branches     119330   117114    -2216     
============================================
- Hits         105811   104056    -1755     
+ Misses        34591    34276     -315     
+ Partials       3367     3223     -144     
Components Coverage Δ
Rust Core 73.71% <ø> (-0.14%) ⬇️
Java SDK 62.44% <ø> (ø)
C# SDK 72.21% <ø> (+0.01%) ⬆️
Python SDK 92.17% <ø> (ø)
PHP SDK 84.29% <ø> (ø)
Node SDK 91.35% <ø> (+0.12%) ⬆️
Go SDK 42.71% <66.66%> (+0.43%) ⬆️
Files with missing lines Coverage Δ
foreign/go/client/tcp/tcp_core.go 57.14% <66.66%> (+6.09%) ⬆️

... and 23 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.

@spetz
spetz merged commit b7c87d8 into apache:master Jul 14, 2026
52 checks passed
@github-actions github-actions Bot removed the S-waiting-on-review PR is waiting on a reviewer label Jul 14, 2026
@chengxilo
chengxilo deleted the fix-go-tcp-teardown branch July 14, 2026 18:05
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.

4 participants