Skip to content

Commit 73aba32

Browse files
author
Stanislav Deviatykh
committed
feat(graphql): block retry if connection not established
1 parent d2e27ba commit 73aba32

2 files changed

Lines changed: 130 additions & 9 deletions

File tree

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
import { WebSocket } from 'mock-socket';
2+
3+
import { GraphQLSubscriptionsFixture } from '../__fixtures__/graphql-subscriptions-fixture';
4+
import * as backoff from '../../../util/randomized-exponential-backoff/randomized-exponential-backoff';
5+
6+
jest.mock('isomorphic-ws', () => WebSocket);
7+
jest.mock('../../../util/sleep-ms/sleep-ms', () => ({
8+
sleepMs: () => new Promise((resolve) => setTimeout(resolve, 4)),
9+
}));
10+
11+
// Mirrors the constants in graphql.service.ts (not exported).
12+
const MAX_FAILED_HANDSHAKES = 5;
13+
const BLOCKED_RETRY_INTERVAL_MS = 5 * 60 * 1000;
14+
15+
describe('GraphQL blocked-WebSocket back-off', () => {
16+
let fixture: GraphQLSubscriptionsFixture;
17+
18+
beforeEach(() => {
19+
fixture = new GraphQLSubscriptionsFixture();
20+
});
21+
22+
afterEach(async () => {
23+
await fixture.cleanup();
24+
});
25+
26+
it('should back off to a slow interval after repeated handshakes fail before connecting', async () => {
27+
const service = fixture.graphqlService as any;
28+
const backoffSpy = jest.spyOn(
29+
backoff,
30+
'calculateRandomizedExponentialBackoffTime',
31+
);
32+
const infoSpy = jest.spyOn(service.logger, 'info');
33+
34+
const sub = fixture.triggerSubscription();
35+
36+
// Each cycle: connect, never ACK, close before the connection is established.
37+
for (let i = 0; i < MAX_FAILED_HANDSHAKES; i++) {
38+
await fixture.waitForConnection();
39+
await fixture.getNextMessage(); // consume connection_init
40+
await fixture.closeWithCode(1001);
41+
fixture.openServer();
42+
}
43+
await new Promise((resolve) => setTimeout(resolve, 20));
44+
45+
expect(service.consecutiveFailedHandshakes).toBeGreaterThanOrEqual(
46+
MAX_FAILED_HANDSHAKES,
47+
);
48+
// Fast exponential backoff is used only for the first (MAX - 1) reconnects;
49+
// once the cap is hit, the flat slow interval is used instead.
50+
expect(backoffSpy).toHaveBeenCalledTimes(MAX_FAILED_HANDSHAKES - 1);
51+
expect(infoSpy).toHaveBeenCalledWith(
52+
`Reconnect socket in ${BLOCKED_RETRY_INTERVAL_MS}ms`,
53+
);
54+
55+
sub.unsubscribe();
56+
});
57+
58+
it('should not count a close that happens after a successful connection', async () => {
59+
const service = fixture.graphqlService as any;
60+
61+
const sub = fixture.triggerSubscription();
62+
await fixture.handleConnectionInit(); // connection_ack -> CONNECTED
63+
await fixture.consumeSubscribeMessage();
64+
65+
expect(service.consecutiveFailedHandshakes).toBe(0);
66+
67+
// A drop after the connection was established is a normal reconnect.
68+
await fixture.closeWithCode(1001);
69+
expect(service.consecutiveFailedHandshakes).toBe(0);
70+
71+
fixture.openServer();
72+
await fixture.handleConnectionInit();
73+
sub.unsubscribe();
74+
});
75+
76+
it('should reset the counter when the connection is established', async () => {
77+
const service = fixture.graphqlService as any;
78+
79+
const sub = fixture.triggerSubscription();
80+
81+
for (let i = 0; i < 2; i++) {
82+
await fixture.waitForConnection();
83+
await fixture.getNextMessage();
84+
await fixture.closeWithCode(1001);
85+
fixture.openServer();
86+
}
87+
expect(service.consecutiveFailedHandshakes).toBe(2);
88+
89+
// A successful connection_ack resets the counter back to fast reconnects.
90+
await fixture.handleConnectionInit();
91+
await fixture.consumeSubscribeMessage();
92+
expect(service.consecutiveFailedHandshakes).toBe(0);
93+
94+
sub.unsubscribe();
95+
});
96+
});

