fix(client): stop the user-token refresh from spinning

.changeset/sweet-donkeys-refresh.md

@@ -0,0 +1,5 @@
+---
+"@knocklabs/client": patch
+---
+
+Stop the user-token refresh from spinning. The expiry timer is no longer scheduled with a negative delay when the client authenticates with a token that is already inside the refresh window, orphaned timers are cleared instead of left live, and a refresh whose token expires too soon to schedule another wait no longer re-authenticates (which would tear down and rebuild the API client and socket on every round trip).

packages/client/src/knock.ts

@@ -22,6 +22,14 @@ import { jwtDecode } from "./jwt";
 
 const DEFAULT_HOST = "https://api.knock.app";
 
+/**
+ * Floor for the token-expiration timer. `exp - timeBeforeExpirationInMs - now`
+ * is negative whenever we authenticate with a token that is already inside the
+ * refresh window, and `setTimeout` treats a negative delay as `0`, so without a
+ * floor the refresh runs on the next tick.
+ */
+const MIN_TOKEN_EXPIRATION_DELAY_MS = 1_000;
+
 class Knock {
   public host: string;
   private apiClient: ApiClient | null = null;
@@ -289,33 +297,99 @@ class Knock {
     const nowMs = Date.now();
 
     // Expiration is in the future
-    if (expiresAtMs && expiresAtMs > nowMs) {
-      // Check how long until the token should be regenerated
-      // | ----------------- | ----------------------- |
-      // ^ now               ^ expiration offset       ^ expires at
-      const msInFuture = expiresAtMs - timeBeforeExpirationInMs - nowMs;
-
-      const timerId = setTimeout(async () => {
-        const newToken = await callbackFn(this.userToken as string, decoded);
-
-        // If we were torn down (logout/unmount) or re-authenticated while the
-        // callback was awaiting, the timer reference will have changed (or been
-        // cleared). Bail so we don't resurrect a logged-out instance by
-        // re-authenticating and re-opening connections.
-        if (this.tokenExpirationTimer !== timerId) {
-          return;
-        }
-
-        // Reauthenticate which will handle reinitializing sockets
-        if (typeof newToken === "string") {
-          this.authenticate(this.userId!, newToken, {
-            onUserTokenExpiring: callbackFn,
-            timeBeforeExpirationInMs: timeBeforeExpirationInMs,
-          });
-        }
-      }, msInFuture);
-      this.tokenExpirationTimer = timerId;
+    if (!expiresAtMs || expiresAtMs <= nowMs) return;
+
+    // Only ever keep one refresh timer per instance. `authenticate()` can be
+    // called repeatedly with unchanged credentials (a React effect re-running,
+    // for example), which does not tear down, so replacing the reference
+    // without clearing the old timer leaves it live: it still wakes up and
+    // calls the app's refresh callback before failing the identity check below.
+    if (this.tokenExpirationTimer) {
+      clearTimeout(this.tokenExpirationTimer);
+      this.tokenExpirationTimer = null;
     }
+
+    // Check how long until the token should be regenerated
+    // | ----------------- | ----------------------- |
+    // ^ now               ^ expiration offset       ^ expires at
+    //
+    // Floored: a token that is already inside the refresh window produces a
+    // negative delay, which `setTimeout` runs on the next tick.
+    const msInFuture = Math.max(
+      expiresAtMs - timeBeforeExpirationInMs - nowMs,
+      MIN_TOKEN_EXPIRATION_DELAY_MS,
+    );
+
+    const timerId = setTimeout(async () => {
+      const newToken = await callbackFn(this.userToken as string, decoded);
+
+      // If we were torn down (logout/unmount) or re-authenticated while the
+      // callback was awaiting, the timer reference will have changed (or been
+      // cleared). Bail so we don't resurrect a logged-out instance by
+      // re-authenticating and re-opening connections.
+      if (this.tokenExpirationTimer !== timerId) {
+        return;
+      }
+
+      if (typeof newToken !== "string") {
+        return;
+      }
+
+      // A refresh that does not push the expiration further out cannot be
+      // rescheduled sanely: re-authenticating would schedule another immediate
+      // refresh, and since changed credentials tear down and rebuild the API
+      // client, the instance would spin — hammering the app's token endpoint,
+      // re-identifying the user, and reconnecting the socket every round trip
+      // until the page is closed. Stop instead and let the caller notice.
+      if (
+        !this.refreshMakesProgress(
+          newToken,
+          expiresAtMs,
+          timeBeforeExpirationInMs,
+        )
+      ) {
+        this.log(
+          "Refreshed user token expires too soon to schedule another refresh; not re-authenticating",
+          true,
+        );
+        return;
+      }
+
+      // Reauthenticate which will handle reinitializing sockets
+      this.authenticate(this.userId!, newToken, {
+        onUserTokenExpiring: callbackFn,
+        timeBeforeExpirationInMs: timeBeforeExpirationInMs,
+      });
+    }, msInFuture);
+    this.tokenExpirationTimer = timerId;
+  }
+
+  /**
+   * Whether re-authenticating with `newToken` would buy real time: it has to
+   * expire later than the token it replaces, and late enough that the next
+   * refresh timer is a genuine wait rather than another immediate refresh.
+   *
+   * Tokens we cannot read an expiration from are treated as progress, leaving
+   * the behaviour to `authenticate()` as before — an unreadable or `exp`-less
+   * token schedules nothing, so it cannot spin.
+   */
+  private refreshMakesProgress(
+    newToken: string,
+    previousExpiresAtMs: number,
+    timeBeforeExpirationInMs: number,
+  ): boolean {
+    let nextExpiresAtMs: number;
+
+    try {
+      nextExpiresAtMs = (jwtDecode(newToken).exp ?? 0) * 1000;
+    } catch {
+      return true;
+    }
+
+    if (!nextExpiresAtMs) return true;
+    if (nextExpiresAtMs <= previousExpiresAtMs) return false;
+
+    return nextExpiresAtMs - timeBeforeExpirationInMs - Date.now() > 0;
   }
 
   /**

packages/client/test/knock.test.ts

@@ -663,6 +663,97 @@ describe("Knock Client", () => {
       expect(authenticateSpy).not.toHaveBeenCalled();
       expect(knock.isAuthenticated()).toBe(false);
     });
+
+    test("floors the refresh delay for a token already inside the expiration window", async () => {
+      const knock = new Knock("pk_test_12345");
+      const onUserTokenExpiring = vi.fn().mockResolvedValue("token_new");
+
+      // 5s of life left, but we refresh 30s before expiry: the unfloored delay
+      // is -25s, which `setTimeout` would run on the next tick.
+      vi.mocked(jwtDecode).mockReturnValueOnce({
+        exp: Math.floor((Date.now() + 5000) / 1000),
+      });
+
+      knock.authenticate("user_123", "token_abc", {
+        onUserTokenExpiring,
+        timeBeforeExpirationInMs: 30000,
+      });
+
+      await vi.advanceTimersByTimeAsync(0);
+      expect(onUserTokenExpiring).not.toHaveBeenCalled();
+
+      await vi.advanceTimersByTimeAsync(1000);
+      expect(onUserTokenExpiring).toHaveBeenCalledTimes(1);
+    });
+
+    test("does not re-authenticate when the refreshed token expires no later than the current one", async () => {
+      const knock = new Knock("pk_test_12345");
+      const authenticateSpy = vi.spyOn(knock, "authenticate");
+      const onUserTokenExpiring = vi.fn().mockResolvedValue("token_same_exp");
+
+      const exp = Math.floor((Date.now() + 60000) / 1000);
+      vi.mocked(jwtDecode)
+        .mockReturnValueOnce({ exp }) // the token we authenticate with
+        .mockReturnValueOnce({ exp }); // the refreshed token: no progress
+
+      knock.authenticate("user_123", "token_abc", {
+        onUserTokenExpiring,
+        timeBeforeExpirationInMs: 10000,
+      });
+      authenticateSpy.mockClear();
+
+      await vi.advanceTimersByTimeAsync(50000);
+
+      expect(onUserTokenExpiring).toHaveBeenCalledTimes(1);
+      expect(authenticateSpy).not.toHaveBeenCalled();
+    });
+
+    // A refreshed token that is itself inside the refresh window would schedule
+    // another refresh immediately. Re-authenticating on it spins: token endpoint
+    // flood, one identify and one socket reconnect per iteration, and 429s.
+    test("does not spin when the refreshed token is also inside the expiration window", async () => {
+      const knock = new Knock("pk_test_12345");
+      const authenticateSpy = vi.spyOn(knock, "authenticate");
+      const onUserTokenExpiring = vi
+        .fn()
+        .mockResolvedValue("token_also_near_expiry");
+
+      vi.mocked(jwtDecode)
+        // Authenticated with a token that has 5s of life left.
+        .mockReturnValueOnce({ exp: Math.floor((Date.now() + 5000) / 1000) })
+        // The refresh returns a later token, but still inside the 30s window.
+        .mockReturnValueOnce({ exp: Math.floor((Date.now() + 10000) / 1000) });
+
+      knock.authenticate("user_123", "token_abc", {
+        onUserTokenExpiring,
+        timeBeforeExpirationInMs: 30000,
+      });
+      authenticateSpy.mockClear();
+
+      await vi.advanceTimersByTimeAsync(60000);
+
+      expect(onUserTokenExpiring).toHaveBeenCalledTimes(1);
+      expect(authenticateSpy).not.toHaveBeenCalled();
+    });
+
+    test("keeps a single refresh timer when re-authenticating with unchanged credentials", async () => {
+      const knock = new Knock("pk_test_12345");
+      const onUserTokenExpiring = vi.fn().mockResolvedValue("token_new");
+
+      const options = {
+        onUserTokenExpiring,
+        timeBeforeExpirationInMs: 10000,
+      };
+
+      // Same credentials twice: no teardown runs, so the first timer has to be
+      // cleared explicitly or it stays live alongside the second.
+      knock.authenticate("user_123", "token_abc", options);
+      knock.authenticate("user_123", "token_abc", options);
+
+      await vi.advanceTimersByTimeAsync(60000);
+
+      expect(onUserTokenExpiring).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe("Authentication with reinitialize", () => {