Skip to content

feat(graphql): block retry if connection not established#854

Open
stdevi wants to merge 1 commit into
masterfrom
stdevi/graphql-connection-backoff
Open

feat(graphql): block retry if connection not established#854
stdevi wants to merge 1 commit into
masterfrom
stdevi/graphql-connection-backoff

Conversation

@stdevi

@stdevi stdevi commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Back off the GraphQL WebSocket reconnect loop when the socket repeatedly fails to establish, so the SDK stops minting a new temporary API key on every retry.

When a client's firewall blocks wss:// (but not HTTPS), every reconnect attempt fetched a fresh temporary API key over HTTPS and then failed the WebSocket handshake forever, ~1 key/minute per device.

So now we:

  • Count consecutive closes that happen before the connection is established (consecutiveFailedHandshakes).
  • After MAX_FAILED_HANDSHAKES (5), the existing reconnect loop backs off to a flat BLOCKED_RETRY_INTERVAL_MS (5 min) instead of the fast exponential backoff.
  • Reset the counter on GQL_CONNECTION_ACK, so it recovers automatically once the connection succeeds.

How to test

Set up sign-in-wrapper with javascript-api and ipad-sig-in on master first.

Navigate to name.local and set up iPad.

Go to uBlock Origin settings:
image

Go to My filters and disable websockets:

*$websocket

image

You will see how iPad created keys.

Then switch to this PR for javascript-api. The backoff will be applied.

@stdevi stdevi self-assigned this Jun 5, 2026
@stdevi stdevi marked this pull request as ready for review June 5, 2026 17:07
@stdevi stdevi requested review from 2ndalpha and raluik June 5, 2026 17:07

@raluik raluik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM 🚀

// Each cycle: connect, never ACK, close before the connection is established.
for (let i = 0; i < MAX_FAILED_HANDSHAKES; i++) {
await fixture.waitForConnection();
await fixture.getNextMessage(); // consume connection_init

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

polish: we have consumeInitMessage. Then there's no need for the comment.

await fixture.closeWithCode(1001);
fixture.openServer();
}
await new Promise((resolve) => setTimeout(resolve, 20));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

polish: seems to be unnecessary.

const timer = calculateRandomizedExponentialBackoffTime(
this.connectionAttemptsCount,
if (closedBeforeEstablished) {
this.consecutiveFailedHandshakes++;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion(non-blocking): could we possibly gain sth by logging this?

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.

2 participants