From 3b20fb0e48a8dd738daa3de521d6cb26debb5f27 Mon Sep 17 00:00:00 2001 From: Matthew Mikolay Date: Thu, 22 Oct 2026 16:52:49 -0400 Subject: [PATCH] chore(PLA-4090): cache socket URLs with null expiration indefinitely --- packages/client/src/gatewayTransport.ts | 21 ++++++--- packages/client/src/helpers.ts | 11 +++-- packages/client/test/api.test.ts | 47 +++++++++++++++++++ packages/client/test/gatewayTransport.test.ts | 29 +++++++++++- packages/client/test/helpers.test.ts | 35 ++++++++++++++ 5 files changed, 133 insertions(+), 10 deletions(-) diff --git a/packages/client/src/gatewayTransport.ts b/packages/client/src/gatewayTransport.ts index a1a229766..7b9165083 100644 --- a/packages/client/src/gatewayTransport.ts +++ b/packages/client/src/gatewayTransport.ts @@ -25,7 +25,7 @@ export type TransportConstructor = new ( export type GatewayConnectResult = { signedUrl: string; - expiresAt: number; + expiresAt: number | null; }; export type GatewayTransportOptions = { @@ -91,13 +91,22 @@ export async function fetchSocketConnectUrl({ } const body = result.body as - | Partial<{ url: string; expires_at: string }> + | Partial<{ url: string; expires_at: string | null }> | undefined; - const expiresAt = - typeof body?.expires_at === "string" ? Date.parse(body.expires_at) : NaN; - - if (typeof body?.url !== "string" || !body.url || Number.isNaN(expiresAt)) { + // expires_at may be null, meaning the URL is always valid + const expiresAt: number | null = + body?.expires_at === null + ? null + : typeof body?.expires_at === "string" + ? Date.parse(body.expires_at) + : NaN; + + if ( + typeof body?.url !== "string" || + !body.url || + (expiresAt !== null && Number.isNaN(expiresAt)) + ) { throw new Error("Invalid socket connect response"); } diff --git a/packages/client/src/helpers.ts b/packages/client/src/helpers.ts index a5b09517e..29faab36c 100644 --- a/packages/client/src/helpers.ts +++ b/packages/client/src/helpers.ts @@ -91,17 +91,22 @@ export const CACHE_EXPIRY_PADDING_MS = 5_000; /** * Wraps `fetchFunc` so its result is cached until `expiresAt` (epoch ms), minus - * `CACHE_EXPIRY_PADDING_MS`. + * `CACHE_EXPIRY_PADDING_MS`. If `expiresAt` is `null`, the value is cached indefinitely or until + * the cache is invalidated. */ export function cacheUntilExpired< Args extends unknown[], - T extends { expiresAt: number }, + T extends { expiresAt: number | null }, >(fetchFunc: (...args: Args) => Promise): CachedFetch { const store = new Store(null); const get = async (...args: Args) => { const cached = store.state; - if (cached && cached.expiresAt - CACHE_EXPIRY_PADDING_MS > Date.now()) { + if ( + cached && + (cached.expiresAt === null || + cached.expiresAt - CACHE_EXPIRY_PADDING_MS > Date.now()) + ) { return cached; } const value = await fetchFunc(...args); diff --git a/packages/client/test/api.test.ts b/packages/client/test/api.test.ts index 609b353f7..ff7eec76b 100644 --- a/packages/client/test/api.test.ts +++ b/packages/client/test/api.test.ts @@ -1275,6 +1275,53 @@ describe("API Client", () => { expect(fetchMock).toHaveBeenCalledTimes(2); }); + test("caches a signed WebSocket connect URL with a null expiration indefinitely", async () => { + const apiClient = new ApiClient({ + host: "https://api.knock.app", + apiKey: "pk_test_12345", + userToken: "user_token_456", + disconnectOnPageHidden: false, + }); + + const startDate = new Date(2024, 0, 1, 0, 0, 0); + vi.setSystemTime(startDate); + + const fetchMock = vi.fn().mockImplementation(() => + createJsonResponse({ + url: "wss://ws1.knock.app/ws/v1/websocket?auth_token=signed-token", + expires_at: null, + }), + ); + setFetchMock(apiClient, fetchMock); + + const socketOpts = vi.mocked(Socket).mock.calls[0]![1] as Record< + string, + unknown + >; + const GatewayTransport = socketOpts.transport as TransportConstructor; + + const PHOENIX_URL = + "wss://api.knock.app/ws/v1/websocket?vsn=2.0.0&api_key=pk_test_12345&user_token=user_token_456"; + + // The first connection fetches a signed connect URL. + new GatewayTransport(PHOENIX_URL); + await vi.waitFor(() => { + expect(MockWebSocket).toHaveBeenCalledTimes(1); + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // Advance the system clock well past when a normal TTL would expire. + vi.setSystemTime(new Date(startDate.getTime() + 60_000 + 1)); + + // A second connection, made well after a typical TTL, still reuses + // the cached URL since it never expires. + new GatewayTransport(PHOENIX_URL); + await vi.waitFor(() => { + expect(MockWebSocket).toHaveBeenCalledTimes(2); + }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + test("skips establishing a WebSocket connection when a signed URL fetch fails", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", diff --git a/packages/client/test/gatewayTransport.test.ts b/packages/client/test/gatewayTransport.test.ts index 057f44004..5cdf09dab 100644 --- a/packages/client/test/gatewayTransport.test.ts +++ b/packages/client/test/gatewayTransport.test.ts @@ -86,7 +86,7 @@ describe("fetchSocketConnectUrl", () => { vi.clearAllMocks(); }); - test("returns valid signed URL and expiration", async () => { + test("returns valid connect URL and expiration", async () => { const expiresAt = "2024-01-01T00:00:00.000Z"; makeRequest.mockResolvedValueOnce({ @@ -112,6 +112,21 @@ describe("fetchSocketConnectUrl", () => { }); }); + test("returns a valid connect URL without expiration", async () => { + makeRequest.mockResolvedValueOnce({ + statusCode: "ok", + status: 200, + body: { url: SIGNED_URL, expires_at: null }, + }); + + const result = await fetchSocketConnectUrl({ + apiClient, + signal: abortController.signal, + }); + + expect(result).toEqual({ signedUrl: SIGNED_URL, expiresAt: null }); + }); + test("throws an error when the request fails", async () => { const error = new Error("network down"); @@ -137,6 +152,18 @@ describe("fetchSocketConnectUrl", () => { ).rejects.toThrow("Socket connect request failed with status 500"); }); + test("throws an error when expiration is missing", async () => { + makeRequest.mockResolvedValueOnce({ + statusCode: "ok", + status: 200, + body: { url: SIGNED_URL }, + }); + + await expect( + fetchSocketConnectUrl({ apiClient, signal: abortController.signal }), + ).rejects.toThrow("Invalid socket connect response"); + }); + test("throws an error when expiration is not a string", async () => { makeRequest.mockResolvedValueOnce({ statusCode: "ok", diff --git a/packages/client/test/helpers.test.ts b/packages/client/test/helpers.test.ts index 7e000666c..2748e6aed 100644 --- a/packages/client/test/helpers.test.ts +++ b/packages/client/test/helpers.test.ts @@ -349,6 +349,41 @@ describe("Helper Functions", () => { expect(second).toEqual({ value: 2, expiresAt: 120_000 }); }); + test("caches indefinitely when expiresAt is null", async () => { + const fetchFunc = vi.fn(async (_testArg: string) => ({ + value: 1, + expiresAt: null, + })); + const { get } = cacheUntilExpired(fetchFunc); + + const first = await get(TEST_ARG); + vi.setSystemTime(60_001); + const second = await get(TEST_ARG); + vi.setSystemTime(120_001); + const third = await get(TEST_ARG); + + expect(fetchFunc).toHaveBeenCalledTimes(1); + expect(fetchFunc).toHaveBeenCalledWith(TEST_ARG); + expect(second).toBe(first); + expect(third).toBe(first); + }); + + test("invalidate forces a refetch when expiresAt is null", async () => { + const fetchFunc = vi + .fn<(testArg: string) => Promise<{ value: number; expiresAt: null }>>() + .mockResolvedValueOnce({ value: 1, expiresAt: null }) + .mockResolvedValueOnce({ value: 2, expiresAt: null }); + const { get, invalidate } = cacheUntilExpired(fetchFunc); + + await get(TEST_ARG); + invalidate(); + const second = await get(TEST_ARG); + + expect(fetchFunc).toHaveBeenCalledTimes(2); + expect(fetchFunc).toHaveBeenCalledWith(TEST_ARG); + expect(second).toEqual({ value: 2, expiresAt: null }); + }); + test("a rejected fetchFunc propagates and does not populate the cache", async () => { const error = new Error("boom"); const fetchFunc = vi