[PR #5750] [MERGED] [client] Fix/grpc retry #23793

Closed
opened 2026-08-05 06:07:51 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/5750
Author: @pappz
Created: 3/31/2026
Status: Merged
Merged: 4/13/2026
Merged by: @pappz

Base: mainHead: fix/grpc-retry


📝 Commits (10+)

  • 13dfc5f [client] Fix flow client Receive retry loop not stopping after Close
  • fae5052 [client] remove WaitForReady from stream open call
  • 863f1d8 [client] Add connection recreation and improve flow client error handling
  • a178ec7 [client] Remove Unauthenticated, PermissionDenied, and Unimplemented from permanent error handling
  • 8ba4183 [client] Fix error handling in Receive to properly re-establish stream and improve reconnection messaging
  • bb9ead4 Fix test
  • 11e9c05 [client] Add graceful shutdown handling and test for concurrent Close during Receive
  • e9007f7 [client] Fix connection swap to properly close old gRPC connection
  • 03f023b [client] Reset backoff
  • 62a6dc9 [client] Ensure stream closure on error during initialization

📊 Changes

2 files changed (+450 additions, -53 deletions)

View changed files

📝 flow/client/client.go (+154 -37)
📝 flow/client/client_test.go (+296 -16)

📄 Description

Describe your changes

  • Fix Receive stuck after Close: Use backoff.Permanent for codes.Canceled errors so Receive() returns immediately when the connection is closed, instead of retrying for up to 3 months
  • Add connection recreation on corrupt stream: Store gRPC dial options on the client and recreate the connection on codes.Internal errors (RST_STREAM/PROTOCOL_ERROR), with logging for visibility
  • Remove WaitForReady: Let the backoff loop handle retry timing instead of blocking inside gRPC
  • Unify mutex: Replace streamMu with a single mu that protects clientConn, realClient, and stream
  • Fix back-off reset: After success connection reset the backoff. Without it after 3 month this logic expire

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • New Features

    • Added a public Send API, preserved connection options for automatic reconnection, and introduced an explicit client-closed error.
  • Bug Fixes

    • Close now reliably cancels in-flight work and detaches/closes connections without deadlock.
    • Prevent concurrent receives; receive/setup fail fast on initiation/header errors and treat cancellations as permanent.
    • Improved reconnection and backoff behavior (health-window reset, reduced jitter) to avoid spurious retries.
  • Tests

    • Added concurrency, shutdown-during-receive, and protocol-level stream-reset reconnection tests.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/5750 **Author:** [@pappz](https://github.com/pappz) **Created:** 3/31/2026 **Status:** ✅ Merged **Merged:** 4/13/2026 **Merged by:** [@pappz](https://github.com/pappz) **Base:** `main` ← **Head:** `fix/grpc-retry` --- ### 📝 Commits (10+) - [`13dfc5f`](https://github.com/netbirdio/netbird/commit/13dfc5fcdd0c1afefd99ff1e3dbb8417aa338c22) [client] Fix flow client Receive retry loop not stopping after Close - [`fae5052`](https://github.com/netbirdio/netbird/commit/fae50520ff82939afafbdb49f7d462a60483223e) [client] remove WaitForReady from stream open call - [`863f1d8`](https://github.com/netbirdio/netbird/commit/863f1d88b8a21cab5829052a448923aa6e481d22) [client] Add connection recreation and improve flow client error handling - [`a178ec7`](https://github.com/netbirdio/netbird/commit/a178ec785d2ba01ced706a95f4e6a97a7ac57435) [client] Remove Unauthenticated, PermissionDenied, and Unimplemented from permanent error handling - [`8ba4183`](https://github.com/netbirdio/netbird/commit/8ba4183acd75ed4f3e79342a20bf2a8745a9efcc) [client] Fix error handling in Receive to properly re-establish stream and improve reconnection messaging - [`bb9ead4`](https://github.com/netbirdio/netbird/commit/bb9ead4e62374a951811169b957abb11437f4aa4) Fix test - [`11e9c05`](https://github.com/netbirdio/netbird/commit/11e9c052b4d1fb977cfd69eda0ca09708c08c0cf) [client] Add graceful shutdown handling and test for concurrent Close during Receive - [`e9007f7`](https://github.com/netbirdio/netbird/commit/e9007f7191972fd773f2c52ee207c7b62ea86e9d) [client] Fix connection swap to properly close old gRPC connection - [`03f023b`](https://github.com/netbirdio/netbird/commit/03f023b00aa818d1579ed78e154fdf0aacfcea16) [client] Reset backoff - [`62a6dc9`](https://github.com/netbirdio/netbird/commit/62a6dc938655091e9609af0932a35d2fa75282ff) [client] Ensure stream closure on error during initialization ### 📊 Changes **2 files changed** (+450 additions, -53 deletions) <details> <summary>View changed files</summary> 📝 `flow/client/client.go` (+154 -37) 📝 `flow/client/client_test.go` (+296 -16) </details> ### 📄 Description ## Describe your changes - **Fix Receive stuck after Close**: Use `backoff.Permanent` for `codes.Canceled` errors so `Receive()` returns immediately when the connection is closed, instead of retrying for up to 3 months - **Add connection recreation on corrupt stream**: Store gRPC dial options on the client and recreate the connection on `codes.Internal` errors (RST_STREAM/PROTOCOL_ERROR), with logging for visibility - **Remove `WaitForReady`**: Let the backoff loop handle retry timing instead of blocking inside gRPC - **Unify mutex**: Replace `streamMu` with a single `mu` that protects `clientConn`, `realClient`, and `stream` - **Fix back-off reset**: After success connection reset the backoff. Without it after 3 month this logic expire ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [x] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added a public Send API, preserved connection options for automatic reconnection, and introduced an explicit client-closed error. * **Bug Fixes** * Close now reliably cancels in-flight work and detaches/closes connections without deadlock. * Prevent concurrent receives; receive/setup fail fast on initiation/header errors and treat cancellations as permanent. * Improved reconnection and backoff behavior (health-window reset, reduced jitter) to avoid spurious retries. * **Tests** * Added concurrency, shutdown-during-receive, and protocol-level stream-reset reconnection tests. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 06:07:51 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#23793