From d81a184b4bcff7e190778eb9ce616e0bae8c19de Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 11:15:21 +0300 Subject: [PATCH 01/35] WIP --- .../graphql-subscriptions-fixture.ts | 9 +- .../__tests__/graphql-subscriptions.spec.ts | 210 ++++++++-- .../lib/services/graphql/graphql.service.ts | 370 +++++++++++------- 3 files changed, 402 insertions(+), 187 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts index ded2e3e7..4534ac39 100644 --- a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts +++ b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts @@ -40,12 +40,12 @@ export class GraphQLSubscriptionsFixture { return (this.graphqlService as any).subscriptions.length; } - getGraphqlServiceSubscriptionObserverMapSize(): number { - return Object.keys(this.getGraphqlServiceSubscriptionObserverMap()).length; + getMessageSubscribersSize(): number { + return (this.graphqlService as any).messageSubscribers.size; } - getGraphqlServiceSubscriptionObserverMap(): Record> { - return (this.graphqlService as any).subscriptionObserverMap; + hasMessageSubscriber(id: string): boolean { + return (this.graphqlService as any).messageSubscribers.has(id); } async waitForConnection() { @@ -125,6 +125,7 @@ export class GraphQLSubscriptionsFixture { } async cleanup() { + (this.graphqlService as any).clearSubscriptionRetry(); WS.clean(); await this.server.closed; } diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 88ed8840..22c16641 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -1,10 +1,8 @@ import gql from 'graphql-tag'; import fetchMock from 'jest-fetch-mock'; import { WebSocket } from 'mock-socket'; -import { Subscriber } from 'rxjs'; import { ConnectionStatus } from '../../../model/connection-status'; import { GraphQLSubscriptionsFixture } from '../__fixtures__/graphql-subscriptions-fixture'; -import { QminderGraphQLError } from '../graphql.service'; jest.mock('isomorphic-ws', () => WebSocket); jest.mock('../../../util/sleep-ms/sleep-ms', () => ({ @@ -96,24 +94,20 @@ describe('GraphQL subscriptions', () => { }); it('cleans up internal state when unsubscribing', async () => { - // start the test with an empty observer-map - expect(fixture.getGraphqlServiceSubscriptionObserverMapSize()).toBe(0); + expect(fixture.getMessageSubscribersSize()).toBe(0); const subscription = fixture.triggerSubscription(); await fixture.handleConnectionInit(); await fixture.consumeSubscribeMessage(); - // the observer map should equal { "1": Subscriber => spy } - expect(fixture.getGraphqlServiceSubscriptionObserverMap()).toEqual({ - '1': expect.any(Subscriber), - }); + expect(fixture.getMessageSubscribersSize()).toBe(1); + expect(fixture.hasMessageSubscriber('1')).toBe(true); - // unsubscribing should clean up subscription.unsubscribe(); await fixture.consumeAnyMessage(); - expect(fixture.getGraphqlServiceSubscriptionObserverMapSize()).toBe(0); + expect(fixture.getMessageSubscribersSize()).toBe(0); }); it('when receiving a published message for a subscription that does not exist anymore, it does not throw', async () => { - expect(fixture.getGraphqlServiceSubscriptionObserverMapSize()).toBe(0); + expect(fixture.getMessageSubscribersSize()).toBe(0); const subscription = fixture.triggerSubscription(); await fixture.handleConnectionInit(); @@ -335,43 +329,191 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it('error messages are propagated to the subscriber', async () => { - const ERRORS: QminderGraphQLError[] = [ - { - message: - "Invalid Syntax : offending token 'createdTickets' at line 1 column 1", - sourcePreview: - 'createdTickets(locationId: 673) {\n' + - ' id\n' + - ' firstName\n' + - ' lastName\n', - offendingToken: 'createdTickets', - locations: [], - errorType: 'InvalidSyntax', - extensions: null, - path: null, - }, - ]; + it('GQL_ERROR does not kill the subscription or trigger reconnect', async () => { + const reconnectSpy = jest.spyOn( + fixture.graphqlService as any, + 'handleConnectionDrop', + ); const errorSpy = jest.fn(); const subscription = fixture.triggerSubscription('subscription { baba }', { error: errorSpy, }); await fixture.handleConnectionInit(); await fixture.consumeSubscribeMessage(); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: [{ message: 'Subscription limit reached' }], + }, + }); + + await new Promise((r) => setTimeout(r, 10)); + + expect(errorSpy).not.toHaveBeenCalled(); + expect(reconnectSpy).not.toHaveBeenCalled(); + expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(1); + expect(fixture.hasMessageSubscriber('1')).toBe(true); + + subscription.unsubscribe(); + }); + + it('GQL_ERROR emits true on the subscription error observable', async () => { + const values: boolean[] = []; + fixture.graphqlService + .getSubscriptionErrorObservable() + .subscribe((v) => values.push(v)); + + const subscription = fixture.triggerSubscription('subscription { baba }'); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: [ + { + message: + 'The maximum subscription limit of 100 has been reached', + }, + ], + }, + }); + + await new Promise((r) => setTimeout(r, 10)); + + expect(values).toEqual([false, true]); + + subscription.unsubscribe(); + }); + + it('retries failed subscriptions after delay and clears error state', async () => { + (fixture.graphqlService as any).subscriptionRetryDelayMs = 50; + + const values: boolean[] = []; + fixture.graphqlService + .getSubscriptionErrorObservable() + .subscribe((v) => values.push(v)); + + const subscription = fixture.triggerSubscription('subscription { baba }'); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(); + fixture.server.send({ id: '1', type: 'error', payload: { data: null, - errors: ERRORS, + errors: [{ message: 'Limit reached' }], }, }); - expect(errorSpy).toHaveBeenCalledWith(ERRORS); - // Cleans up as well - expect( - (fixture.graphqlService as any).subscriptionObserverMap['1'], - ).toBeUndefined(); + await new Promise((r) => setTimeout(r, 10)); + expect(values).toEqual([false, true]); + + await new Promise((r) => setTimeout(r, 60)); + + expect(values).toEqual([false, true, false]); + expect(await fixture.getNextMessage()).toEqual({ + id: '1', + type: 'start', + payload: { query: 'subscription { baba }' }, + }); + + subscription.unsubscribe(); + }); + + it('does not send GQL_STOP when server sends GQL_COMPLETE', async () => { + const completeSpy = jest.fn(); + const subscription = fixture.triggerSubscription('subscription { baba }', { + next: () => {}, + complete: completeSpy, + }); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', + }); + + await new Promise((r) => setTimeout(r, 10)); + + expect(completeSpy).toHaveBeenCalled(); + expect(fixture.hasMessageSubscriber('1')).toBe(false); + expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(0); + expect(fixture.server.messagesToConsume.pendingItems).toHaveLength(0); + + subscription.unsubscribe(); + }); + + it('GQL_ERROR keeps subscription tracked so it re-subscribes on natural reconnect and clears error state', async () => { + const values: boolean[] = []; + fixture.graphqlService + .getSubscriptionErrorObservable() + .subscribe((v) => values.push(v)); + + const subscription = fixture.triggerSubscription('subscription { baba }'); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(); + + fixture.sendMessageToClient({ + id: '1', + type: 'error', + payload: { + data: null, + errors: [{ message: 'Limit reached' }], + }, + }); + + await new Promise((r) => setTimeout(r, 10)); + + expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(1); + expect(fixture.hasMessageSubscriber('1')).toBe(true); + expect(values).toEqual([false, true]); + + await fixture.closeWithCode(1001); + fixture.openServer(); + await fixture.handleConnectionInit(); + expect(await fixture.getNextMessage()).toEqual({ + id: '1', + type: 'start', + payload: { query: 'subscription { baba }' }, + }); + + expect(values).toEqual([false, true, false]); + + subscription.unsubscribe(); + }); + + it('cleans up subscription on unknown message type with errors', async () => { + const errorSpy = jest.fn(); + const subscription = fixture.triggerSubscription('subscription { baba }', { + error: errorSpy, + }); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(); + + fixture.sendMessageToClient({ + id: '1', + type: 'unknown_type', + payload: { + errors: [{ message: 'Something went wrong' }], + }, + }); + + await new Promise((r) => setTimeout(r, 10)); + + expect(errorSpy).toHaveBeenCalledWith( + expect.objectContaining({ message: 'Something went wrong' }), + ); + expect(errorSpy.mock.calls[0][0]).toBeInstanceOf(Error); + expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(0); + expect(fixture.hasMessageSubscriber('1')).toBe(false); subscription.unsubscribe(); }); diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 87458fff..58375f4e 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -9,28 +9,16 @@ import { BehaviorSubject, distinctUntilChanged, Observable, - Observer, shareReplay, + Subscriber, } from 'rxjs'; import { ConnectionStatus } from '../../model/connection-status.js'; +import { Logger } from '../../util/logger/logger.js'; import { calculateRandomizedExponentialBackoffTime } from '../../util/randomized-exponential-backoff/randomized-exponential-backoff.js'; import { sleepMs } from '../../util/sleep-ms/sleep-ms.js'; import { ApiBase, GraphqlQuery } from '../api-base/api-base.js'; import { TemporaryApiKeyService } from '../temporary-api-key/temporary-api-key.service.js'; -import { Logger } from '../../util/logger/logger.js'; - -type QueryOrDocument = string | DocumentNode; - -function queryToString(query: QueryOrDocument): string { - if (typeof query === 'string') { - return query; - } - if (query.kind === 'Document') { - return print(query); - } - throw new Error('queryToString: query must be a string or a DocumentNode'); -} export interface QminderGraphQLError { message: string; @@ -51,14 +39,9 @@ interface OperationMessage { }; } -class Subscription { - id: string; - query: string; - - constructor(id: string, query: string) { - this.id = id; - this.query = query; - } +interface Subscription { + readonly messageId: string; + readonly query: string; } enum MessageType { @@ -79,6 +62,7 @@ enum MessageType { const PONG_TIMEOUT_IN_MS = 12000; const PING_PONG_INTERVAL_IN_MS = 20000; +const SUBSCRIBER_RETRY_DELAY_MS = 5000; // https://www.w3.org/TR/websockets/#concept-websocket-close-fail const CLIENT_SIDE_CLOSE_EVENT = 1000; @@ -102,14 +86,21 @@ export class GraphqlService { ConnectionStatus.DISCONNECTED, ); - private nextSubscriptionId = 1; + private nextMessageId = 1; private subscriptions: Subscription[] = []; - private readonly subscriptionObserverMap: { [id: string]: Observer } = - {}; + private readonly messageSubscribers = new Map< + string, + Subscriber> + >(); private readonly subscriptionConnection$: Observable; + + private readonly hasSubscriberErrored$ = new BehaviorSubject(false); + private readonly failedSubscribers = new Set(); + + private subscriberRetryTimeout: ReturnType | null = null; private temporaryApiKeyService: TemporaryApiKeyService | undefined; private pongTimeout: any; @@ -198,25 +189,30 @@ export class GraphqlService { * ``` * * @param queryDocument required: the GraphQL query to send, for example `"subscription { createdTickets(locationId: 123) { id firstName } }"` - * @returns an RxJS Observable that will push data as - * @throws when the 'query' argument is undefined or an empty string + * @returns an RxJS Observable that will push data + * @throws when the 'queryDocument' argument is an empty string */ - subscribe(queryDocument: QueryOrDocument): Observable { - const query = queryToString(queryDocument); + subscribe>( + queryDocument: string | DocumentNode, + ): Observable { + const query = + typeof queryDocument === 'string' ? queryDocument : print(queryDocument); - if (!query || query.length === 0) { + if (!query) { throw new Error( 'GraphQLService query expects a GraphQL query as its first argument', ); } - return new Observable((observer: Observer) => { - const id = this.generateOperationId(); - this.subscriptions.push(new Subscription(id, query)); - this.sendMessage(id, MessageType.GQL_START, { query }); - this.subscriptionObserverMap[id] = observer; + return new Observable((subscriber) => { + const messageId = `${this.nextMessageId++}`; + this.subscriptions.push({ messageId, query }); + this.sendMessage(messageId, MessageType.GQL_START, { query }); + this.messageSubscribers.set(messageId, subscriber); - return () => this.stopSubscription(id); + return () => { + this.removeSubscriber(messageId); + }; }); } @@ -258,6 +254,13 @@ export class GraphqlService { return this.subscriptionConnection$; } + /** + * Emits `false` if errored subscriptions are successfully retried + */ + hasSubscriberErrored(): Observable { + return this.hasSubscriberErrored$.pipe(distinctUntilChanged()); + } + /** * Set the WebSocket hostname the GraphQL service uses. * @hidden @@ -266,19 +269,28 @@ export class GraphqlService { this.apiServer = apiServer; } - private stopSubscription(id: string) { - this.sendMessage(id, MessageType.GQL_STOP, null); - this.cleanupSubscription(id); + private removeSubscriber(messageId: string): void { + if (this.messageSubscribers.has(messageId)) { + this.sendMessage(messageId, MessageType.GQL_STOP, null); + } + + this.cleanUpSubscription(messageId); } - private cleanupSubscription(id: string) { - delete this.subscriptionObserverMap[id]; - this.subscriptions = this.subscriptions.filter((sub) => { - return sub.id !== id; - }); + private cleanUpSubscription(messageId: string): void { + this.messageSubscribers.delete(messageId); + this.failedSubscribers.delete(messageId); + + this.subscriptions = this.subscriptions.filter( + ({ messageId: id }) => id !== messageId, + ); + + if (!this.failedSubscribers.size) { + this.hasSubscriberErrored$.next(false); + } } - private openSocket() { + private async openSocket(): Promise { if ( [ConnectionStatus.CONNECTING, ConnectionStatus.CONNECTED].includes( this.connectionStatus, @@ -286,26 +298,23 @@ export class GraphqlService { ) { return; } + this.setConnectionStatus(ConnectionStatus.CONNECTING); this.logger.info('Connecting to websocket'); - this.fetchTemporaryApiKey() - .then((temporaryApiKey: string) => { - this.createSocketConnection(temporaryApiKey); - }) - .catch((e) => { - throw e; - }); + + const temporaryApiKey = await this.getTemporaryApiKey(); + this.createSocketConnection(temporaryApiKey); } - private async fetchTemporaryApiKey(): Promise { - return this.temporaryApiKeyService.fetchTemporaryApiKey(); + private async getTemporaryApiKey(): Promise { + return await this.temporaryApiKeyService.fetchTemporaryApiKey(); } private getServerUrl(temporaryApiKey: string): string { return `wss://${this.apiServer}:443/graphql/subscription?rest-api-key=${temporaryApiKey}`; } - private createSocketConnection(temporaryApiKey: string) { + private createSocketConnection(temporaryApiKey: string): void { if (this.socket) { this.socket.onclose = null; this.socket.onmessage = null; @@ -314,9 +323,10 @@ export class GraphqlService { this.socket.close(); this.socket = null; } - this.socket = new WebSocket(this.getServerUrl(temporaryApiKey)); + this.socket = new WebSocket(this.getServerUrl(temporaryApiKey)); const socket = this.socket; + socket.onopen = () => { this.sendRawMessage( JSON.stringify({ @@ -327,7 +337,7 @@ export class GraphqlService { ); }; - socket.onclose = (event: CloseEvent) => { + socket.onclose = (event) => { this.logger.warn('WebSocket connection closed:', { code: event.code, reason: event.reason, @@ -337,15 +347,18 @@ export class GraphqlService { this.socket = null; this.clearPingMonitoring(); + if (this.shouldRetry(event)) { const timer = calculateRandomizedExponentialBackoffTime( this.connectionAttemptsCount, ); + this.logger.info( `Waiting for ${timer.toFixed(1)}ms before reconnecting`, ); + sleepMs(timer).then(() => { - this.connectionAttemptsCount += 1; + this.connectionAttemptsCount++; this.openSocket(); }); } @@ -358,104 +371,133 @@ export class GraphqlService { }; socket.onerror = () => { - const message = 'Websocket error occurred!'; if (this.isBrowserOnline()) { - this.logger.error(message); + this.logger.error('Websocket error occurred!'); } else { - this.logger.info(message); + this.logger.info('Websocket error occurred!'); } }; - socket.onmessage = (rawMessage: { data: WebSocket.Data }) => { - if (typeof rawMessage.data === 'string') { - const message: OperationMessage = JSON.parse(rawMessage.data); - - switch (message.type) { - case MessageType.GQL_CONNECTION_KEEP_ALIVE: - break; - - case MessageType.GQL_CONNECTION_ACK: { - this.connectionAttemptsCount = 0; - this.setConnectionStatus(ConnectionStatus.CONNECTED); - this.logger.info('Connected to websocket'); - this.startConnectionMonitoring(); - let resubscriptionFailed = false; - for (const subscription of this.subscriptions) { - const payload = { query: subscription.query }; - const msg = JSON.stringify({ - id: subscription.id, - type: MessageType.GQL_START, - payload, - }); - if (!this.sendRawMessage(msg)) { - this.logger.warn( - `Failed to re-subscribe ${this.subscriptions.length} subscription(s): WebSocket not open`, - ); - resubscriptionFailed = true; - break; - } - } - if (resubscriptionFailed) { - this.handleConnectionDrop(); - } - break; - } + socket.onmessage = (event) => { + if (typeof event.data !== 'string') { + return; + } - case MessageType.GQL_DATA: - this.subscriptionObserverMap[message.id]?.next( - message.payload.data, - ); - break; + const message: OperationMessage = JSON.parse(event.data); - case MessageType.GQL_COMPLETE: - this.subscriptionObserverMap[message.id]?.complete(); - break; + switch (message.type) { + case MessageType.GQL_CONNECTION_KEEP_ALIVE: + break; - case MessageType.GQL_PONG: - clearTimeout(this.pongTimeout); - break; + case MessageType.GQL_CONNECTION_ACK: { + this.connectionAttemptsCount = 0; - case MessageType.GQL_ERROR: - this.subscriptionObserverMap[message.id]?.error( - message.payload.errors, - ); - this.cleanupSubscription(message.id); - break; + this.clearSubscriberRetry(); + this.failedSubscribers.clear(); + this.hasSubscriberErrored$.next(false); - default: - if (message.payload && message.payload.data) { - this.subscriptionObserverMap[message.id]?.error( - message.payload.data, - ); - } else if ( - message.payload.errors && - message.payload.errors.length > 0 - ) { - this.subscriptionObserverMap[message.id]?.error( - message.payload.errors, + this.setConnectionStatus(ConnectionStatus.CONNECTED); + this.logger.info('Connected to websocket'); + this.startConnectionMonitoring(); + + let resubscriptionFailed = false; + + for (const { messageId, query } of this.subscriptions) { + const msg = JSON.stringify({ + id: messageId, + type: MessageType.GQL_START, + payload: { query }, + }); + + if (!this.sendRawMessage(msg)) { + this.logger.warn( + `Failed to re-subscribe ${this.subscriptions.length} subscription(s): WebSocket not open`, ); + + resubscriptionFailed = true; + break; } + } + + if (resubscriptionFailed) { + this.handleConnectionDrop(); + } + + break; + } + + case MessageType.GQL_DATA: + this.messageSubscribers.get(message.id)?.next(message.payload.data); + break; + + case MessageType.GQL_COMPLETE: { + const subscriber: Subscriber> | undefined = + this.messageSubscribers.get(message.id); + + this.cleanUpSubscription(message.id); + subscriber?.complete(); + + break; + } + + case MessageType.GQL_PONG: + clearTimeout(this.pongTimeout); + break; + + case MessageType.GQL_ERROR: + this.logger.warn( + `GraphQL subscription error: ${JSON.stringify(message)}`, + ); + + this.failedSubscribers.add(message.id); + this.hasSubscriberErrored$.next(true); + + if (!this.subscriberRetryTimeout) { + this.scheduleSubscriberRetry(); + } + + break; + + default: { + const subscriber = this.messageSubscribers.get(message.id); + if (!subscriber) { + return; + } + + this.cleanUpSubscription(message.id); + + if (message.payload?.data) { + subscriber.error(message.payload.data); + return; + } + + if (message.payload?.errors?.length) { + subscriber.error( + new Error( + message.payload.errors.map(({ message }) => message).join('; '), + ), + ); + } } } }; } - private shouldRetry(event: CloseEvent) { - if (event.code !== CLIENT_SIDE_CLOSE_EVENT) { - return true; - } - - return Object.entries(this.subscriptionObserverMap).length > 0; + private shouldRetry(event: CloseEvent): boolean { + return ( + event.code !== CLIENT_SIDE_CLOSE_EVENT || !!this.messageSubscribers.size + ); } - private sendMessage(id: string, type: MessageType, payload: any) { - if (this.connectionStatus === ConnectionStatus.CONNECTED) { - if (!this.sendRawMessage(JSON.stringify({ id, type, payload }))) { - this.logger.warn('Message dropped: WebSocket is not in OPEN state'); - this.handleConnectionDrop(); - } - } else { + private sendMessage(id: string, type: MessageType, payload: any): void { + if (this.connectionStatus !== ConnectionStatus.CONNECTED) { this.openSocket(); + return; + } + + if (!this.sendRawMessage(JSON.stringify({ id, type, payload }))) { + this.logger.warn('Message dropped: WebSocket is not in OPEN state'); + this.handleConnectionDrop(); } } @@ -464,16 +506,11 @@ export class GraphqlService { this.socket.send(message); return true; } - return false; - } - private generateOperationId(): string { - const currentId = `${this.nextSubscriptionId}`; - this.nextSubscriptionId += 1; - return currentId; + return false; } - private setConnectionStatus(status: ConnectionStatus) { + private setConnectionStatus(status: ConnectionStatus): void { this.connectionStatus = status; this.connectionStatus$.next(status); } @@ -509,11 +546,13 @@ export class GraphqlService { if (this.connectionStatus === ConnectionStatus.CONNECTING) { return; } + if (this.isBrowserOnline()) { this.logger.warn(`Websocket connection dropped!`); } else { this.logger.info(`Websocket connection dropped. We are offline.`); } + this.setConnectionStatus(ConnectionStatus.DISCONNECTED); this.clearPingMonitoring(); @@ -525,14 +564,47 @@ export class GraphqlService { clearInterval(this.pingPongInterval); } + private clearSubscriberRetry(): void { + clearTimeout(this.subscriberRetryTimeout); + this.subscriberRetryTimeout = null; + } + + private scheduleSubscriberRetry(): void { + this.subscriberRetryTimeout = setTimeout(() => { + this.subscriberRetryTimeout = null; + this.retryFailedSubscribers(); + }, SUBSCRIBER_RETRY_DELAY_MS); + } + + private retryFailedSubscribers(): void { + const subscribers = [...this.failedSubscribers]; + + this.failedSubscribers.clear(); + this.hasSubscriberErrored$.next(false); + + for (const messageId of subscribers) { + const subscription = this.subscriptions.find( + (subscription) => subscription.messageId === messageId, + ); + + if (!subscription) { + continue; + } + + this.sendRawMessage( + JSON.stringify({ + id: subscription.messageId, + type: MessageType.GQL_START, + payload: { query: subscription.query }, + }), + ); + } + } + /** - * Returns the online status of the browser. - * In the non-browser environment (NodeJS) this always returns true. + * In a non-browser environment (NodeJS) returns true. */ private isBrowserOnline(): boolean { - if (typeof navigator === 'undefined') { - return true; - } - return navigator.onLine; + return typeof navigator === 'undefined' || navigator.onLine; } } From 8a07e0a186fd6fadb0182b434cfa663b1cb595a9 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 15:28:57 +0300 Subject: [PATCH 02/35] Cleanup --- .../lib/services/graphql/graphql.service.ts | 171 +++++++++--------- 1 file changed, 81 insertions(+), 90 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 58375f4e..f864f8fc 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -20,6 +20,12 @@ import { sleepMs } from '../../util/sleep-ms/sleep-ms.js'; import { ApiBase, GraphqlQuery } from '../api-base/api-base.js'; import { TemporaryApiKeyService } from '../temporary-api-key/temporary-api-key.service.js'; +function parseQuery(queryOrDocumentNode: string | DocumentNode): string { + return typeof queryOrDocumentNode === 'string' + ? queryOrDocumentNode + : print(queryOrDocumentNode); +} + export interface QminderGraphQLError { message: string; errorType?: string | null; @@ -30,12 +36,12 @@ export interface QminderGraphQLError { path?: (string | number)[] | null; } -interface OperationMessage { - id?: string; - type: MessageType; - payload?: { - data?: T | null; - errors?: QminderGraphQLError[]; +interface Message { + readonly id?: string; + readonly type: MessageType; + readonly payload?: { + readonly data?: Record | null; + readonly errors?: QminderGraphQLError[]; }; } @@ -60,9 +66,9 @@ enum MessageType { GQL_ERROR = 'error', } +const ERRORED_SUBSCRIPTIONS_RETRY_DELAY_MS = 5 /* seconds */ * 1000; /* ms */ const PONG_TIMEOUT_IN_MS = 12000; const PING_PONG_INTERVAL_IN_MS = 20000; -const SUBSCRIBER_RETRY_DELAY_MS = 5000; // https://www.w3.org/TR/websockets/#concept-websocket-close-fail const CLIENT_SIDE_CLOSE_EVENT = 1000; @@ -86,7 +92,7 @@ export class GraphqlService { ConnectionStatus.DISCONNECTED, ); - private nextMessageId = 1; + private subcriptionsCount = 0; private subscriptions: Subscription[] = []; @@ -97,10 +103,13 @@ export class GraphqlService { private readonly subscriptionConnection$: Observable; - private readonly hasSubscriberErrored$ = new BehaviorSubject(false); - private readonly failedSubscribers = new Set(); + private readonly haveAnySubscriptionsErrored$ = new BehaviorSubject(false); + private readonly erroredSubscriptionsMessageIds = new Set(); + + private erroredSubscriptionsRetryTimeout: ReturnType< + typeof setTimeout + > | null = null; - private subscriberRetryTimeout: ReturnType | null = null; private temporaryApiKeyService: TemporaryApiKeyService | undefined; private pongTimeout: any; @@ -188,16 +197,14 @@ export class GraphqlService { * } * ``` * - * @param queryDocument required: the GraphQL query to send, for example `"subscription { createdTickets(locationId: 123) { id firstName } }"` + * @param queryOrDocumentNode the GraphQL query to send, for example `"subscription { createdTickets(locationId: 123) { id firstName } }"` * @returns an RxJS Observable that will push data * @throws when the 'queryDocument' argument is an empty string */ subscribe>( - queryDocument: string | DocumentNode, + queryOrDocumentNode: string | DocumentNode, ): Observable { - const query = - typeof queryDocument === 'string' ? queryDocument : print(queryDocument); - + const query = parseQuery(queryOrDocumentNode); if (!query) { throw new Error( 'GraphQLService query expects a GraphQL query as its first argument', @@ -205,13 +212,18 @@ export class GraphqlService { } return new Observable((subscriber) => { - const messageId = `${this.nextMessageId++}`; + const messageId = `${++this.subcriptionsCount}`; this.subscriptions.push({ messageId, query }); this.sendMessage(messageId, MessageType.GQL_START, { query }); this.messageSubscribers.set(messageId, subscriber); return () => { - this.removeSubscriber(messageId); + this.sendMessage(messageId, MessageType.GQL_STOP, null); + this.cleanUpSubscription(messageId); + + if (!this.erroredSubscriptionsMessageIds.size) { + this.haveAnySubscriptionsErrored$.next(false); + } }; }); } @@ -223,13 +235,13 @@ export class GraphqlService { * There is no need to call this method in order for data transfer to work. The `subscribe()` method also initializes * a websocket connection before proceeding. */ - openPendingWebSocket(): void { + async openPendingWebSocket(): Promise { if ( ![ConnectionStatus.CONNECTING, ConnectionStatus.CONNECTED].includes( this.connectionStatus, ) ) { - this.openSocket(); + await this.openSocket(); } } @@ -239,7 +251,7 @@ export class GraphqlService { * This method is automatically called when doing Qminder.setKey(). * @hidden */ - setKey(apiKey: string) { + setKey(apiKey: string): void { this.temporaryApiKeyService = new TemporaryApiKeyService( this.apiServer, apiKey, @@ -255,39 +267,29 @@ export class GraphqlService { } /** - * Emits `false` if errored subscriptions are successfully retried + * Have any GraphQL subscriptions been rejected by the server. + * + * Emits `false` if all errored subscriptions have been successfully retried. */ - hasSubscriberErrored(): Observable { - return this.hasSubscriberErrored$.pipe(distinctUntilChanged()); + haveAnySubscriptionsErrored(): Observable { + return this.haveAnySubscriptionsErrored$.pipe(distinctUntilChanged()); } /** * Set the WebSocket hostname the GraphQL service uses. * @hidden */ - setServer(apiServer: string) { + setServer(apiServer: string): void { this.apiServer = apiServer; } - private removeSubscriber(messageId: string): void { - if (this.messageSubscribers.has(messageId)) { - this.sendMessage(messageId, MessageType.GQL_STOP, null); - } - - this.cleanUpSubscription(messageId); - } - private cleanUpSubscription(messageId: string): void { + this.erroredSubscriptionsMessageIds.delete(messageId); this.messageSubscribers.delete(messageId); - this.failedSubscribers.delete(messageId); this.subscriptions = this.subscriptions.filter( - ({ messageId: id }) => id !== messageId, + (subscription) => subscription.messageId !== messageId, ); - - if (!this.failedSubscribers.size) { - this.hasSubscriberErrored$.next(false); - } } private async openSocket(): Promise { @@ -325,9 +327,8 @@ export class GraphqlService { } this.socket = new WebSocket(this.getServerUrl(temporaryApiKey)); - const socket = this.socket; - socket.onopen = () => { + this.socket.onopen = () => { this.sendRawMessage( JSON.stringify({ id: undefined, @@ -337,7 +338,7 @@ export class GraphqlService { ); }; - socket.onclose = (event) => { + this.socket.onclose = (event) => { this.logger.warn('WebSocket connection closed:', { code: event.code, reason: event.reason, @@ -345,7 +346,6 @@ export class GraphqlService { this.setConnectionStatus(ConnectionStatus.DISCONNECTED); this.socket = null; - this.clearPingMonitoring(); if (this.shouldRetry(event)) { @@ -370,7 +370,7 @@ export class GraphqlService { } }; - socket.onerror = () => { + this.socket.onerror = () => { if (this.isBrowserOnline()) { this.logger.error('Websocket error occurred!'); } else { @@ -378,12 +378,12 @@ export class GraphqlService { } }; - socket.onmessage = (event) => { + this.socket.onmessage = (event) => { if (typeof event.data !== 'string') { return; } - const message: OperationMessage = JSON.parse(event.data); + const message: Message = JSON.parse(event.data); switch (message.type) { case MessageType.GQL_CONNECTION_KEEP_ALIVE: @@ -392,9 +392,9 @@ export class GraphqlService { case MessageType.GQL_CONNECTION_ACK: { this.connectionAttemptsCount = 0; - this.clearSubscriberRetry(); - this.failedSubscribers.clear(); - this.hasSubscriberErrored$.next(false); + this.clearErroredSubscriptionsRetry(); + this.erroredSubscriptionsMessageIds.clear(); + this.haveAnySubscriptionsErrored$.next(false); this.setConnectionStatus(ConnectionStatus.CONNECTED); this.logger.info('Connected to websocket'); @@ -431,12 +431,8 @@ export class GraphqlService { break; case MessageType.GQL_COMPLETE: { - const subscriber: Subscriber> | undefined = - this.messageSubscribers.get(message.id); - + this.messageSubscribers.get(message.id)?.complete(); this.cleanUpSubscription(message.id); - subscriber?.complete(); - break; } @@ -449,11 +445,11 @@ export class GraphqlService { `GraphQL subscription error: ${JSON.stringify(message)}`, ); - this.failedSubscribers.add(message.id); - this.hasSubscriberErrored$.next(true); + this.erroredSubscriptionsMessageIds.add(message.id); + this.haveAnySubscriptionsErrored$.next(true); - if (!this.subscriberRetryTimeout) { - this.scheduleSubscriberRetry(); + if (!this.erroredSubscriptionsRetryTimeout) { + this.scheduleErroredSubscriptionsRetry(); } break; @@ -464,19 +460,12 @@ export class GraphqlService { return; } - this.cleanUpSubscription(message.id); - if (message.payload?.data) { subscriber.error(message.payload.data); - return; - } - - if (message.payload?.errors?.length) { - subscriber.error( - new Error( - message.payload.errors.map(({ message }) => message).join('; '), - ), - ); + this.cleanUpSubscription(message.id); + } else if (message.payload?.errors?.length) { + subscriber.error(message.payload.errors); + this.cleanUpSubscription(message.id); } } } @@ -489,15 +478,19 @@ export class GraphqlService { ); } - private sendMessage(id: string, type: MessageType, payload: any): void { + private async sendMessage( + id: string, + type: MessageType, + payload: Record | null, + ): Promise { if (this.connectionStatus !== ConnectionStatus.CONNECTED) { - this.openSocket(); + await this.openSocket(); return; } if (!this.sendRawMessage(JSON.stringify({ id, type, payload }))) { this.logger.warn('Message dropped: WebSocket is not in OPEN state'); - this.handleConnectionDrop(); + await this.handleConnectionDrop(); } } @@ -542,7 +535,7 @@ export class GraphqlService { this.sendRawMessage(JSON.stringify({ type: MessageType.GQL_PING })); } - private handleConnectionDrop(): void { + private async handleConnectionDrop(): Promise { if (this.connectionStatus === ConnectionStatus.CONNECTING) { return; } @@ -556,7 +549,7 @@ export class GraphqlService { this.setConnectionStatus(ConnectionStatus.DISCONNECTED); this.clearPingMonitoring(); - this.openSocket(); + await this.openSocket(); } private clearPingMonitoring(): void { @@ -564,25 +557,20 @@ export class GraphqlService { clearInterval(this.pingPongInterval); } - private clearSubscriberRetry(): void { - clearTimeout(this.subscriberRetryTimeout); - this.subscriberRetryTimeout = null; + private clearErroredSubscriptionsRetry(): void { + clearTimeout(this.erroredSubscriptionsRetryTimeout ?? undefined); + this.erroredSubscriptionsRetryTimeout = null; } - private scheduleSubscriberRetry(): void { - this.subscriberRetryTimeout = setTimeout(() => { - this.subscriberRetryTimeout = null; - this.retryFailedSubscribers(); - }, SUBSCRIBER_RETRY_DELAY_MS); + private scheduleErroredSubscriptionsRetry(): void { + this.erroredSubscriptionsRetryTimeout = setTimeout(() => { + this.retryErroredSubscriptions(); + this.erroredSubscriptionsRetryTimeout = null; + }, ERRORED_SUBSCRIPTIONS_RETRY_DELAY_MS); } - private retryFailedSubscribers(): void { - const subscribers = [...this.failedSubscribers]; - - this.failedSubscribers.clear(); - this.hasSubscriberErrored$.next(false); - - for (const messageId of subscribers) { + private retryErroredSubscriptions(): void { + for (const messageId of this.erroredSubscriptionsMessageIds) { const subscription = this.subscriptions.find( (subscription) => subscription.messageId === messageId, ); @@ -599,10 +587,13 @@ export class GraphqlService { }), ); } + + this.erroredSubscriptionsMessageIds.clear(); + this.haveAnySubscriptionsErrored$.next(false); } /** - * In a non-browser environment (NodeJS) returns true. + * In a non-browser environment (NodeJS) returns `true`. */ private isBrowserOnline(): boolean { return typeof navigator === 'undefined' || navigator.onLine; From aa4fab18efcd2ad240c1e110b7de37baa8decc18 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 15:39:44 +0300 Subject: [PATCH 03/35] Refactor to remove unnecessary steps --- .../lib/services/graphql/graphql.service.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index f864f8fc..1b22d2ab 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -218,12 +218,11 @@ export class GraphqlService { this.messageSubscribers.set(messageId, subscriber); return () => { - this.sendMessage(messageId, MessageType.GQL_STOP, null); - this.cleanUpSubscription(messageId); - - if (!this.erroredSubscriptionsMessageIds.size) { - this.haveAnySubscriptionsErrored$.next(false); + if (this.messageSubscribers.has(messageId)) { + this.sendMessage(messageId, MessageType.GQL_STOP, null); } + + this.cleanUpSubscription(messageId); }; }); } @@ -290,6 +289,10 @@ export class GraphqlService { this.subscriptions = this.subscriptions.filter( (subscription) => subscription.messageId !== messageId, ); + + if (!this.erroredSubscriptionsMessageIds.size) { + this.haveAnySubscriptionsErrored$.next(false); + } } private async openSocket(): Promise { @@ -431,8 +434,9 @@ export class GraphqlService { break; case MessageType.GQL_COMPLETE: { - this.messageSubscribers.get(message.id)?.complete(); + const subscriber = this.messageSubscribers.get(message.id); this.cleanUpSubscription(message.id); + subscriber?.complete(); break; } @@ -461,11 +465,11 @@ export class GraphqlService { } if (message.payload?.data) { - subscriber.error(message.payload.data); this.cleanUpSubscription(message.id); + subscriber.error(message.payload.data); } else if (message.payload?.errors?.length) { - subscriber.error(message.payload.errors); this.cleanUpSubscription(message.id); + subscriber.error(message.payload.errors); } } } From a26644be104497a6c87b97fea819341f40211aa6 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 18:41:47 +0300 Subject: [PATCH 04/35] Add exponential backoff --- .../lib/services/graphql/graphql.service.ts | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 1b22d2ab..af4ac8fb 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -66,7 +66,6 @@ enum MessageType { GQL_ERROR = 'error', } -const ERRORED_SUBSCRIPTIONS_RETRY_DELAY_MS = 5 /* seconds */ * 1000; /* ms */ const PONG_TIMEOUT_IN_MS = 12000; const PING_PONG_INTERVAL_IN_MS = 20000; @@ -110,6 +109,8 @@ export class GraphqlService { typeof setTimeout > | null = null; + private erroredSubscriptionsRetryCount = 0; + private temporaryApiKeyService: TemporaryApiKeyService | undefined; private pongTimeout: any; @@ -356,9 +357,7 @@ export class GraphqlService { this.connectionAttemptsCount, ); - this.logger.info( - `Waiting for ${timer.toFixed(1)}ms before reconnecting`, - ); + this.logger.info(`Reconnect socket in ${timer.toFixed(1)}ms`); sleepMs(timer).then(() => { this.connectionAttemptsCount++; @@ -396,6 +395,7 @@ export class GraphqlService { this.connectionAttemptsCount = 0; this.clearErroredSubscriptionsRetry(); + this.erroredSubscriptionsRetryCount = 0; this.erroredSubscriptionsMessageIds.clear(); this.haveAnySubscriptionsErrored$.next(false); @@ -430,6 +430,13 @@ export class GraphqlService { } case MessageType.GQL_DATA: + if ( + this.erroredSubscriptionsMessageIds.delete(message.id) && + !this.erroredSubscriptionsMessageIds.size + ) { + this.haveAnySubscriptionsErrored$.next(false); + } + this.messageSubscribers.get(message.id)?.next(message.payload.data); break; @@ -567,10 +574,16 @@ export class GraphqlService { } private scheduleErroredSubscriptionsRetry(): void { + const delay = calculateRandomizedExponentialBackoffTime( + this.erroredSubscriptionsRetryCount++, + ); + + this.logger.info(`Retry errored subscriptions in ${delay.toFixed(0)}ms`); + this.erroredSubscriptionsRetryTimeout = setTimeout(() => { this.retryErroredSubscriptions(); this.erroredSubscriptionsRetryTimeout = null; - }, ERRORED_SUBSCRIPTIONS_RETRY_DELAY_MS); + }, delay); } private retryErroredSubscriptions(): void { @@ -591,9 +604,6 @@ export class GraphqlService { }), ); } - - this.erroredSubscriptionsMessageIds.clear(); - this.haveAnySubscriptionsErrored$.next(false); } /** From b926a0e991681f30e69c54934ddf840707782606 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 18:45:05 +0300 Subject: [PATCH 05/35] Improve JS docs formatting --- .../javascript-api/src/lib/services/graphql/graphql.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index af4ac8fb..dbf186fc 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -200,7 +200,7 @@ export class GraphqlService { * * @param queryOrDocumentNode the GraphQL query to send, for example `"subscription { createdTickets(locationId: 123) { id firstName } }"` * @returns an RxJS Observable that will push data - * @throws when the 'queryDocument' argument is an empty string + * @throws when the `queryDocument` argument is an empty string */ subscribe>( queryOrDocumentNode: string | DocumentNode, From f8a6aaeac1edd31c9a7d2ed7596aa10d0ddf5195 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 19:39:37 +0300 Subject: [PATCH 06/35] Simplify --- .../lib/services/graphql/graphql.service.ts | 112 ++++++++++++------ 1 file changed, 79 insertions(+), 33 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index dbf186fc..eb915b22 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -8,9 +8,14 @@ import WebSocket, { CloseEvent } from 'isomorphic-ws'; import { BehaviorSubject, distinctUntilChanged, + map, Observable, + scan, shareReplay, + startWith, + Subject, Subscriber, + take, } from 'rxjs'; import { ConnectionStatus } from '../../model/connection-status.js'; @@ -102,8 +107,44 @@ export class GraphqlService { private readonly subscriptionConnection$: Observable; - private readonly haveAnySubscriptionsErrored$ = new BehaviorSubject(false); - private readonly erroredSubscriptionsMessageIds = new Set(); + private readonly erroredSubscriptionsAction$ = new Subject< + | { + readonly type: 'add'; + readonly messageId: string; + } + | { + readonly type: 'remove'; + readonly messageId: string; + } + | { + readonly type: 'clear'; + } + >(); + + private readonly erroredSubscriptionsMessageIds$ = + this.erroredSubscriptionsAction$.pipe( + scan((messageIds, action) => { + const result = new Set(messageIds); + + switch (action.type) { + case 'add': + return result.add(action.messageId); + case 'remove': + result.delete(action.messageId); + return result; + case 'clear': + return new Set(); + } + }, new Set()), + startWith(new Set()), + shareReplay(1), + ); + + private readonly haveAnySubscriptionsErrored$ = + this.erroredSubscriptionsMessageIds$.pipe( + map(({ size }) => !!size), + distinctUntilChanged(), + ); private erroredSubscriptionsRetryTimeout: ReturnType< typeof setTimeout @@ -129,6 +170,8 @@ export class GraphqlService { distinctUntilChanged(), shareReplay(1), ); + + this.erroredSubscriptionsMessageIds$.subscribe(); } /** @@ -272,7 +315,7 @@ export class GraphqlService { * Emits `false` if all errored subscriptions have been successfully retried. */ haveAnySubscriptionsErrored(): Observable { - return this.haveAnySubscriptionsErrored$.pipe(distinctUntilChanged()); + return this.haveAnySubscriptionsErrored$; } /** @@ -284,16 +327,16 @@ export class GraphqlService { } private cleanUpSubscription(messageId: string): void { - this.erroredSubscriptionsMessageIds.delete(messageId); + this.erroredSubscriptionsAction$.next({ + type: 'remove', + messageId, + }); + this.messageSubscribers.delete(messageId); this.subscriptions = this.subscriptions.filter( (subscription) => subscription.messageId !== messageId, ); - - if (!this.erroredSubscriptionsMessageIds.size) { - this.haveAnySubscriptionsErrored$.next(false); - } } private async openSocket(): Promise { @@ -396,8 +439,7 @@ export class GraphqlService { this.clearErroredSubscriptionsRetry(); this.erroredSubscriptionsRetryCount = 0; - this.erroredSubscriptionsMessageIds.clear(); - this.haveAnySubscriptionsErrored$.next(false); + this.erroredSubscriptionsAction$.next({ type: 'clear' }); this.setConnectionStatus(ConnectionStatus.CONNECTED); this.logger.info('Connected to websocket'); @@ -430,12 +472,10 @@ export class GraphqlService { } case MessageType.GQL_DATA: - if ( - this.erroredSubscriptionsMessageIds.delete(message.id) && - !this.erroredSubscriptionsMessageIds.size - ) { - this.haveAnySubscriptionsErrored$.next(false); - } + this.erroredSubscriptionsAction$.next({ + type: 'remove', + messageId: message.id, + }); this.messageSubscribers.get(message.id)?.next(message.payload.data); break; @@ -456,8 +496,10 @@ export class GraphqlService { `GraphQL subscription error: ${JSON.stringify(message)}`, ); - this.erroredSubscriptionsMessageIds.add(message.id); - this.haveAnySubscriptionsErrored$.next(true); + this.erroredSubscriptionsAction$.next({ + type: 'add', + messageId: message.id, + }); if (!this.erroredSubscriptionsRetryTimeout) { this.scheduleErroredSubscriptionsRetry(); @@ -587,23 +629,27 @@ export class GraphqlService { } private retryErroredSubscriptions(): void { - for (const messageId of this.erroredSubscriptionsMessageIds) { - const subscription = this.subscriptions.find( - (subscription) => subscription.messageId === messageId, - ); + this.erroredSubscriptionsMessageIds$ + .pipe(take(1)) + .subscribe((messageIds) => { + for (const messageId of messageIds) { + const subscription = this.subscriptions.find( + (subscription) => subscription.messageId === messageId, + ); - if (!subscription) { - continue; - } + if (!subscription) { + continue; + } - this.sendRawMessage( - JSON.stringify({ - id: subscription.messageId, - type: MessageType.GQL_START, - payload: { query: subscription.query }, - }), - ); - } + this.sendRawMessage( + JSON.stringify({ + id: subscription.messageId, + type: MessageType.GQL_START, + payload: { query: subscription.query }, + }), + ); + } + }); } /** From fd52a21c0e9fceb60c8b7a7e14183985b7e5cc89 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 20:13:24 +0300 Subject: [PATCH 07/35] Cleanup --- .../src/lib/services/graphql/graphql.service.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index eb915b22..1140844a 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -158,9 +158,6 @@ export class GraphqlService { private pingPongInterval: any; private readonly sendPingWithThisBound = this.sendPing.bind(this); - private readonly handleConnectionDropWithThisBound = - this.handleConnectionDrop.bind(this); - private connectionAttemptsCount = 0; constructor() { @@ -400,7 +397,7 @@ export class GraphqlService { this.connectionAttemptsCount, ); - this.logger.info(`Reconnect socket in ${timer.toFixed(1)}ms`); + this.logger.info(`Reconnect socket in ${timer.toFixed(0)}ms`); sleepMs(timer).then(() => { this.connectionAttemptsCount++; @@ -582,9 +579,10 @@ export class GraphqlService { private sendPing(): void { this.pongTimeout = setTimeout( - this.handleConnectionDropWithThisBound, + () => this.handleConnectionDrop(), PONG_TIMEOUT_IN_MS, ); + this.sendRawMessage(JSON.stringify({ type: MessageType.GQL_PING })); } From 333ce7d6bf93e2eff5bb129df046d511f9e717c3 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 20:43:08 +0300 Subject: [PATCH 08/35] Improve JSDoc --- .../lib/services/graphql/graphql.service.ts | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 1140844a..cc4fafa0 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -223,24 +223,36 @@ export class GraphqlService { /** * Subscribe to Qminder Events API using GraphQL. * - * For example + * @example + * + * Be notified of any created tickets * * ```javascript * import { Qminder } from 'qminder-api'; - * // 1. Be notified of any created tickets - * try { - * const observable = Qminder.GraphQL.subscribe("subscription { createdTickets(locationId: 123) { id firstName } }") * - * observable.subscribe(data => console.log(data)); - * // => { createdTickets: { id: '12', firstName: 'Marta' } } + * try { + * Qminder.GraphQL.subscribe(` + * subscription { + * createdTickets(locationId: 123) { + * id + * firstName + * } + * } + * `).subscribe((data) => { + * console.log(data); // { createdTickets: { id: '12', firstName: 'Marta' } } + * }); * } catch (error) { * console.error(error); * } * ``` * * @param queryOrDocumentNode the GraphQL query to send, for example `"subscription { createdTickets(locationId: 123) { id firstName } }"` - * @returns an RxJS Observable that will push data + * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string + * + * Retries errored subscriptions (doesn't throw) with exponential backoff. + * + * To get notified when any subscriptions have errored, use the {@link haveAnySubscriptionsErrored} method. */ subscribe>( queryOrDocumentNode: string | DocumentNode, From 6e7f6127c78dc1ff7a16919b06a6cead7b3ef0ae Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 22:23:41 +0300 Subject: [PATCH 09/35] Add subscription retry limit --- .../lib/services/graphql/graphql.service.ts | 39 ++++++++++++++++--- 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index cc4fafa0..04e3f591 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -71,6 +71,7 @@ enum MessageType { GQL_ERROR = 'error', } +const ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; const PONG_TIMEOUT_IN_MS = 12000; const PING_PONG_INTERVAL_IN_MS = 20000; @@ -250,7 +251,7 @@ export class GraphqlService { * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string * - * Retries errored subscriptions (doesn't throw) with exponential backoff. + * Retries errored subscriptions up to 3 times with exponential backoff. Afterwards throws an error. * * To get notified when any subscriptions have errored, use the {@link haveAnySubscriptionsErrored} method. */ @@ -510,8 +511,14 @@ export class GraphqlService { messageId: message.id, }); - if (!this.erroredSubscriptionsRetryTimeout) { + if ( + this.erroredSubscriptionsRetryCount < + ERRORED_SUBSCRIPTIONS_RETRY_LIMIT && + !this.erroredSubscriptionsRetryTimeout + ) { this.scheduleErroredSubscriptionsRetry(); + } else if (!this.erroredSubscriptionsRetryTimeout) { + this.failErroredSubscriptions(); } break; @@ -626,18 +633,38 @@ export class GraphqlService { } private scheduleErroredSubscriptionsRetry(): void { - const delay = calculateRandomizedExponentialBackoffTime( - this.erroredSubscriptionsRetryCount++, - ); - + const retryCount = this.erroredSubscriptionsRetryCount + 1; + const delay = calculateRandomizedExponentialBackoffTime(retryCount); this.logger.info(`Retry errored subscriptions in ${delay.toFixed(0)}ms`); this.erroredSubscriptionsRetryTimeout = setTimeout(() => { this.retryErroredSubscriptions(); + this.erroredSubscriptionsRetryCount = retryCount; this.erroredSubscriptionsRetryTimeout = null; }, delay); } + private failErroredSubscriptions(): void { + this.logger.error( + `Errored subscriptions retry limit (${ERRORED_SUBSCRIPTIONS_RETRY_LIMIT}) reached, giving up`, + ); + + this.erroredSubscriptionsMessageIds$ + .pipe(take(1)) + .subscribe((messageIds) => { + for (const messageId of messageIds) { + const subscriber = this.messageSubscribers.get(messageId); + this.cleanUpSubscription(messageId); + + subscriber?.error( + new Error( + `Subscription failed after ${this.erroredSubscriptionsRetryCount} retries`, + ), + ); + } + }); + } + private retryErroredSubscriptions(): void { this.erroredSubscriptionsMessageIds$ .pipe(take(1)) From 31eb44e407b07574d7923dc9e535a0c28b4a4f16 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 14 May 2026 22:41:04 +0300 Subject: [PATCH 10/35] Log unhandled promise errors --- .../lib/services/graphql/graphql.service.ts | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 04e3f591..2bcd0973 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -268,12 +268,22 @@ export class GraphqlService { return new Observable((subscriber) => { const messageId = `${++this.subcriptionsCount}`; this.subscriptions.push({ messageId, query }); - this.sendMessage(messageId, MessageType.GQL_START, { query }); + + this.sendMessage(messageId, MessageType.GQL_START, { query }).catch( + (error: Error) => { + this.logger.error('Failed to start subscription: ', error); + }, + ); + this.messageSubscribers.set(messageId, subscriber); return () => { if (this.messageSubscribers.has(messageId)) { - this.sendMessage(messageId, MessageType.GQL_STOP, null); + this.sendMessage(messageId, MessageType.GQL_STOP, null).catch( + (error) => { + this.logger.error('Failed to stop subscription: ', error); + }, + ); } this.cleanUpSubscription(messageId); @@ -412,10 +422,14 @@ export class GraphqlService { this.logger.info(`Reconnect socket in ${timer.toFixed(0)}ms`); - sleepMs(timer).then(() => { - this.connectionAttemptsCount++; - this.openSocket(); - }); + sleepMs(timer) + .then(async () => { + this.connectionAttemptsCount++; + return await this.openSocket(); + }) + .catch((error: Error) => { + this.logger.error('Failed to reconnect socket: ', error); + }); } if (this.connectionStatus === ConnectionStatus.CONNECTING) { @@ -475,7 +489,12 @@ export class GraphqlService { } if (resubscriptionFailed) { - this.handleConnectionDrop(); + this.handleConnectionDrop().catch((error) => { + this.logger.error( + 'Failed to handle connection drop after resubscription failure: ', + error, + ); + }); } break; @@ -597,10 +616,11 @@ export class GraphqlService { } private sendPing(): void { - this.pongTimeout = setTimeout( - () => this.handleConnectionDrop(), - PONG_TIMEOUT_IN_MS, - ); + this.pongTimeout = setTimeout(() => { + this.handleConnectionDrop().catch((error) => { + this.logger.error('Failed to handle pong connection drop: ', error); + }); + }, PONG_TIMEOUT_IN_MS); this.sendRawMessage(JSON.stringify({ type: MessageType.GQL_PING })); } From c9e850b87c5741f2da8a797cf40a44d04d4c3a93 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Fri, 15 May 2026 14:34:03 +0300 Subject: [PATCH 11/35] Improve log --- .../src/lib/services/graphql/graphql.service.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 2bcd0973..a782a49b 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -655,7 +655,10 @@ export class GraphqlService { private scheduleErroredSubscriptionsRetry(): void { const retryCount = this.erroredSubscriptionsRetryCount + 1; const delay = calculateRandomizedExponentialBackoffTime(retryCount); - this.logger.info(`Retry errored subscriptions in ${delay.toFixed(0)}ms`); + + this.logger.info( + `Retry (${retryCount}) errored subscriptions in ${delay.toFixed(0)}ms`, + ); this.erroredSubscriptionsRetryTimeout = setTimeout(() => { this.retryErroredSubscriptions(); From d71deb0c6965486057079d02458faadcad22549c Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 11:20:18 +0300 Subject: [PATCH 12/35] Remove console logs --- .../lib/services/graphql/graphql.service.ts | 41 ++++++++++++++----- 1 file changed, 31 insertions(+), 10 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index a782a49b..3ef49471 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -72,8 +72,10 @@ enum MessageType { } const ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; -const PONG_TIMEOUT_IN_MS = 12000; -const PING_PONG_INTERVAL_IN_MS = 20000; +// To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. +const ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 500; +const PONG_TIMEOUT_IN_MS = 12_000; +const PING_PONG_INTERVAL_IN_MS = 20_000; // https://www.w3.org/TR/websockets/#concept-websocket-close-fail const CLIENT_SIDE_CLOSE_EVENT = 1000; @@ -151,6 +153,10 @@ export class GraphqlService { typeof setTimeout > | null = null; + private erroredSubscriptionsSuccessTimeout: ReturnType< + typeof setTimeout + > | null = null; + private erroredSubscriptionsRetryCount = 0; private temporaryApiKeyService: TemporaryApiKeyService | undefined; @@ -251,7 +257,7 @@ export class GraphqlService { * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string * - * Retries errored subscriptions up to 3 times with exponential backoff. Afterwards throws an error. + * Retries errored subscriptions up to 3 times. Afterwards throws an error. * * To get notified when any subscriptions have errored, use the {@link haveAnySubscriptionsErrored} method. */ @@ -284,6 +290,11 @@ export class GraphqlService { this.logger.error('Failed to stop subscription: ', error); }, ); + + this.erroredSubscriptionsAction$.next({ + type: 'remove', + messageId, + }); } this.cleanUpSubscription(messageId); @@ -347,11 +358,6 @@ export class GraphqlService { } private cleanUpSubscription(messageId: string): void { - this.erroredSubscriptionsAction$.next({ - type: 'remove', - messageId, - }); - this.messageSubscribers.delete(messageId); this.subscriptions = this.subscriptions.filter( @@ -461,7 +467,7 @@ export class GraphqlService { case MessageType.GQL_CONNECTION_ACK: { this.connectionAttemptsCount = 0; - this.clearErroredSubscriptionsRetry(); + this.clearErroredSubscriptionsTimeouts(); this.erroredSubscriptionsRetryCount = 0; this.erroredSubscriptionsAction$.next({ type: 'clear' }); @@ -525,6 +531,8 @@ export class GraphqlService { `GraphQL subscription error: ${JSON.stringify(message)}`, ); + this.clearErroredSubscriptionsSuccessTimeout(); + this.erroredSubscriptionsAction$.next({ type: 'add', messageId: message.id, @@ -647,9 +655,16 @@ export class GraphqlService { clearInterval(this.pingPongInterval); } - private clearErroredSubscriptionsRetry(): void { + private clearErroredSubscriptionsTimeouts(): void { clearTimeout(this.erroredSubscriptionsRetryTimeout ?? undefined); this.erroredSubscriptionsRetryTimeout = null; + + this.clearErroredSubscriptionsSuccessTimeout(); + } + + private clearErroredSubscriptionsSuccessTimeout(): void { + clearTimeout(this.erroredSubscriptionsSuccessTimeout ?? undefined); + this.erroredSubscriptionsSuccessTimeout = null; } private scheduleErroredSubscriptionsRetry(): void { @@ -709,6 +724,12 @@ export class GraphqlService { }), ); } + + this.erroredSubscriptionsSuccessTimeout = setTimeout(() => { + this.erroredSubscriptionsAction$.next({ type: 'clear' }); + this.erroredSubscriptionsRetryCount = 0; + this.erroredSubscriptionsSuccessTimeout = null; + }, ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS); }); } From 182763133d78dd35858adf0d7f893b7e14b9eee8 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 12:47:09 +0300 Subject: [PATCH 13/35] Improve types --- .../src/lib/services/graphql/graphql.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 3ef49471..565d6c2c 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -495,7 +495,7 @@ export class GraphqlService { } if (resubscriptionFailed) { - this.handleConnectionDrop().catch((error) => { + this.handleConnectionDrop().catch((error: Error) => { this.logger.error( 'Failed to handle connection drop after resubscription failure: ', error, @@ -625,7 +625,7 @@ export class GraphqlService { private sendPing(): void { this.pongTimeout = setTimeout(() => { - this.handleConnectionDrop().catch((error) => { + this.handleConnectionDrop().catch((error: Error) => { this.logger.error('Failed to handle pong connection drop: ', error); }); }, PONG_TIMEOUT_IN_MS); From f865ac16d87c6da2f5ea34dc28e3e9f329d982ca Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 15:37:48 +0300 Subject: [PATCH 14/35] Fix unit tests --- .../graphql-subscriptions-fixture.ts | 22 +- .../__tests__/graphql-subscriptions.spec.ts | 592 ++++++++++++++---- .../lib/services/graphql/graphql.service.ts | 18 +- 3 files changed, 476 insertions(+), 156 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts index 4534ac39..821bf382 100644 --- a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts +++ b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts @@ -1,9 +1,10 @@ /* eslint-env jest */ -import { DocumentNode } from 'graphql'; +import { DocumentNode, print } from 'graphql'; import WS from 'jest-websocket-mock'; import { DeserializedMessage } from 'jest-websocket-mock/lib/websocket'; -import { lastValueFrom, Observer, Subscription, take } from 'rxjs'; +import { lastValueFrom, Observer, Subscriber, Subscription, take } from 'rxjs'; + import { ConnectionStatus } from '../../../model/connection-status'; import { GraphqlService } from '../graphql.service'; @@ -23,7 +24,7 @@ export class GraphQLSubscriptionsFixture { .mockReturnValue(SERVER_URL); jest - .spyOn(this.graphqlService as any, 'fetchTemporaryApiKey') + .spyOn(this.graphqlService as any, 'getTemporaryApiKey') .mockResolvedValue(DUMMY_API_KEY); } @@ -40,12 +41,16 @@ export class GraphQLSubscriptionsFixture { return (this.graphqlService as any).subscriptions.length; } - getMessageSubscribersSize(): number { - return (this.graphqlService as any).messageSubscribers.size; + getMessagesSubscribers(): Map>> { + return this.graphqlService['messagesSubscribers']; + } + + getSubscribedMessagesCount(): number { + return this.graphqlService['messagesSubscribers'].size; } - hasMessageSubscriber(id: string): boolean { - return (this.graphqlService as any).messageSubscribers.has(id); + hasMessageSubscribers(messageId: string): boolean { + return this.graphqlService['messagesSubscribers'].has(messageId); } async waitForConnection() { @@ -103,7 +108,7 @@ export class GraphQLSubscriptionsFixture { expect(await this.server.nextMessage).toEqual({ id: '1', type: 'start', - payload: { query }, + payload: { query: typeof query === 'string' ? query : print(query) }, }); } @@ -125,7 +130,6 @@ export class GraphQLSubscriptionsFixture { } async cleanup() { - (this.graphqlService as any).clearSubscriptionRetry(); WS.clean(); await this.server.closed; } diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 22c16641..7eb97ab9 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -1,6 +1,8 @@ import gql from 'graphql-tag'; import fetchMock from 'jest-fetch-mock'; import { WebSocket } from 'mock-socket'; +import { firstValueFrom } from 'rxjs'; + import { ConnectionStatus } from '../../../model/connection-status'; import { GraphQLSubscriptionsFixture } from '../__fixtures__/graphql-subscriptions-fixture'; @@ -94,20 +96,24 @@ describe('GraphQL subscriptions', () => { }); it('cleans up internal state when unsubscribing', async () => { - expect(fixture.getMessageSubscribersSize()).toBe(0); + expect(fixture.getSubscribedMessagesCount()).toBe(0); + const subscription = fixture.triggerSubscription(); await fixture.handleConnectionInit(); await fixture.consumeSubscribeMessage(); - expect(fixture.getMessageSubscribersSize()).toBe(1); - expect(fixture.hasMessageSubscriber('1')).toBe(true); + + expect(fixture.getSubscribedMessagesCount()).toBe(1); + expect(fixture.hasMessageSubscribers('1')).toBe(true); subscription.unsubscribe(); await fixture.consumeAnyMessage(); - expect(fixture.getMessageSubscribersSize()).toBe(0); + + expect(fixture.getSubscribedMessagesCount()).toBe(0); }); it('when receiving a published message for a subscription that does not exist anymore, it does not throw', async () => { - expect(fixture.getMessageSubscribersSize()).toBe(0); + expect(fixture.getSubscribedMessagesCount()).toBe(0); + const subscription = fixture.triggerSubscription(); await fixture.handleConnectionInit(); @@ -144,7 +150,7 @@ describe('GraphQL subscriptions', () => { it('when the server does not reply to ping message, reconnects', async () => { const reconnectSpy = jest.spyOn( fixture.graphqlService as any, - 'handleConnectionDropWithThisBound', + 'handleConnectionDrop', ); jest.useFakeTimers(); const subscription = fixture.triggerSubscription(); @@ -169,7 +175,7 @@ describe('GraphQL subscriptions', () => { it('handles multiple consecutive connect/ping/timeout cycles gracefully', async () => { const reconnectSpy = jest.spyOn( fixture.graphqlService as any, - 'handleConnectionDropWithThisBound', + 'handleConnectionDrop', ); jest.useFakeTimers(); const subscription = fixture.triggerSubscription(); @@ -215,7 +221,7 @@ describe('GraphQL subscriptions', () => { it('when the server replies to ping message, does not reconnect', async () => { const reconnectSpy = jest.spyOn( fixture.graphqlService as any, - 'handleConnectionDropWithThisBound', + 'handleConnectionDrop', ); jest.useFakeTimers(); const subscription = fixture.triggerSubscription(); @@ -329,193 +335,503 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it('GQL_ERROR does not kill the subscription or trigger reconnect', async () => { - const reconnectSpy = jest.spyOn( - fixture.graphqlService as any, - 'handleConnectionDrop', - ); - const errorSpy = jest.fn(); - const subscription = fixture.triggerSubscription('subscription { baba }', { - error: errorSpy, - }); + it(`should retry errored subscriptions if socket reconnects`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(); + await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', type: 'error', payload: { data: null, - errors: [{ message: 'Subscription limit reached' }], + errors: ['The maximum subscription limit of 100 has been reached'], }, }); - await new Promise((r) => setTimeout(r, 10)); + await fixture.closeWithCode(1001); - expect(errorSpy).not.toHaveBeenCalled(); - expect(reconnectSpy).not.toHaveBeenCalled(); - expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(1); - expect(fixture.hasMessageSubscriber('1')).toBe(true); + fixture.openServer(); + await fixture.handleConnectionInit(); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '1', + type: 'start', + }), + ); subscription.unsubscribe(); }); - it('GQL_ERROR emits true on the subscription error observable', async () => { - const values: boolean[] = []; - fixture.graphqlService - .getSubscriptionErrorObservable() - .subscribe((v) => values.push(v)); + describe('error', () => { + it(`shouldn't error the subscription`, async () => { + const subscriptionErrorSpy = jest.fn(); - const subscription = fixture.triggerSubscription('subscription { baba }'); - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(); + const query = gql` + subscription { + name + } + `; - fixture.server.send({ - id: '1', - type: 'error', - payload: { - data: null, - errors: [ - { - message: - 'The maximum subscription limit of 100 has been reached', - }, - ], - }, + const subscription = fixture.triggerSubscription(query, { + error: subscriptionErrorSpy, + }); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + expect(subscriptionErrorSpy).not.toHaveBeenCalled(); + + subscription.unsubscribe(); }); - await new Promise((r) => setTimeout(r, 10)); + it(`shouldn't clean up the subscription`, async () => { + const query = gql` + subscription { + name + } + `; - expect(values).toEqual([false, true]); + const subscription = fixture.triggerSubscription(query); - subscription.unsubscribe(); - }); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); - it('retries failed subscriptions after delay and clears error state', async () => { - (fixture.graphqlService as any).subscriptionRetryDelayMs = 50; + expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1']); - const values: boolean[] = []; - fixture.graphqlService - .getSubscriptionErrorObservable() - .subscribe((v) => values.push(v)); + subscription.unsubscribe(); + }); - const subscription = fixture.triggerSubscription('subscription { baba }'); - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(); + it(`shouldn't drop connection`, async () => { + const connectionDropSpy = jest.spyOn( + fixture.graphqlService as any, + 'handleConnectionDrop', + ); - fixture.server.send({ - id: '1', - type: 'error', - payload: { - data: null, - errors: [{ message: 'Limit reached' }], - }, + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + expect(connectionDropSpy).not.toHaveBeenCalled(); + + subscription.unsubscribe(); }); - await new Promise((r) => setTimeout(r, 10)); - expect(values).toEqual([false, true]); + it('should retry errored subscriptions after delay', async () => { + jest.useFakeTimers(); - await new Promise((r) => setTimeout(r, 60)); + const query = gql` + subscription { + name + } + `; - expect(values).toEqual([false, true, false]); - expect(await fixture.getNextMessage()).toEqual({ - id: '1', - type: 'start', - payload: { query: 'subscription { baba }' }, + const subscription = fixture.triggerSubscription(query); + + // Wait for temporary api key + await jest.runAllTimersAsync(); + + await fixture.handleConnectionInit(); + + // Send subscriptions start messages + await jest.runOnlyPendingTimersAsync(); + + await fixture.consumeSubscribeMessage(query); + + // Send ping message + await jest.advanceTimersToNextTimerAsync(); + + await fixture.consumePingMessage(); + fixture.sendMessageToClient({ type: 'pong' }); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + // Get latest haveAnySubscriptionsErrored state + await jest.advanceTimersByTimeAsync(0); + + const haveAnySubscriptionsErroredAfterError = await firstValueFrom( + fixture.graphqlService.haveAnySubscriptionsErrored(), + ); + + expect(haveAnySubscriptionsErroredAfterError).toBe(true); + + // Wait for retry + await jest.advanceTimersByTimeAsync(7_000); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '1', + type: 'start', + }), + ); + + subscription.unsubscribe(); + + jest.useRealTimers(); }); - subscription.unsubscribe(); - }); + it(`should error subscription if server sends unknown message (has data)`, async () => { + const subscriptionErrorSpy = jest.fn(); - it('does not send GQL_STOP when server sends GQL_COMPLETE', async () => { - const completeSpy = jest.fn(); - const subscription = fixture.triggerSubscription('subscription { baba }', { - next: () => {}, - complete: completeSpy, + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query, { + error: subscriptionErrorSpy, + }); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'unknown', + payload: { data: { unknown: 'unknown' } }, + }); + + expect(subscriptionErrorSpy).toHaveBeenCalledWith({ unknown: 'unknown' }); + + subscription.unsubscribe(); }); - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(); - fixture.sendMessageToClient({ - id: '1', - type: 'complete', + it(`should clean up subscription if server sends unknown message (has data)`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query, { + error: () => {}, + }); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'unknown', + payload: { data: { unknown: 'unknown' } }, + }); + + expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + + subscription.unsubscribe(); }); - await new Promise((r) => setTimeout(r, 10)); + it(`should error subscription if server sends unknown message (has errors)`, async () => { + const subscriptionErrorSpy = jest.fn(); - expect(completeSpy).toHaveBeenCalled(); - expect(fixture.hasMessageSubscriber('1')).toBe(false); - expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(0); - expect(fixture.server.messagesToConsume.pendingItems).toHaveLength(0); + const query = gql` + subscription { + name + } + `; - subscription.unsubscribe(); + const subscription = fixture.triggerSubscription(query, { + error: subscriptionErrorSpy, + }); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'unknown', + payload: { + errors: [{ message: 'Something went wrong' }], + }, + }); + + expect(subscriptionErrorSpy).toHaveBeenCalledWith([ + { message: 'Something went wrong' }, + ]); + + subscription.unsubscribe(); + }); + + it(`should clean up subscription if server sends unknown message (has errors)`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query, { + error: () => {}, + }); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'unknown', + payload: { + errors: [{ message: 'Something went wrong' }], + }, + }); + + expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + + subscription.unsubscribe(); + }); }); - it('GQL_ERROR keeps subscription tracked so it re-subscribes on natural reconnect and clears error state', async () => { - const values: boolean[] = []; - fixture.graphqlService - .getSubscriptionErrorObservable() - .subscribe((v) => values.push(v)); + describe('complete', () => { + it(`should complete subscription`, async () => { + const subscriptionCompleteSpy = jest.fn(); - const subscription = fixture.triggerSubscription('subscription { baba }'); - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(); + const query = gql` + subscription { + name + } + `; - fixture.sendMessageToClient({ - id: '1', - type: 'error', - payload: { - data: null, - errors: [{ message: 'Limit reached' }], - }, + const subscription = fixture.triggerSubscription(query, { + complete: subscriptionCompleteSpy, + }); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', + }); + + expect(subscriptionCompleteSpy).toHaveBeenCalled(); + + subscription.unsubscribe(); }); - await new Promise((r) => setTimeout(r, 10)); + it(`should clean up subscription`, async () => { + const query = gql` + subscription { + name + } + `; - expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(1); - expect(fixture.hasMessageSubscriber('1')).toBe(true); - expect(values).toEqual([false, true]); + const subscription = fixture.triggerSubscription(query); - await fixture.closeWithCode(1001); - fixture.openServer(); - await fixture.handleConnectionInit(); - expect(await fixture.getNextMessage()).toEqual({ - id: '1', - type: 'start', - payload: { query: 'subscription { baba }' }, + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', + }); + + expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + + subscription.unsubscribe(); }); - expect(values).toEqual([false, true, false]); + it(`shouldn't send GQL_STOP if subscription completes`, async () => { + const query = gql` + subscription { + name + } + `; - subscription.unsubscribe(); + const subscription = fixture.triggerSubscription(query); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', + }); + + expect(fixture.server.messagesToConsume.pendingItems).toHaveLength(0); + + subscription.unsubscribe(); + }); }); - it('cleans up subscription on unknown message type with errors', async () => { - const errorSpy = jest.fn(); - const subscription = fixture.triggerSubscription('subscription { baba }', { - error: errorSpy, + describe('haveAnySubscriptionsErrored', () => { + it(`should emit 'true' if subscription errors`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + const haveAnySubscriptionsErrored = await firstValueFrom( + fixture.graphqlService.haveAnySubscriptionsErrored(), + ); + + expect(haveAnySubscriptionsErrored).toBe(true); + + subscription.unsubscribe(); }); - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(); - fixture.sendMessageToClient({ - id: '1', - type: 'unknown_type', - payload: { - errors: [{ message: 'Something went wrong' }], - }, + it('should clear errored subscriptions with a delay after successful retry', async () => { + jest.useFakeTimers(); + + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + + // Wait for temporary api key + await jest.runAllTimersAsync(); + + await fixture.handleConnectionInit(); + + // Send subscriptions start messages + await jest.runOnlyPendingTimersAsync(); + + await fixture.consumeSubscribeMessage(query); + + // Send ping message + await jest.advanceTimersToNextTimerAsync(); + + await fixture.consumePingMessage(); + fixture.sendMessageToClient({ type: 'pong' }); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + // Get latest haveAnySubscriptionsErrored state + await jest.advanceTimersByTimeAsync(0); + + const haveAnySubscriptionsErroredAfterError = await firstValueFrom( + fixture.graphqlService.haveAnySubscriptionsErrored(), + ); + + expect(haveAnySubscriptionsErroredAfterError).toBe(true); + + // Wait for retry + await jest.advanceTimersByTimeAsync(7_000); + + const haveAnySubscriptionsErroredAfterRetry = await firstValueFrom( + fixture.graphqlService.haveAnySubscriptionsErrored(), + ); + + expect(haveAnySubscriptionsErroredAfterRetry).toBe(false); + + subscription.unsubscribe(); + + jest.useRealTimers(); }); - await new Promise((r) => setTimeout(r, 10)); + it(`should emit 'true' if there are errored subscriptions but socket reconnects`, async () => { + const query = gql` + subscription { + name + } + `; - expect(errorSpy).toHaveBeenCalledWith( - expect.objectContaining({ message: 'Something went wrong' }), - ); - expect(errorSpy.mock.calls[0][0]).toBeInstanceOf(Error); - expect(fixture.getGraphqlServiceActiveSubscriptionCount()).toBe(0); - expect(fixture.hasMessageSubscriber('1')).toBe(false); + const subscription = fixture.triggerSubscription(query); - subscription.unsubscribe(); + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + const haveAnySubscriptionsErroredBeforeReconnect = await firstValueFrom( + fixture.graphqlService.haveAnySubscriptionsErrored(), + ); + + expect(haveAnySubscriptionsErroredBeforeReconnect).toBe(true); + + await fixture.closeWithCode(1001); + + fixture.openServer(); + await fixture.handleConnectionInit(); + + const haveAnySubscriptionsErroredAfterReconnect = await firstValueFrom( + fixture.graphqlService.haveAnySubscriptionsErrored(), + ); + + expect(haveAnySubscriptionsErroredAfterReconnect).toBe(false); + + subscription.unsubscribe(); + }); }); describe('WebSocket readyState guards', () => { diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 565d6c2c..26f9453d 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -103,7 +103,7 @@ export class GraphqlService { private subscriptions: Subscription[] = []; - private readonly messageSubscribers = new Map< + private readonly messagesSubscribers = new Map< string, Subscriber> >(); @@ -281,10 +281,10 @@ export class GraphqlService { }, ); - this.messageSubscribers.set(messageId, subscriber); + this.messagesSubscribers.set(messageId, subscriber); return () => { - if (this.messageSubscribers.has(messageId)) { + if (this.messagesSubscribers.has(messageId)) { this.sendMessage(messageId, MessageType.GQL_STOP, null).catch( (error) => { this.logger.error('Failed to stop subscription: ', error); @@ -358,7 +358,7 @@ export class GraphqlService { } private cleanUpSubscription(messageId: string): void { - this.messageSubscribers.delete(messageId); + this.messagesSubscribers.delete(messageId); this.subscriptions = this.subscriptions.filter( (subscription) => subscription.messageId !== messageId, @@ -512,11 +512,11 @@ export class GraphqlService { messageId: message.id, }); - this.messageSubscribers.get(message.id)?.next(message.payload.data); + this.messagesSubscribers.get(message.id)?.next(message.payload.data); break; case MessageType.GQL_COMPLETE: { - const subscriber = this.messageSubscribers.get(message.id); + const subscriber = this.messagesSubscribers.get(message.id); this.cleanUpSubscription(message.id); subscriber?.complete(); break; @@ -551,7 +551,7 @@ export class GraphqlService { break; default: { - const subscriber = this.messageSubscribers.get(message.id); + const subscriber = this.messagesSubscribers.get(message.id); if (!subscriber) { return; } @@ -570,7 +570,7 @@ export class GraphqlService { private shouldRetry(event: CloseEvent): boolean { return ( - event.code !== CLIENT_SIDE_CLOSE_EVENT || !!this.messageSubscribers.size + event.code !== CLIENT_SIDE_CLOSE_EVENT || !!this.messagesSubscribers.size ); } @@ -691,7 +691,7 @@ export class GraphqlService { .pipe(take(1)) .subscribe((messageIds) => { for (const messageId of messageIds) { - const subscriber = this.messageSubscribers.get(messageId); + const subscriber = this.messagesSubscribers.get(messageId); this.cleanUpSubscription(messageId); subscriber?.error( From 48944a7c14dbc665990af0322770e4a4fedd055e Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 15:42:54 +0300 Subject: [PATCH 15/35] Improve test descriptions --- .../graphql/__tests__/graphql-subscriptions.spec.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 7eb97ab9..e9e24741 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -372,7 +372,7 @@ describe('GraphQL subscriptions', () => { }); describe('error', () => { - it(`shouldn't error the subscription`, async () => { + it(`shouldn't error subscriptions before retrying`, async () => { const subscriptionErrorSpy = jest.fn(); const query = gql` @@ -402,7 +402,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`shouldn't clean up the subscription`, async () => { + it(`shouldn't clean up subscriptions before retrying`, async () => { const query = gql` subscription { name @@ -428,7 +428,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`shouldn't drop connection`, async () => { + it(`shouldn't drop socket connection`, async () => { const connectionDropSpy = jest.spyOn( fixture.graphqlService as any, 'handleConnectionDrop', @@ -703,7 +703,7 @@ describe('GraphQL subscriptions', () => { }); describe('haveAnySubscriptionsErrored', () => { - it(`should emit 'true' if subscription errors`, async () => { + it(`should emit 'true' if any subscriptions error`, async () => { const query = gql` subscription { name @@ -733,7 +733,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it('should clear errored subscriptions with a delay after successful retry', async () => { + it('should clear errored subscriptions with a delay after successful batch retry', async () => { jest.useFakeTimers(); const query = gql` From 1262507293ffb12f6ad9809e62fa036811decac0 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 16:01:30 +0300 Subject: [PATCH 16/35] Add tests --- .../__tests__/graphql-subscriptions.spec.ts | 87 ++++++++++++++++++- 1 file changed, 85 insertions(+), 2 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index e9e24741..99d01d26 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -371,7 +371,90 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - describe('error', () => { + it(`should send GQL_STOP for errored subscription if it's unsubscribed`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1']); + + subscription.unsubscribe(); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '1', + type: 'stop', + }), + ); + }); + + it(`should not send GQL_STOP for subscription if it has been cleaned up`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'complete', + }); + + expect(fixture.server.messagesToConsume.pendingItems).toHaveLength(0); + + subscription.unsubscribe(); + }); + + it(`should clean up errored subscription if it's unsubscribed`, async () => { + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query); + + await fixture.handleConnectionInit(); + await fixture.consumeSubscribeMessage(query); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1']); + + subscription.unsubscribe(); + + expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + }); + + describe('GQL_ERROR', () => { it(`shouldn't error subscriptions before retrying`, async () => { const subscriptionErrorSpy = jest.fn(); @@ -630,7 +713,7 @@ describe('GraphQL subscriptions', () => { }); }); - describe('complete', () => { + describe('GQL_COMPLETE', () => { it(`should complete subscription`, async () => { const subscriptionCompleteSpy = jest.fn(); From 87cc5001a5ffca3c59dff36ab48e9b0e51f9cd84 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 16:57:40 +0300 Subject: [PATCH 17/35] Improve tests --- .../graphql-subscriptions-fixture.ts | 3 +- .../__tests__/graphql-subscriptions.spec.ts | 353 ++++++++++-------- 2 files changed, 197 insertions(+), 159 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts index 821bf382..1312a9c3 100644 --- a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts +++ b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts @@ -104,9 +104,10 @@ export class GraphQLSubscriptionsFixture { async consumeSubscribeMessage( query: DocumentNode | string = 'subscription { baba }', + { id }: { readonly id: string } = { id: '1' }, ) { expect(await this.server.nextMessage).toEqual({ - id: '1', + id, type: 'start', payload: { query: typeof query === 'string' ? query : print(query) }, }); diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 99d01d26..2a2f1c7b 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -336,16 +336,19 @@ describe('GraphQL subscriptions', () => { }); it(`should retry errored subscriptions if socket reconnects`, async () => { - const query = gql` + const subscription1 = fixture.triggerSubscription(gql` subscription { name } - `; + `); - const subscription = fixture.triggerSubscription(query); + const subscription2 = fixture.triggerSubscription(gql` + subscription { + name2 + } + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -356,6 +359,15 @@ describe('GraphQL subscriptions', () => { }, }); + fixture.server.send({ + id: '2', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + await fixture.closeWithCode(1001); fixture.openServer(); @@ -368,7 +380,15 @@ describe('GraphQL subscriptions', () => { }), ); - subscription.unsubscribe(); + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '2', + type: 'start', + }), + ); + + subscription1.unsubscribe(); + subscription2.unsubscribe(); }); it(`should send GQL_STOP for errored subscription if it's unsubscribed`, async () => { @@ -405,16 +425,13 @@ describe('GraphQL subscriptions', () => { }); it(`should not send GQL_STOP for subscription if it has been cleaned up`, async () => { - const query = gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; - - const subscription = fixture.triggerSubscription(query); + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -427,16 +444,13 @@ describe('GraphQL subscriptions', () => { }); it(`should clean up errored subscription if it's unsubscribed`, async () => { - const query = gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; - - const subscription = fixture.triggerSubscription(query); + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -454,22 +468,111 @@ describe('GraphQL subscriptions', () => { expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); }); - describe('GQL_ERROR', () => { - it(`shouldn't error subscriptions before retrying`, async () => { - const subscriptionErrorSpy = jest.fn(); + describe('GQL_DATA', () => { + it('should send data to subscriber', async () => { + const subscriptionNextSpy = jest.fn(); + + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { next: subscriptionNextSpy }, + ); - const query = gql` + await fixture.handleConnectionInit(); + + fixture.sendMessageToClient({ + id: '1', + type: 'data', + payload: { data: { id: '1' } }, + }); + + expect(subscriptionNextSpy).toHaveBeenCalledWith({ id: '1' }); + + subscription.unsubscribe(); + }); + }); + + describe('GQL_COMPLETE', () => { + it(`should complete subscription`, async () => { + const subscriptionCompleteSpy = jest.fn(); + + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { complete: subscriptionCompleteSpy }, + ); + + await fixture.handleConnectionInit(); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', + }); + + expect(subscriptionCompleteSpy).toHaveBeenCalled(); + + subscription.unsubscribe(); + }); + + it(`should clean up subscription`, async () => { + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; + `); - const subscription = fixture.triggerSubscription(query, { - error: subscriptionErrorSpy, + await fixture.handleConnectionInit(); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', }); + expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + + subscription.unsubscribe(); + }); + + it(`shouldn't send GQL_STOP if subscription completes`, async () => { + const subscription = fixture.triggerSubscription(gql` + subscription { + name + } + `); + + await fixture.handleConnectionInit(); + + fixture.sendMessageToClient({ + id: '1', + type: 'complete', + }); + + expect(fixture.server.messagesToConsume.pendingItems).toHaveLength(0); + + subscription.unsubscribe(); + }); + }); + + describe('GQL_ERROR', () => { + it(`shouldn't error subscriptions before retrying`, async () => { + const subscriptionErrorSpy = jest.fn(); + + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: subscriptionErrorSpy }, + ); + await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -486,16 +589,13 @@ describe('GraphQL subscriptions', () => { }); it(`shouldn't clean up subscriptions before retrying`, async () => { - const query = gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; - - const subscription = fixture.triggerSubscription(query); + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -517,16 +617,13 @@ describe('GraphQL subscriptions', () => { 'handleConnectionDrop', ); - const query = gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; - - const subscription = fixture.triggerSubscription(query); + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -553,6 +650,14 @@ describe('GraphQL subscriptions', () => { const subscription = fixture.triggerSubscription(query); + const query2 = gql` + subscription { + name2 + } + `; + + const subscription2 = fixture.triggerSubscription(query2); + // Wait for temporary api key await jest.runAllTimersAsync(); @@ -561,7 +666,8 @@ describe('GraphQL subscriptions', () => { // Send subscriptions start messages await jest.runOnlyPendingTimersAsync(); - await fixture.consumeSubscribeMessage(query); + await fixture.consumeSubscribeMessage(query, { id: '1' }); + await fixture.consumeSubscribeMessage(query2, { id: '2' }); // Send ping message await jest.advanceTimersToNextTimerAsync(); @@ -578,6 +684,15 @@ describe('GraphQL subscriptions', () => { }, }); + fixture.server.send({ + id: '2', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + // Get latest haveAnySubscriptionsErrored state await jest.advanceTimersByTimeAsync(0); @@ -597,7 +712,15 @@ describe('GraphQL subscriptions', () => { }), ); + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '2', + type: 'start', + }), + ); + subscription.unsubscribe(); + subscription2.unsubscribe(); jest.useRealTimers(); }); @@ -605,18 +728,16 @@ describe('GraphQL subscriptions', () => { it(`should error subscription if server sends unknown message (has data)`, async () => { const subscriptionErrorSpy = jest.fn(); - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query, { - error: subscriptionErrorSpy, - }); + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: subscriptionErrorSpy }, + ); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.sendMessageToClient({ id: '1', @@ -630,18 +751,16 @@ describe('GraphQL subscriptions', () => { }); it(`should clean up subscription if server sends unknown message (has data)`, async () => { - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query, { - error: () => {}, - }); + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: () => {} }, + ); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.sendMessageToClient({ id: '1', @@ -657,18 +776,16 @@ describe('GraphQL subscriptions', () => { it(`should error subscription if server sends unknown message (has errors)`, async () => { const subscriptionErrorSpy = jest.fn(); - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query, { - error: subscriptionErrorSpy, - }); + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: subscriptionErrorSpy }, + ); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.sendMessageToClient({ id: '1', @@ -686,18 +803,16 @@ describe('GraphQL subscriptions', () => { }); it(`should clean up subscription if server sends unknown message (has errors)`, async () => { - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query, { - error: () => {}, - }); + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: () => {} }, + ); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.sendMessageToClient({ id: '1', @@ -713,90 +828,15 @@ describe('GraphQL subscriptions', () => { }); }); - describe('GQL_COMPLETE', () => { - it(`should complete subscription`, async () => { - const subscriptionCompleteSpy = jest.fn(); - - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query, { - complete: subscriptionCompleteSpy, - }); - - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); - - fixture.sendMessageToClient({ - id: '1', - type: 'complete', - }); - - expect(subscriptionCompleteSpy).toHaveBeenCalled(); - - subscription.unsubscribe(); - }); - - it(`should clean up subscription`, async () => { - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query); - - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); - - fixture.sendMessageToClient({ - id: '1', - type: 'complete', - }); - - expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); - - subscription.unsubscribe(); - }); - - it(`shouldn't send GQL_STOP if subscription completes`, async () => { - const query = gql` - subscription { - name - } - `; - - const subscription = fixture.triggerSubscription(query); - - await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); - - fixture.sendMessageToClient({ - id: '1', - type: 'complete', - }); - - expect(fixture.server.messagesToConsume.pendingItems).toHaveLength(0); - - subscription.unsubscribe(); - }); - }); - describe('haveAnySubscriptionsErrored', () => { it(`should emit 'true' if any subscriptions error`, async () => { - const query = gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; - - const subscription = fixture.triggerSubscription(query); + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', @@ -855,11 +895,11 @@ describe('GraphQL subscriptions', () => { // Get latest haveAnySubscriptionsErrored state await jest.advanceTimersByTimeAsync(0); - const haveAnySubscriptionsErroredAfterError = await firstValueFrom( + const haveAnySubscriptionsErroredBeforeRetry = await firstValueFrom( fixture.graphqlService.haveAnySubscriptionsErrored(), ); - expect(haveAnySubscriptionsErroredAfterError).toBe(true); + expect(haveAnySubscriptionsErroredBeforeRetry).toBe(true); // Wait for retry await jest.advanceTimersByTimeAsync(7_000); @@ -876,16 +916,13 @@ describe('GraphQL subscriptions', () => { }); it(`should emit 'true' if there are errored subscriptions but socket reconnects`, async () => { - const query = gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } - `; - - const subscription = fixture.triggerSubscription(query); + `); await fixture.handleConnectionInit(); - await fixture.consumeSubscribeMessage(query); fixture.server.send({ id: '1', From 130df8d6845e0c634c571993cab758902e607a8c Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 17:23:14 +0300 Subject: [PATCH 18/35] Add unit tests --- .../graphql-subscriptions-fixture.ts | 8 - .../__tests__/graphql-subscriptions.spec.ts | 199 +++++++++++++++++- 2 files changed, 190 insertions(+), 17 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts index 1312a9c3..42969500 100644 --- a/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts +++ b/packages/javascript-api/src/lib/services/graphql/__fixtures__/graphql-subscriptions-fixture.ts @@ -45,14 +45,6 @@ export class GraphQLSubscriptionsFixture { return this.graphqlService['messagesSubscribers']; } - getSubscribedMessagesCount(): number { - return this.graphqlService['messagesSubscribers'].size; - } - - hasMessageSubscribers(messageId: string): boolean { - return this.graphqlService['messagesSubscribers'].has(messageId); - } - async waitForConnection() { await this.server.connected; } diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 2a2f1c7b..cd205678 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -96,23 +96,22 @@ describe('GraphQL subscriptions', () => { }); it('cleans up internal state when unsubscribing', async () => { - expect(fixture.getSubscribedMessagesCount()).toBe(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); const subscription = fixture.triggerSubscription(); await fixture.handleConnectionInit(); await fixture.consumeSubscribeMessage(); - expect(fixture.getSubscribedMessagesCount()).toBe(1); - expect(fixture.hasMessageSubscribers('1')).toBe(true); + expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1']); subscription.unsubscribe(); await fixture.consumeAnyMessage(); - expect(fixture.getSubscribedMessagesCount()).toBe(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); }); it('when receiving a published message for a subscription that does not exist anymore, it does not throw', async () => { - expect(fixture.getSubscribedMessagesCount()).toBe(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); const subscription = fixture.triggerSubscription(); @@ -465,7 +464,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); - expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); }); describe('GQL_DATA', () => { @@ -534,7 +533,7 @@ describe('GraphQL subscriptions', () => { type: 'complete', }); - expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); subscription.unsubscribe(); }); @@ -725,6 +724,188 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); + it('should error subscription after 3 failed retries', async () => { + jest.useFakeTimers(); + + const subscriptionErrorSpy = jest.fn(); + + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query, { + error: subscriptionErrorSpy, + }); + + const query2 = gql` + subscription { + name2 + } + `; + + const subscription2ErrorSpy = jest.fn(); + + const subscription2 = fixture.triggerSubscription(query2, { + error: subscription2ErrorSpy, + }); + + // Wait for temporary api key + await jest.runAllTimersAsync(); + + await fixture.handleConnectionInit(); + + // Send subscriptions start messages + await jest.runOnlyPendingTimersAsync(); + + await fixture.consumeSubscribeMessage(query, { id: '1' }); + await fixture.consumeSubscribeMessage(query2, { id: '2' }); + + // Send ping message + await jest.advanceTimersToNextTimerAsync(); + + await fixture.consumePingMessage(); + fixture.sendMessageToClient({ type: 'pong' }); + + const sendErrorMessages = (): void => { + [{ messageId: '1' }, { messageId: '2' }].forEach(({ messageId }) => { + fixture.server.send({ + id: messageId, + type: 'error', + payload: { + data: null, + errors: [ + 'The maximum subscription limit of 100 has been reached', + ], + }, + }); + }); + }; + + sendErrorMessages(); + + for (let retryCount = 1; retryCount <= 3; retryCount++) { + // Wait for retry + await jest.advanceTimersToNextTimerAsync(); + + // mock-socket delivers client→server messages via setTimeout(4) + await jest.advanceTimersByTimeAsync(10); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ id: '1', type: 'start' }), + ); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ id: '2', type: 'start' }), + ); + + sendErrorMessages(); + } + + expect(subscriptionErrorSpy).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Subscription failed after 3 retries', + }), + ); + + expect(subscription2ErrorSpy).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Subscription failed after 3 retries', + }), + ); + + subscription.unsubscribe(); + subscription2.unsubscribe(); + + jest.useRealTimers(); + }); + + it('should clean up errored subscription after 3 failed retries', async () => { + jest.useFakeTimers(); + + const query = gql` + subscription { + name + } + `; + + const subscription = fixture.triggerSubscription(query, { + error: () => {}, + }); + + const query2 = gql` + subscription { + name2 + } + `; + + const subscription2 = fixture.triggerSubscription(query2, { + error: () => {}, + }); + + // Wait for temporary api key + await jest.runAllTimersAsync(); + + await fixture.handleConnectionInit(); + + // Send subscriptions start messages + await jest.runOnlyPendingTimersAsync(); + + await fixture.consumeSubscribeMessage(query, { id: '1' }); + await fixture.consumeSubscribeMessage(query2, { id: '2' }); + + // Send ping message + await jest.advanceTimersToNextTimerAsync(); + + await fixture.consumePingMessage(); + fixture.sendMessageToClient({ type: 'pong' }); + + const sendErrorMessages = (): void => { + [{ messageId: '1' }, { messageId: '2' }].forEach(({ messageId }) => { + fixture.server.send({ + id: messageId, + type: 'error', + payload: { + data: null, + errors: [ + 'The maximum subscription limit of 100 has been reached', + ], + }, + }); + }); + }; + + sendErrorMessages(); + + expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1', '2']); + + for (let retryCount = 1; retryCount <= 3; retryCount++) { + // Wait for retry + await jest.advanceTimersToNextTimerAsync(); + + // mock-socket delivers client → server messages via setTimeout(4) + await jest.advanceTimersByTimeAsync(10); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ id: '1', type: 'start' }), + ); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ id: '2', type: 'start' }), + ); + + sendErrorMessages(); + } + + expect(fixture.getMessagesSubscribers().size).toBe(0); + + subscription.unsubscribe(); + subscription2.unsubscribe(); + + jest.useRealTimers(); + }); + it(`should error subscription if server sends unknown message (has data)`, async () => { const subscriptionErrorSpy = jest.fn(); @@ -768,7 +949,7 @@ describe('GraphQL subscriptions', () => { payload: { data: { unknown: 'unknown' } }, }); - expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); subscription.unsubscribe(); }); @@ -822,7 +1003,7 @@ describe('GraphQL subscriptions', () => { }, }); - expect([...fixture.getMessagesSubscribers().keys()]).toHaveLength(0); + expect(fixture.getMessagesSubscribers().size).toBe(0); subscription.unsubscribe(); }); From 1511da7818ee59da01e9fcaf002f06b4edcde936 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 17:36:22 +0300 Subject: [PATCH 19/35] Move tests --- .../__tests__/graphql-subscriptions.spec.ts | 120 +++++++++--------- 1 file changed, 62 insertions(+), 58 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index cd205678..2312d898 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -334,62 +334,6 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`should retry errored subscriptions if socket reconnects`, async () => { - const subscription1 = fixture.triggerSubscription(gql` - subscription { - name - } - `); - - const subscription2 = fixture.triggerSubscription(gql` - subscription { - name2 - } - `); - - await fixture.handleConnectionInit(); - - fixture.server.send({ - id: '1', - type: 'error', - payload: { - data: null, - errors: ['The maximum subscription limit of 100 has been reached'], - }, - }); - - fixture.server.send({ - id: '2', - type: 'error', - payload: { - data: null, - errors: ['The maximum subscription limit of 100 has been reached'], - }, - }); - - await fixture.closeWithCode(1001); - - fixture.openServer(); - await fixture.handleConnectionInit(); - - expect(await fixture.getNextMessage()).toEqual( - expect.objectContaining({ - id: '1', - type: 'start', - }), - ); - - expect(await fixture.getNextMessage()).toEqual( - expect.objectContaining({ - id: '2', - type: 'start', - }), - ); - - subscription1.unsubscribe(); - subscription2.unsubscribe(); - }); - it(`should send GQL_STOP for errored subscription if it's unsubscribed`, async () => { const query = gql` subscription { @@ -467,6 +411,66 @@ describe('GraphQL subscriptions', () => { expect(fixture.getMessagesSubscribers().size).toBe(0); }); + describe('GQL_CONNECTION_ACK', () => { + it('should retry errored subscriptions', async () => { + const subscription1 = fixture.triggerSubscription(gql` + subscription { + name + } + `); + + const subscription2 = fixture.triggerSubscription(gql` + subscription { + name2 + } + `); + + await fixture.handleConnectionInit(); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + fixture.server.send({ + id: '2', + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); + + expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1', '2']); + + await fixture.closeWithCode(1001); + + fixture.openServer(); + await fixture.handleConnectionInit(); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '1', + type: 'start', + }), + ); + + expect(await fixture.getNextMessage()).toEqual( + expect.objectContaining({ + id: '2', + type: 'start', + }), + ); + + subscription1.unsubscribe(); + subscription2.unsubscribe(); + }); + }); + describe('GQL_DATA', () => { it('should send data to subscriber', async () => { const subscriptionNextSpy = jest.fn(); @@ -495,7 +499,7 @@ describe('GraphQL subscriptions', () => { }); describe('GQL_COMPLETE', () => { - it(`should complete subscription`, async () => { + it('should complete subscription', async () => { const subscriptionCompleteSpy = jest.fn(); const subscription = fixture.triggerSubscription( @@ -519,7 +523,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`should clean up subscription`, async () => { + it('should clean up subscription', async () => { const subscription = fixture.triggerSubscription(gql` subscription { name From 39922b225017c9ed2550ff98cddc8537185f59ea Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 17:54:01 +0300 Subject: [PATCH 20/35] Cleanup --- .../__tests__/graphql-subscriptions.spec.ts | 115 +++++------------- 1 file changed, 33 insertions(+), 82 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 2312d898..6e2d9c1b 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -355,8 +355,6 @@ describe('GraphQL subscriptions', () => { }, }); - expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1']); - subscription.unsubscribe(); expect(await fixture.getNextMessage()).toEqual( @@ -413,7 +411,7 @@ describe('GraphQL subscriptions', () => { describe('GQL_CONNECTION_ACK', () => { it('should retry errored subscriptions', async () => { - const subscription1 = fixture.triggerSubscription(gql` + const subscription = fixture.triggerSubscription(gql` subscription { name } @@ -427,26 +425,17 @@ describe('GraphQL subscriptions', () => { await fixture.handleConnectionInit(); - fixture.server.send({ - id: '1', - type: 'error', - payload: { - data: null, - errors: ['The maximum subscription limit of 100 has been reached'], - }, - }); - - fixture.server.send({ - id: '2', - type: 'error', - payload: { - data: null, - errors: ['The maximum subscription limit of 100 has been reached'], - }, + [{ messageId: '1' }, { messageId: '2' }].forEach(({ messageId }) => { + fixture.server.send({ + id: messageId, + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); }); - expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1', '2']); - await fixture.closeWithCode(1001); fixture.openServer(); @@ -466,7 +455,7 @@ describe('GraphQL subscriptions', () => { }), ); - subscription1.unsubscribe(); + subscription.unsubscribe(); subscription2.unsubscribe(); }); }); @@ -518,7 +507,7 @@ describe('GraphQL subscriptions', () => { type: 'complete', }); - expect(subscriptionCompleteSpy).toHaveBeenCalled(); + expect(subscriptionCompleteSpy).toHaveBeenCalledTimes(1); subscription.unsubscribe(); }); @@ -542,7 +531,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`shouldn't send GQL_STOP if subscription completes`, async () => { + it('should not send GQL_STOP if subscription completes', async () => { const subscription = fixture.triggerSubscription(gql` subscription { name @@ -563,7 +552,7 @@ describe('GraphQL subscriptions', () => { }); describe('GQL_ERROR', () => { - it(`shouldn't error subscriptions before retrying`, async () => { + it('should not error subscriptions before retrying', async () => { const subscriptionErrorSpy = jest.fn(); const subscription = fixture.triggerSubscription( @@ -586,12 +575,12 @@ describe('GraphQL subscriptions', () => { }, }); - expect(subscriptionErrorSpy).not.toHaveBeenCalled(); + expect(subscriptionErrorSpy).not.toHaveBeenCalledTimes(1); subscription.unsubscribe(); }); - it(`shouldn't clean up subscriptions before retrying`, async () => { + it('should not clean up subscriptions before retrying', async () => { const subscription = fixture.triggerSubscription(gql` subscription { name @@ -614,7 +603,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`shouldn't drop socket connection`, async () => { + it('should not drop socket connection', async () => { const connectionDropSpy = jest.spyOn( fixture.graphqlService as any, 'handleConnectionDrop', @@ -637,7 +626,7 @@ describe('GraphQL subscriptions', () => { }, }); - expect(connectionDropSpy).not.toHaveBeenCalled(); + expect(connectionDropSpy).not.toHaveBeenCalledTimes(1); subscription.unsubscribe(); }); @@ -678,33 +667,17 @@ describe('GraphQL subscriptions', () => { await fixture.consumePingMessage(); fixture.sendMessageToClient({ type: 'pong' }); - fixture.server.send({ - id: '1', - type: 'error', - payload: { - data: null, - errors: ['The maximum subscription limit of 100 has been reached'], - }, - }); - - fixture.server.send({ - id: '2', - type: 'error', - payload: { - data: null, - errors: ['The maximum subscription limit of 100 has been reached'], - }, + [{ messageId: '1' }, { messageId: '2' }].forEach(({ messageId }) => { + fixture.server.send({ + id: messageId, + type: 'error', + payload: { + data: null, + errors: ['The maximum subscription limit of 100 has been reached'], + }, + }); }); - // Get latest haveAnySubscriptionsErrored state - await jest.advanceTimersByTimeAsync(0); - - const haveAnySubscriptionsErroredAfterError = await firstValueFrom( - fixture.graphqlService.haveAnySubscriptionsErrored(), - ); - - expect(haveAnySubscriptionsErroredAfterError).toBe(true); - // Wait for retry await jest.advanceTimersByTimeAsync(7_000); @@ -728,7 +701,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should error subscription after 3 failed retries', async () => { + it('should error subscriptions after 3 failed retries', async () => { jest.useFakeTimers(); const subscriptionErrorSpy = jest.fn(); @@ -796,14 +769,6 @@ describe('GraphQL subscriptions', () => { // mock-socket delivers client→server messages via setTimeout(4) await jest.advanceTimersByTimeAsync(10); - expect(await fixture.getNextMessage()).toEqual( - expect.objectContaining({ id: '1', type: 'start' }), - ); - - expect(await fixture.getNextMessage()).toEqual( - expect.objectContaining({ id: '2', type: 'start' }), - ); - sendErrorMessages(); } @@ -825,7 +790,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should clean up errored subscription after 3 failed retries', async () => { + it('should clean up errored subscriptions after 3 failed retries', async () => { jest.useFakeTimers(); const query = gql` @@ -891,14 +856,6 @@ describe('GraphQL subscriptions', () => { // mock-socket delivers client → server messages via setTimeout(4) await jest.advanceTimersByTimeAsync(10); - expect(await fixture.getNextMessage()).toEqual( - expect.objectContaining({ id: '1', type: 'start' }), - ); - - expect(await fixture.getNextMessage()).toEqual( - expect.objectContaining({ id: '2', type: 'start' }), - ); - sendErrorMessages(); } @@ -975,14 +932,10 @@ describe('GraphQL subscriptions', () => { fixture.sendMessageToClient({ id: '1', type: 'unknown', - payload: { - errors: [{ message: 'Something went wrong' }], - }, + payload: { errors: [{ message: 'error' }] }, }); - expect(subscriptionErrorSpy).toHaveBeenCalledWith([ - { message: 'Something went wrong' }, - ]); + expect(subscriptionErrorSpy).toHaveBeenCalledWith([{ message: 'error' }]); subscription.unsubscribe(); }); @@ -1002,9 +955,7 @@ describe('GraphQL subscriptions', () => { fixture.sendMessageToClient({ id: '1', type: 'unknown', - payload: { - errors: [{ message: 'Something went wrong' }], - }, + payload: { errors: [{ message: 'error' }] }, }); expect(fixture.getMessagesSubscribers().size).toBe(0); @@ -1014,7 +965,7 @@ describe('GraphQL subscriptions', () => { }); describe('haveAnySubscriptionsErrored', () => { - it(`should emit 'true' if any subscriptions error`, async () => { + it(`should emit 'true' if any subscriptions have errored`, async () => { const subscription = fixture.triggerSubscription(gql` subscription { name From 0411062efd39150f78feb4843bdbf2b3c014526b Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 18:04:14 +0300 Subject: [PATCH 21/35] Fix unit tests --- .../graphql/__tests__/graphql.service.spec.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql.service.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql.service.spec.ts index abb2e9ff..3858ee89 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql.service.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql.service.spec.ts @@ -42,7 +42,7 @@ describe('GraphQL service', function () { requestStub = sinon.stub(Qminder.ApiBase, 'queryGraph'); temporaryApiKeySpy = jest - .spyOn(graphqlService as any, 'fetchTemporaryApiKey') + .spyOn(graphqlService as any, 'getTemporaryApiKey') .mockResolvedValue(keyValue); }); @@ -155,15 +155,7 @@ describe('GraphQL service', function () { expect(WebSocket).toHaveBeenCalledTimes(1); }); }); - describe('.generateOperationId', () => { - it('returns an incrementing string', () => { - expect((graphqlService as any).generateOperationId()).toBe('1'); - expect((graphqlService as any).generateOperationId()).toBe('2'); - expect((graphqlService as any).generateOperationId()).toBe('3'); - expect((graphqlService as any).generateOperationId()).toBe('4'); - expect((graphqlService as any).generateOperationId()).toBe('5'); - }); - }); + afterEach(function () { requestStub.restore(); }); From 6e0a54a89a9f09878524a6e49e2f1066672479bd Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 18:09:31 +0300 Subject: [PATCH 22/35] Fix types --- .../__tests__/graphql-subscriptions.spec.ts | 85 +++++++++++++++---- 1 file changed, 67 insertions(+), 18 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 6e2d9c1b..59de46bd 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -5,6 +5,7 @@ import { firstValueFrom } from 'rxjs'; import { ConnectionStatus } from '../../../model/connection-status'; import { GraphQLSubscriptionsFixture } from '../__fixtures__/graphql-subscriptions-fixture'; +import { QminderGraphQLError } from '../graphql.service'; jest.mock('isomorphic-ws', () => WebSocket); jest.mock('../../../util/sleep-ms/sleep-ms', () => ({ @@ -351,7 +352,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); @@ -398,7 +403,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); @@ -431,7 +440,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); }); @@ -571,11 +584,15 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); - expect(subscriptionErrorSpy).not.toHaveBeenCalledTimes(1); + expect(subscriptionErrorSpy).not.toHaveBeenCalled(); subscription.unsubscribe(); }); @@ -594,7 +611,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); @@ -622,11 +643,15 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); - expect(connectionDropSpy).not.toHaveBeenCalledTimes(1); + expect(connectionDropSpy).not.toHaveBeenCalled(); subscription.unsubscribe(); }); @@ -673,7 +698,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); }); @@ -753,8 +782,10 @@ describe('GraphQL subscriptions', () => { payload: { data: null, errors: [ - 'The maximum subscription limit of 100 has been reached', - ], + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); }); @@ -838,8 +869,10 @@ describe('GraphQL subscriptions', () => { payload: { data: null, errors: [ - 'The maximum subscription limit of 100 has been reached', - ], + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); }); @@ -932,7 +965,9 @@ describe('GraphQL subscriptions', () => { fixture.sendMessageToClient({ id: '1', type: 'unknown', - payload: { errors: [{ message: 'error' }] }, + payload: { + errors: [{ message: 'error' }] satisfies QminderGraphQLError[], + }, }); expect(subscriptionErrorSpy).toHaveBeenCalledWith([{ message: 'error' }]); @@ -955,7 +990,9 @@ describe('GraphQL subscriptions', () => { fixture.sendMessageToClient({ id: '1', type: 'unknown', - payload: { errors: [{ message: 'error' }] }, + payload: { + errors: [{ message: 'error' }] satisfies QminderGraphQLError[], + }, }); expect(fixture.getMessagesSubscribers().size).toBe(0); @@ -979,7 +1016,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); @@ -1024,7 +1065,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); @@ -1065,7 +1110,11 @@ describe('GraphQL subscriptions', () => { type: 'error', payload: { data: null, - errors: ['The maximum subscription limit of 100 has been reached'], + errors: [ + { + message: 'The maximum subscription limit of 100 has been reached', + }, + ] satisfies QminderGraphQLError[], }, }); From 89b665255685bbed78c0ee5dedfda093f27d1dd8 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 18:26:02 +0300 Subject: [PATCH 23/35] Fix formatting --- .../graphql/__tests__/graphql-subscriptions.spec.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 59de46bd..6a92c2ff 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -442,7 +442,8 @@ describe('GraphQL subscriptions', () => { data: null, errors: [ { - message: 'The maximum subscription limit of 100 has been reached', + message: + 'The maximum subscription limit of 100 has been reached', }, ] satisfies QminderGraphQLError[], }, @@ -700,7 +701,8 @@ describe('GraphQL subscriptions', () => { data: null, errors: [ { - message: 'The maximum subscription limit of 100 has been reached', + message: + 'The maximum subscription limit of 100 has been reached', }, ] satisfies QminderGraphQLError[], }, @@ -783,7 +785,8 @@ describe('GraphQL subscriptions', () => { data: null, errors: [ { - message: 'The maximum subscription limit of 100 has been reached', + message: + 'The maximum subscription limit of 100 has been reached', }, ] satisfies QminderGraphQLError[], }, @@ -870,7 +873,8 @@ describe('GraphQL subscriptions', () => { data: null, errors: [ { - message: 'The maximum subscription limit of 100 has been reached', + message: + 'The maximum subscription limit of 100 has been reached', }, ] satisfies QminderGraphQLError[], }, From 1c9928219ef0cce4f6305c7c01d512c37fdd02e3 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 19:40:55 +0300 Subject: [PATCH 24/35] Retry only retryable errors --- .../__tests__/graphql-subscriptions.spec.ts | 121 +++++++++++++-- .../lib/services/graphql/graphql.service.ts | 140 ++++++++++++------ 2 files changed, 200 insertions(+), 61 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 6a92c2ff..15afbe4e 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -335,7 +335,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`should send GQL_STOP for errored subscription if it's unsubscribed`, async () => { + it(`should send GQL_STOP for retryable errored subscription if it's unsubscribed`, async () => { const query = gql` subscription { name @@ -389,7 +389,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it(`should clean up errored subscription if it's unsubscribed`, async () => { + it(`should clean up retryable errored subscription if it's unsubscribed`, async () => { const subscription = fixture.triggerSubscription(gql` subscription { name @@ -419,7 +419,7 @@ describe('GraphQL subscriptions', () => { }); describe('GQL_CONNECTION_ACK', () => { - it('should retry errored subscriptions', async () => { + it('should retry retryable errored subscriptions', async () => { const subscription = fixture.triggerSubscription(gql` subscription { name @@ -566,7 +566,96 @@ describe('GraphQL subscriptions', () => { }); describe('GQL_ERROR', () => { - it('should not error subscriptions before retrying', async () => { + it.each([ + 'BAD_REQUEST', + 'FIELD_NOT_FOUND', + 'INVALID_ARGUMENT', + 'InvalidSyntax', + 'NOT_FOUND', + 'PERMISSION_DENIED', + 'ValidationError', + ])( + 'should immediately error non-retryable subscriptions (errorType: %s)', + async (errorType) => { + const subscriptionErrorSpy = jest.fn(); + + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: subscriptionErrorSpy }, + ); + + await fixture.handleConnectionInit(); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + errors: [ + { + message: 'error', + errorType, + }, + ] satisfies QminderGraphQLError[], + }, + }); + + expect(subscriptionErrorSpy).toHaveBeenCalledWith([ + { + message: 'error', + errorType, + }, + ]); + + subscription.unsubscribe(); + }, + ); + + it.each([ + 'BAD_REQUEST', + 'FIELD_NOT_FOUND', + 'INVALID_ARGUMENT', + 'InvalidSyntax', + 'NOT_FOUND', + 'PERMISSION_DENIED', + 'ValidationError', + ])( + 'should clean up non-retryable subscriptions (errorType: %s)', + async (errorType) => { + const subscription = fixture.triggerSubscription( + gql` + subscription { + name + } + `, + { error: () => {} }, + ); + + await fixture.handleConnectionInit(); + + fixture.server.send({ + id: '1', + type: 'error', + payload: { + errors: [ + { + message: 'error', + errorType, + }, + ] satisfies QminderGraphQLError[], + }, + }); + + expect(fixture.getMessagesSubscribers().size).toBe(0); + + subscription.unsubscribe(); + }, + ); + + it('should not immediately error retryable subscriptions', async () => { const subscriptionErrorSpy = jest.fn(); const subscription = fixture.triggerSubscription( @@ -657,7 +746,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it('should retry errored subscriptions after delay', async () => { + it('should retry retryable errored subscriptions after delay', async () => { jest.useFakeTimers(); const query = gql` @@ -732,7 +821,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should error subscriptions after 3 failed retries', async () => { + it('should error retryable subscriptions after 3 failed retries', async () => { jest.useFakeTimers(); const subscriptionErrorSpy = jest.fn(); @@ -824,7 +913,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should clean up errored subscriptions after 3 failed retries', async () => { + it('should clean up retryable errored subscriptions after 3 failed retries', async () => { jest.useFakeTimers(); const query = gql` @@ -1005,8 +1094,8 @@ describe('GraphQL subscriptions', () => { }); }); - describe('haveAnySubscriptionsErrored', () => { - it(`should emit 'true' if any subscriptions have errored`, async () => { + describe('haveAnyRetryableSubscriptionsErrored', () => { + it(`should emit 'true' if any retryable subscriptions have errored`, async () => { const subscription = fixture.triggerSubscription(gql` subscription { name @@ -1029,7 +1118,7 @@ describe('GraphQL subscriptions', () => { }); const haveAnySubscriptionsErrored = await firstValueFrom( - fixture.graphqlService.haveAnySubscriptionsErrored(), + fixture.graphqlService.haveAnyRetryableSubscriptionsErrored(), ); expect(haveAnySubscriptionsErrored).toBe(true); @@ -1037,7 +1126,7 @@ describe('GraphQL subscriptions', () => { subscription.unsubscribe(); }); - it('should clear errored subscriptions with a delay after successful batch retry', async () => { + it('should clear retryable errored subscriptions with a delay after successful batch retry', async () => { jest.useFakeTimers(); const query = gql` @@ -1081,7 +1170,7 @@ describe('GraphQL subscriptions', () => { await jest.advanceTimersByTimeAsync(0); const haveAnySubscriptionsErroredBeforeRetry = await firstValueFrom( - fixture.graphqlService.haveAnySubscriptionsErrored(), + fixture.graphqlService.haveAnyRetryableSubscriptionsErrored(), ); expect(haveAnySubscriptionsErroredBeforeRetry).toBe(true); @@ -1090,7 +1179,7 @@ describe('GraphQL subscriptions', () => { await jest.advanceTimersByTimeAsync(7_000); const haveAnySubscriptionsErroredAfterRetry = await firstValueFrom( - fixture.graphqlService.haveAnySubscriptionsErrored(), + fixture.graphqlService.haveAnyRetryableSubscriptionsErrored(), ); expect(haveAnySubscriptionsErroredAfterRetry).toBe(false); @@ -1100,7 +1189,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it(`should emit 'true' if there are errored subscriptions but socket reconnects`, async () => { + it(`should emit 'true' if there are retryable errored subscriptions but socket reconnects`, async () => { const subscription = fixture.triggerSubscription(gql` subscription { name @@ -1123,7 +1212,7 @@ describe('GraphQL subscriptions', () => { }); const haveAnySubscriptionsErroredBeforeReconnect = await firstValueFrom( - fixture.graphqlService.haveAnySubscriptionsErrored(), + fixture.graphqlService.haveAnyRetryableSubscriptionsErrored(), ); expect(haveAnySubscriptionsErroredBeforeReconnect).toBe(true); @@ -1134,7 +1223,7 @@ describe('GraphQL subscriptions', () => { await fixture.handleConnectionInit(); const haveAnySubscriptionsErroredAfterReconnect = await firstValueFrom( - fixture.graphqlService.haveAnySubscriptionsErrored(), + fixture.graphqlService.haveAnyRetryableSubscriptionsErrored(), ); expect(haveAnySubscriptionsErroredAfterReconnect).toBe(false); diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 26f9453d..9b5d3a46 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -71,9 +71,20 @@ enum MessageType { GQL_ERROR = 'error', } -const ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; +const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; // To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. -const ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 500; +const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 500; + +const NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES = [ + 'BAD_REQUEST', + 'FIELD_NOT_FOUND', + 'INVALID_ARGUMENT', + 'InvalidSyntax', + 'NOT_FOUND', + 'PERMISSION_DENIED', + 'ValidationError', +] as const; + const PONG_TIMEOUT_IN_MS = 12_000; const PING_PONG_INTERVAL_IN_MS = 20_000; @@ -110,7 +121,7 @@ export class GraphqlService { private readonly subscriptionConnection$: Observable; - private readonly erroredSubscriptionsAction$ = new Subject< + private readonly retryableErroredSubscriptionsAction$ = new Subject< | { readonly type: 'add'; readonly messageId: string; @@ -124,8 +135,8 @@ export class GraphqlService { } >(); - private readonly erroredSubscriptionsMessageIds$ = - this.erroredSubscriptionsAction$.pipe( + private readonly retryableErroredSubscriptionsMessageIds$ = + this.retryableErroredSubscriptionsAction$.pipe( scan((messageIds, action) => { const result = new Set(messageIds); @@ -143,21 +154,21 @@ export class GraphqlService { shareReplay(1), ); - private readonly haveAnySubscriptionsErrored$ = - this.erroredSubscriptionsMessageIds$.pipe( + private readonly haveAnyRetryableSubscriptionsErrored$ = + this.retryableErroredSubscriptionsMessageIds$.pipe( map(({ size }) => !!size), distinctUntilChanged(), ); - private erroredSubscriptionsRetryTimeout: ReturnType< + private retryableErroredSubscriptionsRetryTimeout: ReturnType< typeof setTimeout > | null = null; - private erroredSubscriptionsSuccessTimeout: ReturnType< + private retryableErroredSubscriptionsSuccessTimeout: ReturnType< typeof setTimeout > | null = null; - private erroredSubscriptionsRetryCount = 0; + private retryableErroredSubscriptionsRetryCount = 0; private temporaryApiKeyService: TemporaryApiKeyService | undefined; @@ -175,7 +186,7 @@ export class GraphqlService { shareReplay(1), ); - this.erroredSubscriptionsMessageIds$.subscribe(); + this.retryableErroredSubscriptionsMessageIds$.subscribe(); } /** @@ -257,9 +268,11 @@ export class GraphqlService { * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string * - * Retries errored subscriptions up to 3 times. Afterwards throws an error. + * Retries retryable errored subscriptions up to 3 times. Afterwards throws an error. + * + * To get notified when any retryable subscriptions have errored, use the {@link haveAnyRetryableSubscriptionsErrored} method. * - * To get notified when any subscriptions have errored, use the {@link haveAnySubscriptionsErrored} method. + * @see {@link NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES | non-retryable subscription error types} */ subscribe>( queryOrDocumentNode: string | DocumentNode, @@ -291,7 +304,7 @@ export class GraphqlService { }, ); - this.erroredSubscriptionsAction$.next({ + this.retryableErroredSubscriptionsAction$.next({ type: 'remove', messageId, }); @@ -341,12 +354,14 @@ export class GraphqlService { } /** - * Have any GraphQL subscriptions been rejected by the server. + * Have any retryable GraphQL subscriptions been rejected by the server. * - * Emits `false` if all errored subscriptions have been successfully retried. + * Emits `false` if all retryable errored subscriptions have been successfully retried. + * + * @see {@link NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES | non-retryable subscription error types} */ - haveAnySubscriptionsErrored(): Observable { - return this.haveAnySubscriptionsErrored$; + haveAnyRetryableSubscriptionsErrored(): Observable { + return this.haveAnyRetryableSubscriptionsErrored$; } /** @@ -468,8 +483,8 @@ export class GraphqlService { this.connectionAttemptsCount = 0; this.clearErroredSubscriptionsTimeouts(); - this.erroredSubscriptionsRetryCount = 0; - this.erroredSubscriptionsAction$.next({ type: 'clear' }); + this.retryableErroredSubscriptionsRetryCount = 0; + this.retryableErroredSubscriptionsAction$.next({ type: 'clear' }); this.setConnectionStatus(ConnectionStatus.CONNECTED); this.logger.info('Connected to websocket'); @@ -507,7 +522,7 @@ export class GraphqlService { } case MessageType.GQL_DATA: - this.erroredSubscriptionsAction$.next({ + this.retryableErroredSubscriptionsAction$.next({ type: 'remove', messageId: message.id, }); @@ -526,29 +541,52 @@ export class GraphqlService { clearTimeout(this.pongTimeout); break; - case MessageType.GQL_ERROR: + case MessageType.GQL_ERROR: { + const errors = message.payload?.errors ?? []; + + if (this.isAnySubscriptionErrorNonRetryable(errors)) { + this.logger.error( + `Non-retryable GraphQL subscription error: ${JSON.stringify( + message, + )}`, + ); + + // May have been retryable before + this.retryableErroredSubscriptionsAction$.next({ + type: 'remove', + messageId: message.id, + }); + + const subscriber = this.messagesSubscribers.get(message.id); + this.cleanUpSubscription(message.id); + subscriber?.error(errors); + + break; + } + this.logger.warn( - `GraphQL subscription error: ${JSON.stringify(message)}`, + `Retryable GraphQL subscription error: ${JSON.stringify(message)}`, ); this.clearErroredSubscriptionsSuccessTimeout(); - this.erroredSubscriptionsAction$.next({ + this.retryableErroredSubscriptionsAction$.next({ type: 'add', messageId: message.id, }); if ( - this.erroredSubscriptionsRetryCount < - ERRORED_SUBSCRIPTIONS_RETRY_LIMIT && - !this.erroredSubscriptionsRetryTimeout + this.retryableErroredSubscriptionsRetryCount < + RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT && + !this.retryableErroredSubscriptionsRetryTimeout ) { this.scheduleErroredSubscriptionsRetry(); - } else if (!this.erroredSubscriptionsRetryTimeout) { + } else if (!this.retryableErroredSubscriptionsRetryTimeout) { this.failErroredSubscriptions(); } break; + } default: { const subscriber = this.messagesSubscribers.get(message.id); @@ -656,38 +694,50 @@ export class GraphqlService { } private clearErroredSubscriptionsTimeouts(): void { - clearTimeout(this.erroredSubscriptionsRetryTimeout ?? undefined); - this.erroredSubscriptionsRetryTimeout = null; + clearTimeout(this.retryableErroredSubscriptionsRetryTimeout ?? undefined); + this.retryableErroredSubscriptionsRetryTimeout = null; this.clearErroredSubscriptionsSuccessTimeout(); } private clearErroredSubscriptionsSuccessTimeout(): void { - clearTimeout(this.erroredSubscriptionsSuccessTimeout ?? undefined); - this.erroredSubscriptionsSuccessTimeout = null; + clearTimeout(this.retryableErroredSubscriptionsSuccessTimeout ?? undefined); + this.retryableErroredSubscriptionsSuccessTimeout = null; + } + + private isAnySubscriptionErrorNonRetryable( + errors: QminderGraphQLError[], + ): boolean { + return errors + .filter((error) => error.errorType) + .some(({ errorType }) => + ( + NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES as unknown as string[] + ).includes(errorType), + ); } private scheduleErroredSubscriptionsRetry(): void { - const retryCount = this.erroredSubscriptionsRetryCount + 1; + const retryCount = this.retryableErroredSubscriptionsRetryCount + 1; const delay = calculateRandomizedExponentialBackoffTime(retryCount); this.logger.info( `Retry (${retryCount}) errored subscriptions in ${delay.toFixed(0)}ms`, ); - this.erroredSubscriptionsRetryTimeout = setTimeout(() => { + this.retryableErroredSubscriptionsRetryTimeout = setTimeout(() => { this.retryErroredSubscriptions(); - this.erroredSubscriptionsRetryCount = retryCount; - this.erroredSubscriptionsRetryTimeout = null; + this.retryableErroredSubscriptionsRetryCount = retryCount; + this.retryableErroredSubscriptionsRetryTimeout = null; }, delay); } private failErroredSubscriptions(): void { this.logger.error( - `Errored subscriptions retry limit (${ERRORED_SUBSCRIPTIONS_RETRY_LIMIT}) reached, giving up`, + `Errored subscriptions retry limit (${RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT}) reached, giving up`, ); - this.erroredSubscriptionsMessageIds$ + this.retryableErroredSubscriptionsMessageIds$ .pipe(take(1)) .subscribe((messageIds) => { for (const messageId of messageIds) { @@ -696,7 +746,7 @@ export class GraphqlService { subscriber?.error( new Error( - `Subscription failed after ${this.erroredSubscriptionsRetryCount} retries`, + `Subscription failed after ${this.retryableErroredSubscriptionsRetryCount} retries`, ), ); } @@ -704,7 +754,7 @@ export class GraphqlService { } private retryErroredSubscriptions(): void { - this.erroredSubscriptionsMessageIds$ + this.retryableErroredSubscriptionsMessageIds$ .pipe(take(1)) .subscribe((messageIds) => { for (const messageId of messageIds) { @@ -725,11 +775,11 @@ export class GraphqlService { ); } - this.erroredSubscriptionsSuccessTimeout = setTimeout(() => { - this.erroredSubscriptionsAction$.next({ type: 'clear' }); - this.erroredSubscriptionsRetryCount = 0; - this.erroredSubscriptionsSuccessTimeout = null; - }, ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS); + this.retryableErroredSubscriptionsSuccessTimeout = setTimeout(() => { + this.retryableErroredSubscriptionsAction$.next({ type: 'clear' }); + this.retryableErroredSubscriptionsRetryCount = 0; + this.retryableErroredSubscriptionsSuccessTimeout = null; + }, RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS); }); } From 6f38b076e8a351d5114a47fde2c9b090098e6c36 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 20:09:12 +0300 Subject: [PATCH 25/35] Clean up completed subscription --- .../src/lib/services/graphql/graphql.service.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 9b5d3a46..b8577f18 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -531,9 +531,15 @@ export class GraphqlService { break; case MessageType.GQL_COMPLETE: { + this.retryableErroredSubscriptionsAction$.next({ + type: 'remove', + messageId: message.id, + }); + const subscriber = this.messagesSubscribers.get(message.id); this.cleanUpSubscription(message.id); subscriber?.complete(); + break; } From a36ff406aa03a15fec4aedc90a3918db394bb95f Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 20:45:20 +0300 Subject: [PATCH 26/35] Cleanup --- .../lib/services/graphql/graphql.service.ts | 30 +++++++++++-------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index b8577f18..21788c15 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -71,10 +71,6 @@ enum MessageType { GQL_ERROR = 'error', } -const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; -// To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. -const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 500; - const NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES = [ 'BAD_REQUEST', 'FIELD_NOT_FOUND', @@ -85,8 +81,13 @@ const NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES = [ 'ValidationError', ] as const; -const PONG_TIMEOUT_IN_MS = 12_000; -const PING_PONG_INTERVAL_IN_MS = 20_000; +const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 5; + +// To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. +const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 1_000; + +const PONG_TIMEOUT_IN_MS = 2_000; +const PING_PONG_INTERVAL_IN_MS = 2_000; // https://www.w3.org/TR/websockets/#concept-websocket-close-fail const CLIENT_SIDE_CLOSE_EVENT = 1000; @@ -174,7 +175,6 @@ export class GraphqlService { private pongTimeout: any; private pingPongInterval: any; - private readonly sendPingWithThisBound = this.sendPing.bind(this); private connectionAttemptsCount = 0; @@ -654,16 +654,20 @@ export class GraphqlService { } private monitorWithPingPong(): void { - this.pingPongInterval = setInterval( - this.sendPingWithThisBound, - PING_PONG_INTERVAL_IN_MS, - ); + this.pingPongInterval = setInterval(() => { + this.sendPing(); + }, PING_PONG_INTERVAL_IN_MS); } private monitorWithOfflineEvent(): void { if (typeof window !== 'undefined') { - window.removeEventListener('offline', this.sendPingWithThisBound); - window.addEventListener('offline', this.sendPingWithThisBound); + window.removeEventListener('offline', () => { + this.sendPing(); + }); + + window.addEventListener('offline', () => { + this.sendPing(); + }); } } From c23cad17c93fb3af930f1e9ffe5efd2e8bdc02e9 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 20:46:33 +0300 Subject: [PATCH 27/35] Revert changes --- .../src/lib/services/graphql/graphql.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 21788c15..47b8393c 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -86,8 +86,8 @@ const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 5; // To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 1_000; -const PONG_TIMEOUT_IN_MS = 2_000; -const PING_PONG_INTERVAL_IN_MS = 2_000; +const PONG_TIMEOUT_IN_MS = 10_000; +const PING_PONG_INTERVAL_IN_MS = 20_000; // https://www.w3.org/TR/websockets/#concept-websocket-close-fail const CLIENT_SIDE_CLOSE_EVENT = 1000; From 11bd4b9666d9819430f1a18a0a712ee4ea7fb716 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 20:47:06 +0300 Subject: [PATCH 28/35] Revert change --- .../javascript-api/src/lib/services/graphql/graphql.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 47b8393c..53836e60 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -86,7 +86,7 @@ const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 5; // To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 1_000; -const PONG_TIMEOUT_IN_MS = 10_000; +const PONG_TIMEOUT_IN_MS = 12_000; const PING_PONG_INTERVAL_IN_MS = 20_000; // https://www.w3.org/TR/websockets/#concept-websocket-close-fail From 8dcfd30f088517da2a42d37ac8a1812e1de15108 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 20:48:08 +0300 Subject: [PATCH 29/35] Fix typo --- .../src/lib/services/graphql/graphql.service.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 53836e60..2a4b706d 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -111,7 +111,7 @@ export class GraphqlService { ConnectionStatus.DISCONNECTED, ); - private subcriptionsCount = 0; + private subscriptionsCount = 0; private subscriptions: Subscription[] = []; @@ -285,7 +285,7 @@ export class GraphqlService { } return new Observable((subscriber) => { - const messageId = `${++this.subcriptionsCount}`; + const messageId = `${++this.subscriptionsCount}`; this.subscriptions.push({ messageId, query }); this.sendMessage(messageId, MessageType.GQL_START, { query }).catch( From c27282ad87bdb3ea9e27945fe82057b0bcb5d062 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 21:01:43 +0300 Subject: [PATCH 30/35] Fix retry counts --- .../graphql/__tests__/graphql-subscriptions.spec.ts | 12 ++++++------ .../src/lib/services/graphql/graphql.service.ts | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 15afbe4e..9715bd5e 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -821,7 +821,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should error retryable subscriptions after 3 failed retries', async () => { + it('should error retryable subscriptions after 5 failed retries', async () => { jest.useFakeTimers(); const subscriptionErrorSpy = jest.fn(); @@ -885,7 +885,7 @@ describe('GraphQL subscriptions', () => { sendErrorMessages(); - for (let retryCount = 1; retryCount <= 3; retryCount++) { + for (let retryCount = 0; retryCount < 5; retryCount++) { // Wait for retry await jest.advanceTimersToNextTimerAsync(); @@ -897,13 +897,13 @@ describe('GraphQL subscriptions', () => { expect(subscriptionErrorSpy).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Subscription failed after 3 retries', + message: 'Subscription failed after 5 retries', }), ); expect(subscription2ErrorSpy).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Subscription failed after 3 retries', + message: 'Subscription failed after 5 retries', }), ); @@ -913,7 +913,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should clean up retryable errored subscriptions after 3 failed retries', async () => { + it('should clean up retryable errored subscriptions after 5 failed retries', async () => { jest.useFakeTimers(); const query = gql` @@ -975,7 +975,7 @@ describe('GraphQL subscriptions', () => { expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1', '2']); - for (let retryCount = 1; retryCount <= 3; retryCount++) { + for (let retryCount = 0; retryCount < 5; retryCount++) { // Wait for retry await jest.advanceTimersToNextTimerAsync(); diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 2a4b706d..c20de039 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -268,7 +268,7 @@ export class GraphqlService { * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string * - * Retries retryable errored subscriptions up to 3 times. Afterwards throws an error. + * Retries retryable errored subscriptions up to 5 times. Afterwards throws an error. * * To get notified when any retryable subscriptions have errored, use the {@link haveAnyRetryableSubscriptionsErrored} method. * From 38d421a8734ec0539124a1ec000ea427020f1ca0 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 21:15:01 +0300 Subject: [PATCH 31/35] Revert changes --- .../graphql/__tests__/graphql-subscriptions.spec.ts | 12 ++++++------ .../src/lib/services/graphql/graphql.service.ts | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 9715bd5e..49590bab 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -821,7 +821,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should error retryable subscriptions after 5 failed retries', async () => { + it('should error retryable subscriptions after 3 failed retries', async () => { jest.useFakeTimers(); const subscriptionErrorSpy = jest.fn(); @@ -885,7 +885,7 @@ describe('GraphQL subscriptions', () => { sendErrorMessages(); - for (let retryCount = 0; retryCount < 5; retryCount++) { + for (let retryCount = 0; retryCount < 3; retryCount++) { // Wait for retry await jest.advanceTimersToNextTimerAsync(); @@ -897,13 +897,13 @@ describe('GraphQL subscriptions', () => { expect(subscriptionErrorSpy).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Subscription failed after 5 retries', + message: 'Subscription failed after 3 retries', }), ); expect(subscription2ErrorSpy).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Subscription failed after 5 retries', + message: 'Subscription failed after 3 retries', }), ); @@ -913,7 +913,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should clean up retryable errored subscriptions after 5 failed retries', async () => { + it('should clean up retryable errored subscriptions after 3 failed retries', async () => { jest.useFakeTimers(); const query = gql` @@ -975,7 +975,7 @@ describe('GraphQL subscriptions', () => { expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1', '2']); - for (let retryCount = 0; retryCount < 5; retryCount++) { + for (let retryCount = 0; retryCount < 3; retryCount++) { // Wait for retry await jest.advanceTimersToNextTimerAsync(); diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index c20de039..3be210a9 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -81,10 +81,10 @@ const NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES = [ 'ValidationError', ] as const; -const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 5; +const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; // To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. -const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 1_000; +const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 500; const PONG_TIMEOUT_IN_MS = 12_000; const PING_PONG_INTERVAL_IN_MS = 20_000; @@ -268,7 +268,7 @@ export class GraphqlService { * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string * - * Retries retryable errored subscriptions up to 5 times. Afterwards throws an error. + * Retries retryable errored subscriptions up to 3 times. Afterwards throws an error. * * To get notified when any retryable subscriptions have errored, use the {@link haveAnyRetryableSubscriptionsErrored} method. * From 6b70a49dd671832f64ac6f6e7eeab172e1b1eca5 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Tue, 19 May 2026 21:35:07 +0300 Subject: [PATCH 32/35] Increase retry count --- .../__tests__/graphql-subscriptions.spec.ts | 20 ++++++++++++------- .../lib/services/graphql/graphql.service.ts | 4 ++-- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts index 49590bab..5769301f 100644 --- a/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts +++ b/packages/javascript-api/src/lib/services/graphql/__tests__/graphql-subscriptions.spec.ts @@ -821,7 +821,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should error retryable subscriptions after 3 failed retries', async () => { + it('should error retryable subscriptions after 5 failed retries', async () => { jest.useFakeTimers(); const subscriptionErrorSpy = jest.fn(); @@ -865,6 +865,9 @@ describe('GraphQL subscriptions', () => { await fixture.consumePingMessage(); fixture.sendMessageToClient({ type: 'pong' }); + // Prevent ping-pong interval from interfering with retry timer advances + clearInterval(fixture.graphqlService['pingPongInterval']); + const sendErrorMessages = (): void => { [{ messageId: '1' }, { messageId: '2' }].forEach(({ messageId }) => { fixture.server.send({ @@ -885,11 +888,11 @@ describe('GraphQL subscriptions', () => { sendErrorMessages(); - for (let retryCount = 0; retryCount < 3; retryCount++) { + for (let retryCount = 0; retryCount < 5; retryCount++) { // Wait for retry await jest.advanceTimersToNextTimerAsync(); - // mock-socket delivers client→server messages via setTimeout(4) + // mock-socket delivers client → server messages via setTimeout(4) await jest.advanceTimersByTimeAsync(10); sendErrorMessages(); @@ -897,13 +900,13 @@ describe('GraphQL subscriptions', () => { expect(subscriptionErrorSpy).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Subscription failed after 3 retries', + message: 'Subscription failed after 5 retries', }), ); expect(subscription2ErrorSpy).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Subscription failed after 3 retries', + message: 'Subscription failed after 5 retries', }), ); @@ -913,7 +916,7 @@ describe('GraphQL subscriptions', () => { jest.useRealTimers(); }); - it('should clean up retryable errored subscriptions after 3 failed retries', async () => { + it('should clean up retryable errored subscriptions after 5 failed retries', async () => { jest.useFakeTimers(); const query = gql` @@ -953,6 +956,9 @@ describe('GraphQL subscriptions', () => { await fixture.consumePingMessage(); fixture.sendMessageToClient({ type: 'pong' }); + // Prevent ping-pong interval from interfering with retry timer advances + clearInterval(fixture.graphqlService['pingPongInterval']); + const sendErrorMessages = (): void => { [{ messageId: '1' }, { messageId: '2' }].forEach(({ messageId }) => { fixture.server.send({ @@ -975,7 +981,7 @@ describe('GraphQL subscriptions', () => { expect([...fixture.getMessagesSubscribers().keys()]).toEqual(['1', '2']); - for (let retryCount = 0; retryCount < 3; retryCount++) { + for (let retryCount = 0; retryCount < 5; retryCount++) { // Wait for retry await jest.advanceTimersToNextTimerAsync(); diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 3be210a9..64fa5f65 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -81,7 +81,7 @@ const NON_RETRYABLE_SUBSCRIPTION_ERROR_TYPES = [ 'ValidationError', ] as const; -const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 3; +const RETRYABLE_ERRORED_SUBSCRIPTIONS_RETRY_LIMIT = 5; // To avoid haveAnySubscriptionsErrored returning 'false' temporarily if retrying errored subscriptions fails. const RETRYABLE_ERRORED_SUBSCRIPTIONS_SUCCEEDED_DELAY_MS = 500; @@ -268,7 +268,7 @@ export class GraphqlService { * @returns a RxJS Observable that will push data * @throws when the `queryDocument` argument is an empty string * - * Retries retryable errored subscriptions up to 3 times. Afterwards throws an error. + * Retries retryable errored subscriptions up to 5 times. Afterwards throws an error. * * To get notified when any retryable subscriptions have errored, use the {@link haveAnyRetryableSubscriptionsErrored} method. * From 46f7323a959cfeb673c71624dcb1e813f1bf1a74 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 21 May 2026 14:57:31 +0300 Subject: [PATCH 33/35] Revert changes --- .../src/lib/services/graphql/graphql.service.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 64fa5f65..e7f89ab0 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -26,9 +26,15 @@ import { ApiBase, GraphqlQuery } from '../api-base/api-base.js'; import { TemporaryApiKeyService } from '../temporary-api-key/temporary-api-key.service.js'; function parseQuery(queryOrDocumentNode: string | DocumentNode): string { - return typeof queryOrDocumentNode === 'string' - ? queryOrDocumentNode - : print(queryOrDocumentNode); + if (typeof queryOrDocumentNode === 'string') { + return queryOrDocumentNode; + } + + if (queryOrDocumentNode.kind === 'Document') { + return print(queryOrDocumentNode); + } + + throw new Error('queryOrDocumentNode must be a string or a DocumentNode'); } export interface QminderGraphQLError { From 19d3cddfb3f03de87c1f37a27149fa23859127b2 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 21 May 2026 15:00:30 +0300 Subject: [PATCH 34/35] Add optional chaining --- .../javascript-api/src/lib/services/graphql/graphql.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index e7f89ab0..541edda6 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -533,7 +533,7 @@ export class GraphqlService { messageId: message.id, }); - this.messagesSubscribers.get(message.id)?.next(message.payload.data); + this.messagesSubscribers.get(message.id)?.next(message.payload?.data); break; case MessageType.GQL_COMPLETE: { From a30ae99857f71b1770b746c6de11dff3ca205f14 Mon Sep 17 00:00:00 2001 From: Rando Luik Date: Thu, 21 May 2026 15:04:37 +0300 Subject: [PATCH 35/35] Revert changes --- .../src/lib/services/graphql/graphql.service.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts index 541edda6..b030ee8d 100644 --- a/packages/javascript-api/src/lib/services/graphql/graphql.service.ts +++ b/packages/javascript-api/src/lib/services/graphql/graphql.service.ts @@ -181,6 +181,7 @@ export class GraphqlService { private pongTimeout: any; private pingPongInterval: any; + private readonly sendPingWithThisBound = this.sendPing.bind(this); private connectionAttemptsCount = 0; @@ -667,13 +668,8 @@ export class GraphqlService { private monitorWithOfflineEvent(): void { if (typeof window !== 'undefined') { - window.removeEventListener('offline', () => { - this.sendPing(); - }); - - window.addEventListener('offline', () => { - this.sendPing(); - }); + window.removeEventListener('offline', this.sendPingWithThisBound); + window.addEventListener('offline', this.sendPingWithThisBound); } }