‎packages/javascript-api/src/lib/services/graphql/graphql.service.ts‎

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,12 @@ const PING_PONG_INTERVAL_IN_MS = 20_000;
9898
// https://www.w3.org/TR/websockets/#concept-websocket-close-fail
9999
const CLIENT_SIDE_CLOSE_EVENT = 1000;
100100

101+
// Once the WebSocket has failed to establish this many times in a row, the reconnect
102+
// loop backs off to a slow interval instead of minting a new temporary API key on
103+
// every fast retry. A successful connection resets the counter.
104+
const MAX_FAILED_HANDSHAKES = 5;
105+
const BLOCKED_RETRY_INTERVAL_MS = 5 * 60 * 1000;
106+
101107
/**
102108
* A service that lets the user query Qminder API via GraphQL statements.
103109
* Queries and subscriptions are supported. There is no support for mutations.
@@ -192,6 +198,10 @@ export class GraphqlService {
192198

193199
private connectionAttemptsCount = 0;
194200

201+
// Consecutive WebSocket closes that happened before the connection was ever
202+
// established. Reset to 0 on a successful GQL_CONNECTION_ACK.
203+
private consecutiveFailedHandshakes = 0;
204+
195205
constructor() {
196206
this.setServer('api.qminder.com');
197207

@@ -446,14 +456,32 @@ export class GraphqlService {
446456
reason: event.reason,
447457
});
448458

459+
// Capture this before the status is overwritten below: a close while still
460+
// CONNECTING means the connection never established.
461+
const closedBeforeEstablished =
462+
this.connectionStatus === ConnectionStatus.CONNECTING;
463+
449464
this.setConnectionStatus(ConnectionStatus.DISCONNECTED);
450465
this.socket = null;
451466
this.clearPingMonitoring();
452467

453-
if (this.shouldRetry(event)) {
454-
const timer = calculateRandomizedExponentialBackoffTime(
455-
this.connectionAttemptsCount,
468+
if (closedBeforeEstablished) {
469+
this.consecutiveFailedHandshakes++;
470+
this.logger.error(
471+
`Received socket close event before a connection was established! Close code: ${event.code}`,
456472
);
473+
}
474+
475+
if (this.shouldRetry(event)) {
476+
// After repeated failures to establish, back off to a slow interval so we
477+
// stop creating a temporary API key on every retry. Recovers on its own:
478+
// a successful connection resets consecutiveFailedHandshakes.
479+
const timer =
480+
this.consecutiveFailedHandshakes >= MAX_FAILED_HANDSHAKES
481+
? BLOCKED_RETRY_INTERVAL_MS
482+
: calculateRandomizedExponentialBackoffTime(
483+
this.connectionAttemptsCount,
484+
);
457485

458486
this.logger.info(`Reconnect socket in ${timer.toFixed(0)}ms`);
459487

@@ -466,12 +494,6 @@ export class GraphqlService {
466494
this.logger.error('Failed to reconnect socket: ', error);
467495
});
468496
}
469-
470-
if (this.connectionStatus === ConnectionStatus.CONNECTING) {
471-
this.logger.error(
472-
`Received socket close event before a connection was established! Close code: ${event.code}`,
473-
);
474-
}
475497
};
476498

477499
this.socket.onerror = () => {
@@ -498,6 +520,9 @@ export class GraphqlService {
498520
this.retryableErroredSubscriptionsRetryCount = 0;
499521
this.retryableErroredSubscriptionsAction$.next({ type: 'clear' });
500522

523+
// Connection established — clear the blocked back-off.
524+
this.consecutiveFailedHandshakes = 0;
525+
501526
this.setConnectionStatus(ConnectionStatus.CONNECTED);
502527
this.logger.info('Connected to websocket');
503528
this.startConnectionMonitoring();

0 commit comments

Comments
 (0)