Skip to content

fix(account-pool): recover failed requests before streaming - #4

Closed
smsunarto wants to merge 2 commits into
scott/pool-refresh-recoveryfrom
scott/pool-request-recovery
Closed

fix(account-pool): recover failed requests before streaming#4
smsunarto wants to merge 2 commits into
scott/pool-refresh-recoveryfrom
scott/pool-request-recovery

Conversation

@smsunarto

@smsunarto smsunarto commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Superseded by get-bb#3116 in the correct upstream repository. Closing this misplaced PR.

Human comments

What was wrong

The hub returned an account's 401 or a connection error immediately, even when another account was healthy. It never refreshed a rejected OAuth access token. Its separate short-429 retry path also bypassed recovery and could send another request after cancellation.

What changed

Use one recovery loop over the initial account set. Each account gets at most one OAuth 401 recovery and one short rate-limit retry. Concurrent requests share forced refresh and reuse credentials already replaced by another request. A rejected token cannot fall back during a refresh outage. Credential failures and retryable upstream failures can move to another eligible account.

Discard failed responses before retry, bound error diagnostics, and stop additional inference on cancellation or hub shutdown. Recovery ends when the response is handed to the client. Connection failures can occur after upstream work starts, so retries do not guarantee exactly-once generation.

This layer builds on OAuth refresh recovery. Updated the plugin overview and CLI guide.

How you verified

The committed public HTTP regressions failed before the fix: 28 failed and the existing stream no-replay invariant passed. Cases cover both providers, concurrent and late 401s, retry budgets, rejected-token cooldown, a healthy fourth account, a fixed candidate set, and cancellation during pacing.

  • Full plugin Turbo suite: 130 tests passed across 8 files.
  • Turbo typecheck, explicit-file formatting, and oxlint passed.
  • Additional tests cover bounded failed-response disposal, cancellation of shared-refresh waiters, late auth responses, terminal refresh rejection, service restart, and graceful drain before headers.
  • Original actual-hub probe now returns 200 from account B after account A returns 401. Separate probes verify one refresh after terminal rejection and successful restart and drain.

Summary by cubic

Fixes request recovery in the account pool hub so one account's 401 or connection failure no longer aborts a request when another account is healthy. The hub now runs a single recovery loop over the initial account set and can move authentication and retryable upstream failures to another eligible account before a response reaches the client.

Bug Fixes

  • Each account now gets one OAuth 401 refresh and one short rate-limit retry per request.
  • Concurrent requests share a forced refresh and reuse credentials another request already replaced.
  • A rejected token stays rejected during a refresh outage instead of falling back.
  • The short-429 retry path now uses the same recovery loop instead of bypassing it.
  • Failed responses are disposed before retry, error detail is bounded, and OAuth refreshes that ignore abort time out.
  • Recovery ends when the response is handed to the client; cancellation or hub shutdown stops further inference.
  • Retries do not guarantee exactly-once generation since connection failures can occur after upstream work starts.
  • Updated the plugin overview and CLI guide.

Written for commit a72ef3d. Summary will update on new commits.

Review in cubic

Stack

Review and merge in order:

  1. Serialize credential refreshes
  2. Recover transient OAuth failures
  3. Recover requests before streaming
  4. Preserve session affinity

All PRs target get-bb/bb:main. The source branches remain stacked in the fork. GitHub diffs are currently cumulative because upstream access is read-only, so the parent branches cannot be published there.

AGENT GENERATED

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 8 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="plugins/account-pool/src/provider-adapter.ts">

<violation number="1" location="plugins/account-pool/src/provider-adapter.ts:139">
P2: When the token endpoint sends headers but leaves its body open and the injected fetch ignores abort, this timeout rejects the race without releasing the response stream. The refresh fails, but `response.text()` and the underlying connection can remain active indefinitely; read the body through a cancellable reader or otherwise explicitly abort/cancel the in-flight body on timeout.</violation>
</file>

<file name="plugins/account-pool/src/hub.ts">

<violation number="1" location="plugins/account-pool/src/hub.ts:339">
P2: When account preparation or header generation throws, this branch escapes `forward` and prevents fallback to another eligible account. Treat non-connection adapter errors as account failures (and mark the account) before continuing the recovery loop.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

throw new Error(message);
}
try {
return await Promise.race([response.text(), timedOut]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When the token endpoint sends headers but leaves its body open and the injected fetch ignores abort, this timeout rejects the race without releasing the response stream. The refresh fails, but response.text() and the underlying connection can remain active indefinitely; read the body through a cancellable reader or otherwise explicitly abort/cancel the in-flight body on timeout.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/account-pool/src/provider-adapter.ts, line 139:

<comment>When the token endpoint sends headers but leaves its body open and the injected fetch ignores abort, this timeout rejects the race without releasing the response stream. The refresh fails, but `response.text()` and the underlying connection can remain active indefinitely; read the body through a cancellable reader or otherwise explicitly abort/cancel the in-flight body on timeout.</comment>

<file context>
@@ -84,47 +85,66 @@ export async function fetchOAuthRefresh(
+      throw new Error(message);
+    }
+    try {
+      return await Promise.race([response.text(), timedOut]);
+    } catch {
+      throw new TransientOAuthRefreshError(
</file context>

family,
} catch (error) {
signal.throwIfAborted();
if (!(error instanceof UpstreamConnectionError)) throw error;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: When account preparation or header generation throws, this branch escapes forward and prevents fallback to another eligible account. Treat non-connection adapter errors as account failures (and mark the account) before continuing the recovery loop.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At plugins/account-pool/src/hub.ts, line 339:

<comment>When account preparation or header generation throws, this branch escapes `forward` and prevents fallback to another eligible account. Treat non-connection adapter errors as account failures (and mark the account) before continuing the recovery loop.</comment>

<file context>
@@ -257,148 +278,293 @@ export class AccountPoolHub {
-              family,
+          } catch (error) {
+            signal.throwIfAborted();
+            if (!(error instanceof UpstreamConnectionError)) throw error;
+            failure = {
+              status: 502,
</file context>

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.

1 participant