fix(client): Some close events are not worth retrying

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 <id> 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));
           }
         }

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<Client>((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