diff --git a/src/client.ts b/src/client.ts index 976fb218..105e09fa 100644 --- a/src/client.ts +++ b/src/client.ts @@ -406,6 +406,48 @@ export function createClient(options: ClientOptions): Client { }), ]; } + /** + * Checks the `connect` problem and evaluates if the client should + * retry. If the problem is worth throwing, it will be thrown immediately. + */ + function shouldRetryConnectOrThrow(errOrCloseEvent: unknown): boolean { + // throw non `CloseEvent`s immediately, something else is wrong + if (!isCloseEvent(errOrCloseEvent)) { + throw errOrCloseEvent; + } + + // some close codes are worth reporting immediately + if ( + [ + 1002, // Protocol Error + 1011, // Internal Error + 4400, // Bad Request + 4401, // Unauthorized (tried subscribing before connect ack) + 4409, // Subscriber for already exists (distinction is very important) + 4429, // Too many initialisation requests + ].includes(errOrCloseEvent.code) + ) { + throw errOrCloseEvent; + } + + // normal closure is disposal, shouldnt try again + if (errOrCloseEvent.code === 1000) { + return false; + } + + // user cancelled early, shouldnt try again + if (errOrCloseEvent.code === 3499) { + return false; + } + + // retries are not allowed or we tried to many times, report error + if (!retryAttempts || state.tries > retryAttempts) { + throw errOrCloseEvent; + } + + // looks good, please retry + return true; + } // in non-lazy (hot?) mode always hold one connection lock to persist the socket if (!lazy) { @@ -419,22 +461,11 @@ export function createClient(options: ClientOptions): Client { // cancelled, shouldnt try again return; } catch (errOrCloseEvent) { - // throw non `CloseEvent`s immediately, something else is wrong - if (!isCloseEvent(errOrCloseEvent)) { - throw errOrCloseEvent; // TODO-db-200909 promise is uncaught, will appear in console - } - - // normal closure is disposal, shouldnt try again - if (errOrCloseEvent.code === 1000) { - return; - } - - // retries are not allowed or we tried to many times, close for good - if (!retryAttempts || state.tries > retryAttempts) { + // return if shouldnt try again + if (!shouldRetryConnectOrThrow(errOrCloseEvent)) { return; } - - // otherwise, wait a bit and retry + // if should try again, wait a bit and continue loop await new Promise((resolve) => setTimeout(resolve, retryTimeout)); } } @@ -533,27 +564,11 @@ export function createClient(options: ClientOptions): Client { // cancelled, shouldnt try again return; } catch (errOrCloseEvent) { - // throw non `CloseEvent`s immediately, something else is wrong - if (!isCloseEvent(errOrCloseEvent)) { - throw errOrCloseEvent; - } - - // normal closure is disposal, shouldnt try again - if (errOrCloseEvent.code === 1000) { + // return if shouldnt try again + if (!shouldRetryConnectOrThrow(errOrCloseEvent)) { return; } - - // user cancelled early, shouldnt try again - if (errOrCloseEvent.code === 3499) { - return; - } - - // retries are not allowed or we tried to many times, close for good - if (!retryAttempts || state.tries > retryAttempts) { - throw errOrCloseEvent; - } - - // otherwise, wait a bit and retry + // if should try again, wait a bit and continue loop await new Promise((resolve) => setTimeout(resolve, retryTimeout)); } } diff --git a/src/tests/client.ts b/src/tests/client.ts index bce7b8ec..4658bdeb 100644 --- a/src/tests/client.ts +++ b/src/tests/client.ts @@ -640,12 +640,16 @@ describe('reconnecting', () => { it('should not reconnect if retry attempts is zero', async () => { const { url, ...server } = await startTServer(); - createClient({ - url, - lazy: false, - retryAttempts: 0, - retryTimeout: 5, // fake timeout - }); + const sub = tsubscribe( + createClient({ + url, + retryAttempts: 0, + retryTimeout: 5, // fake timeout + }), + { + query: 'subscription { ping }', + }, + ); await server.waitForClient((client) => { client.close(); @@ -654,17 +658,26 @@ describe('reconnecting', () => { await server.waitForClient(() => { fail('Shouldnt have tried again'); }, 20); + + // client reported the error + await sub.waitForError((err) => { + expect((err as CloseEvent).code).toBe(1005); + }); }); it('should reconnect silently after socket closes', async () => { const { url, ...server } = await startTServer(); - createClient({ - url, - lazy: false, - retryAttempts: 1, - retryTimeout: 5, - }); + const sub = tsubscribe( + createClient({ + url, + retryAttempts: 1, + retryTimeout: 5, + }), + { + query: 'subscription { ping }', + }, + ); await server.waitForClient((client) => { client.close(); @@ -679,6 +692,48 @@ describe('reconnecting', () => { await server.waitForClient(() => { fail('Shouldnt have tried again'); }, 20); + + // client reported the error + await sub.waitForError((err) => { + expect((err as CloseEvent).code).toBe(1005); + }); + }); + + it('should report some close events immediately and not reconnect', async () => { + const { url, ...server } = await startTServer(); + + async function testCloseCode(code: number) { + const sub = tsubscribe( + createClient({ + url, + retryAttempts: Infinity, // keep retrying forever + retryTimeout: 5, + }), + { + query: 'subscription { ping }', + }, + ); + + await server.waitForClient((client) => { + client.close(code); + }); + + await server.waitForClient(() => { + fail('Shouldnt have tried again'); + }, 20); + + // client reported the error + await sub.waitForError((err) => { + expect((err as CloseEvent).code).toBe(code); + }); + } + + await testCloseCode(1002); + await testCloseCode(1011); + await testCloseCode(4400); + await testCloseCode(4401); + await testCloseCode(4409); + await testCloseCode(4429); }); it.todo( @@ -698,7 +753,6 @@ describe('events', () => { const client = await new Promise((resolve) => { const client = createClient({ url, - lazy: false, retryAttempts: 0, on: { connecting: connectingFn, @@ -712,9 +766,12 @@ describe('events', () => { resolve(client); }); client.on('closed', closedFn); + + // trigger connecting + tsubscribe(client, { query: 'subscription {ping}' }); }); - expect(connectingFn).toBeCalledTimes(1); // only once because `client.on` missed the initial connecting event + expect(connectingFn).toBeCalledTimes(2); expect(connectingFn.mock.calls[0].length).toBe(0); expect(connectedFn).toBeCalledTimes(2); // initial and registered listener @@ -738,7 +795,7 @@ describe('events', () => { } // retrying is disabled - expect(connectingFn).toBeCalledTimes(1); + expect(connectingFn).toBeCalledTimes(2); expect(connectedFn).toBeCalledTimes(2); expect(closedFn).toBeCalledTimes(2); // initial and registered listener