Skip to content

fix(client): back off reconnects when the stream never connects - #47204

Closed
holny wants to merge 1 commit into
anomalyco:v2from
holny:auth-401-reconnect
Closed

holny wants to merge 1 commit into
anomalyco:v2from
holny:auth-401-reconnect

Conversation

@holny

@holny holny commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Issue for this PR

Fixes #47062

Type of change

  • Bug fix
  • New feature
  • Refactor / code improvement
  • Documentation

What does this PR do?

The event-stream client retried on a fixed one-second delay regardless of why the stream died, so an unauthenticated browser session re-triggered the Basic auth prompt every second while the shell sat there unauthenticated. Streams that never connected successfully now scale the retry delay with the attempt count up to 30 seconds, which gives the browser prompt time to be answered; streams that connected and later dropped reset the attempt counter and keep the existing one-second retry.

The failure reason is intentionally not inspected — a declared 401 with an empty body surfaces client-side as an indistinguishable ClientError("UnsupportedContentType"), and the never-connected case is the one that needs the guard regardless of cause.

How did you verify your code works?

  • New test/solid-connection.test.ts: a 401-empty-response API under a real clock — reconnect attempts stay monotonically increasing across 400ms and the inter-attempt gaps grow (no backoff would produce ~20 attempts, backoff produces ~7)
  • reconnectBackoffDelay unit cases for scaling, the 30s cap, and the attempt-0 floor
  • bun typecheck + full packages/client suite (152 passing)

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@holny

holny commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Fixes #47062 — the linked-issue check doesn't resolve the keyword from a fork PR, so referencing it here directly.

The event-stream client retried on a fixed one-second delay no matter why the stream died, so an unauthenticated browser session re-triggered the Basic auth prompt every second. Streams that never connected successfully now scale the delay with the attempt count up to 30 seconds; streams that connected and later dropped keep the existing one-second retry.
@holny
holny force-pushed the auth-401-reconnect branch from b4e64bf to 309fec8 Compare September 6, 2026 10:12
@holny

holny commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto the latest v2 — the connection layer was rewritten upstream in the meantime, so the backoff now sits on top of the new lifecycle/idle-timeout structure. Behavior is unchanged: streams that never connect successfully scale their retry delay up to 30s; streams that connected and dropped keep the 1s retry. Tests merged with the new upstream coverage (162 passing).

@holny

holny commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Following up after the rebase — no checks have run on the new head, so the workflows may need a maintainer approval to kick off. Behavior is unchanged from the original backoff fix (capped retry scaling for never-connected streams, 1s retry kept for post-connect drops). Happy to rework anything if review turns something up.

@holny holny closed this by deleting the head repository Sep 9, 2026
@pedro757

pedro757 commented Sep 9, 2026 •

Copy link
Copy Markdown

I think you unintentionally closed this PR @holny

@holny

holny commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #48161 - this PR's head fork was accidentally deleted on 2026-09-09 (not intentional); re-filed so the review can continue. Sorry for the noise.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants