fix(server): onDisconnect is called exclusively if the connection i…

…s acknowledged
ilijaNL · Jan 13, 2021 · 33ed5f2 · 33ed5f2
1 parent a8a4173
commit 33ed5f2
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 10 deletions.
diff --git a/docs/interfaces/server.serveroptions.md b/docs/interfaces/server.serveroptions.md
@@ -135,14 +135,21 @@ ___
 
 • `Optional` **onDisconnect**: *undefined* \| (`ctx`: [*Context*](server.context.md)<E\>, `code`: *number*, `reason`: *string*) => *void* \| *Promise*<*void*\>
 
-Called when the socket/client closes/disconnects for
-whatever reason. Provides the close event too. Beware
-that this callback happens AFTER all subscriptions have
-been gracefuly completed.
+Called when the client disconnects for whatever reason after
+he successfully went through the connection initialisation phase.
+Provides the close event too. Beware that this callback happens
+AFTER all subscriptions have been gracefully completed and BEFORE
+the `onClose` callback.
 
 If you are interested in tracking the subscriptions completions,
 consider using the `onComplete` callback.
 
+This callback will be called EXCLUSIVELY if the client connection
+is acknowledged. Meaning, `onConnect` will be called before the `onDisconnect`.
+
+For tracking socket closures at any point in time, regardless
+of the connection state - consider using the `onClose` callback.
+
 ___
 
 ### onError

diff --git a/src/server.ts b/src/server.ts
@@ -164,13 +164,20 @@ export interface ServerOptions<E = unknown> {
     | boolean
     | void;
   /**
-   * Called when the socket/client closes/disconnects for
-   * whatever reason. Provides the close event too. Beware
-   * that this callback happens AFTER all subscriptions have
-   * been gracefuly completed.
+   * Called when the client disconnects for whatever reason after
+   * he successfully went through the connection initialisation phase.
+   * Provides the close event too. Beware that this callback happens
+   * AFTER all subscriptions have been gracefully completed and BEFORE
+   * the `onClose` callback.
    *
    * If you are interested in tracking the subscriptions completions,
    * consider using the `onComplete` callback.
+   *
+   * This callback will be called EXCLUSIVELY if the client connection
+   * is acknowledged. Meaning, `onConnect` will be called before the `onDisconnect`.
+   *
+   * For tracking socket closures at any point in time, regardless
+   * of the connection state - consider using the `onClose` callback.
    */
   onDisconnect?: (
     ctx: Context<E>,
@@ -679,7 +686,7 @@ export function makeServer<E = unknown>(options: ServerOptions<E>): Server<E> {
         for (const sub of Object.values(ctx.subscriptions)) {
           await sub?.return?.();
         }
-        await onDisconnect?.(ctx, code, reason);
+        if (ctx.acknowledged) await onDisconnect?.(ctx, code, reason);
       };
     },
   };

diff --git a/src/tests/server.ts b/src/tests/server.ts
@@ -1392,7 +1392,7 @@ describe('Subscribe', () => {
 });
 
 describe('Disconnect', () => {
-  it('should report close code and reason to disconnect callback', async (done) => {
+  it('should report close code and reason to disconnect callback after connection acknowledgement', async (done) => {
     const { url, waitForConnect } = await startTServer({
       onDisconnect: (_ctx, code, reason) => {
         expect(code).toBe(4321);
@@ -1412,4 +1412,18 @@ describe('Disconnect', () => {
 
     client.ws.close(4321, 'Byebye');
   });
+
+  it('should not trigger the disconnect callback if connection is not acknowledged', async () => {
+    const { url, waitForClientClose } = await startTServer({
+      onDisconnect: () => {
+        fail("Disconnect callback shouldn't be triggered");
+      },
+    });
+
+    const client = await createTClient(url);
+
+    client.ws.close(4321, 'Byebye');
+
+    await waitForClientClose();
+  });
 });