diff --git a/.changeset/remove-client-axios.md b/.changeset/remove-client-axios.md new file mode 100644 index 000000000..54ba88274 --- /dev/null +++ b/.changeset/remove-client-axios.md @@ -0,0 +1,5 @@ +--- +"@knocklabs/client": patch +--- + +Replace the internal axios transport with native fetch and remove axios dependencies. diff --git a/packages/client/package.json b/packages/client/package.json index 8809dea23..e3ec1fc03 100644 --- a/packages/client/package.json +++ b/packages/client/package.json @@ -72,8 +72,6 @@ "@knocklabs/types": "workspace:^", "@tanstack/store": "^0.7.2", "@types/phoenix": "^1.6.7", - "axios": "^1.15.1", - "axios-retry": "^4.5.0", "eventemitter2": "^6.4.5", "jwt-decode": "^4.0.0", "nanoid": "^3.3.12", diff --git a/packages/client/src/api.ts b/packages/client/src/api.ts index e69fcfc33..2ee525520 100644 --- a/packages/client/src/api.ts +++ b/packages/client/src/api.ts @@ -1,5 +1,3 @@ -import axios, { AxiosError, AxiosInstance, AxiosRequestConfig } from "axios"; -import axiosRetry from "axios-retry"; import { Socket } from "phoenix"; import { exponentialBackoffFullJitter } from "./helpers"; @@ -23,12 +21,50 @@ export interface ApiResponse { status: number; } +export type ApiRequestConfig = { + method?: string; + url?: string; + params?: unknown; + data?: unknown; + headers?: HeadersInit; + signal?: AbortSignal; +}; + +type FetchClient = ( + input: RequestInfo | URL, + init?: RequestInit, +) => Promise; + +type ErrorWithResponse = Error & { + response?: { + status: number; + data?: unknown; + }; +}; + +class ApiRequestError extends Error { + response: { + status: number; + data?: unknown; + }; + + constructor(response: Response, data: unknown) { + super(`Request failed with status code ${response.status}`); + this.name = "ApiRequestError"; + this.response = { + status: response.status, + data, + }; + } +} + class ApiClient { private host: string; private apiKey: string; private userToken: string | null; private branch: string | null; - private axiosClient: AxiosInstance; + private fetchClient: FetchClient; + private defaultHeaders: Record; public socket: Socket | undefined; private pageVisibility: PageVisibilityManager | undefined; @@ -39,17 +75,14 @@ class ApiClient { this.userToken = options.userToken || null; this.branch = options.branch || null; - // Create a retryable axios client - this.axiosClient = axios.create({ - baseURL: this.host, - headers: { - Accept: "application/json", - "Content-Type": "application/json", - Authorization: `Bearer ${this.apiKey}`, - "X-Knock-User-Token": this.userToken, - "X-Knock-Client": this.getKnockClientHeader(), - "X-Knock-Branch": this.branch, - }, + this.fetchClient = this.getFetchClient(); + this.defaultHeaders = this.compactHeaders({ + Accept: "application/json", + "Content-Type": "application/json", + Authorization: `Bearer ${this.apiKey}`, + "X-Knock-User-Token": this.userToken, + "X-Knock-Client": this.getKnockClientHeader(), + "X-Knock-Branch": this.branch, }); if (typeof window !== "undefined") { @@ -77,38 +110,172 @@ class ApiClient { this.pageVisibility = new PageVisibilityManager(this.socket); } } - - axiosRetry(this.axiosClient, { - retries: 3, - retryCondition: this.canRetryRequest, - retryDelay: axiosRetry.exponentialDelay, - }); } - async makeRequest(req: AxiosRequestConfig): Promise { + async makeRequest(req: ApiRequestConfig): Promise { try { - const result = await this.axiosClient(req); + const result = await this.requestWithRetries(req); + const body = await this.parseResponseBody(result); return { - statusCode: result.status < 300 ? "ok" : "error", - body: result.data, - error: undefined, + statusCode: result.ok ? "ok" : "error", + body, + error: result.ok ? undefined : new ApiRequestError(result, body), status: result.status, }; // eslint:disable-next-line } catch (e: unknown) { console.error(e); + const response = (e as ErrorWithResponse)?.response; return { statusCode: "error", - status: 500, - body: undefined, + status: response?.status ?? 500, + body: response?.data, error: e, }; } } + private async requestWithRetries(req: ApiRequestConfig) { + let lastError: unknown; + + for (let attempt = 0; attempt <= 3; attempt++) { + try { + const response = await this.fetchClient( + this.buildUrl(req.url, req.params), + this.buildRequestInit(req), + ); + + if (!response.ok && this.canRetryRequest({ response })) { + lastError = new ApiRequestError( + response, + await this.parseResponseBody(response.clone()), + ); + } else { + return response; + } + } catch (error) { + lastError = error; + + if (!this.canRetryRequest(error)) { + throw error; + } + } + + if (attempt < 3) { + await this.delay(this.getRetryDelay(attempt + 1)); + } + } + + throw lastError; + } + + private buildRequestInit(req: ApiRequestConfig): RequestInit { + return { + method: req.method, + headers: { + ...this.defaultHeaders, + ...this.compactHeaders(req.headers), + }, + body: req.data === undefined ? undefined : JSON.stringify(req.data), + signal: req.signal, + }; + } + + private buildUrl(path = "", params?: ApiRequestConfig["params"]) { + const url = new URL(path, this.host); + + if (params) { + if (params instanceof URLSearchParams) { + params.forEach((value, key) => { + url.searchParams.append(key, value); + }); + } else if (typeof params === "object") { + Object.entries(params).forEach(([key, value]) => { + this.appendSearchParam(url.searchParams, key, value); + }); + } + } + + return url.toString(); + } + + private appendSearchParam( + searchParams: URLSearchParams, + key: string, + value: unknown, + ) { + if (value === undefined || value === null) { + return; + } + + if (Array.isArray(value)) { + value.forEach((item) => { + this.appendSearchParam(searchParams, `${key}[]`, item); + }); + return; + } + + if (value instanceof Date) { + searchParams.append(key, value.toISOString()); + return; + } + + if (typeof value === "object") { + Object.entries(value).forEach(([nestedKey, nestedValue]) => { + this.appendSearchParam( + searchParams, + `${key}[${nestedKey}]`, + nestedValue, + ); + }); + return; + } + + searchParams.append(key, String(value)); + } + + private async parseResponseBody(response: Response) { + if (response.status === 204) { + return undefined; + } + + const text = await response.text(); + + if (!text) { + return undefined; + } + + try { + return JSON.parse(text); + } catch { + return text; + } + } + + private delay(ms: number) { + return new Promise((resolve) => setTimeout(resolve, ms)); + } + + private getRetryDelay(retryCount: number) { + return Math.min(100 * 2 ** retryCount, 30_000); + } + + private getFetchClient(): FetchClient { + if (typeof globalThis.fetch === "function") { + return globalThis.fetch.bind(globalThis); + } + + return () => + Promise.reject( + new Error( + "Fetch is not available in this environment. Please provide a native fetch implementation.", + ), + ); + } + teardown() { this.pageVisibility?.teardown(); @@ -117,30 +284,77 @@ class ApiClient { } } - private canRetryRequest(error: AxiosError) { - // Retry Network Errors. - if (axiosRetry.isNetworkError(error)) { + private canRetryRequest(error: unknown) { + if (this.isFetchNetworkError(error)) { return true; } - if (!error.response) { + const response = (error as ErrorWithResponse)?.response; + + if (!response) { // Cannot determine if the request can be retried return false; } // Retry Server Errors (5xx). - if (error.response.status >= 500 && error.response.status <= 599) { + if (response.status >= 500 && response.status <= 599) { return true; } // Retry if rate limited. - if (error.response.status === 429) { + if (response.status === 429) { return true; } return false; } + private isFetchNetworkError(error: unknown) { + if (error instanceof TypeError) { + return true; + } + + if ( + typeof DOMException !== "undefined" && + error instanceof DOMException && + error.name === "NetworkError" + ) { + return true; + } + + return false; + } + + private compactHeaders(headers?: Record | HeadersInit) { + const output: Record = {}; + + if (!headers) { + return output; + } + + if (headers instanceof Headers) { + headers.forEach((value, key) => { + output[key] = value; + }); + return output; + } + + if (Array.isArray(headers)) { + headers.forEach(([key, value]) => { + output[key] = value; + }); + return output; + } + + Object.entries(headers).forEach(([key, value]) => { + if (value !== undefined && value !== null) { + output[key] = String(value); + } + }); + + return output; + } + private getKnockClientHeader() { // Note: we're following format used in our Stainless SDKs: // https://github.com/knocklabs/knock-node/blob/main/src/client.ts#L335 diff --git a/packages/client/test/api.test.ts b/packages/client/test/api.test.ts index 44f9f031b..321a43da4 100644 --- a/packages/client/test/api.test.ts +++ b/packages/client/test/api.test.ts @@ -1,38 +1,38 @@ -import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import { Socket } from "phoenix"; +import { afterEach, beforeEach, describe, expect, test, vi } from "vitest"; import packageJson from "../package.json"; import ApiClient from "../src/api"; -import { createAxiosMock, mockAxios } from "./test-utils/mocks"; - const TEST_BRANCH_SLUG = "lorem-ipsum-dolor-branch"; -// Type for accessing global properties type GlobalWithWindow = Record; -// Use vi.hoisted to ensure proper mock setup -const { mockIsNetworkError, mockExponentialDelay, mockAxiosRetry } = vi.hoisted( - () => { - const mockIsNetworkError = vi.fn(); - const mockExponentialDelay = vi.fn().mockReturnValue(1000); - const mockAxiosRetry = Object.assign(vi.fn(), { - isNetworkError: mockIsNetworkError, - exponentialDelay: mockExponentialDelay, - }); - - return { mockIsNetworkError, mockExponentialDelay, mockAxiosRetry }; - }, -); +const createJsonResponse = (body: unknown, status = 200) => + new Response(JSON.stringify(body), { + status, + headers: { "Content-Type": "application/json" }, + }); -// Mock axios-retry using the hoisted mocks -vi.mock("axios-retry", () => ({ - default: mockAxiosRetry, - isNetworkError: mockIsNetworkError, - exponentialDelay: mockExponentialDelay, -})); +const getDefaultHeaders = (apiClient: ApiClient) => + (apiClient as unknown as Record).defaultHeaders as Record< + string, + string + >; + +const setFetchMock = ( + apiClient: ApiClient, + fetchMock: ReturnType, +) => { + (apiClient as unknown as Record).fetchClient = fetchMock; +}; + +const skipRetryDelays = (apiClient: ApiClient) => { + (apiClient as unknown as Record).delay = vi + .fn() + .mockResolvedValue(undefined); +}; -// Mock Phoenix Socket directly in this file - vi.mock() calls are hoisted vi.mock("phoenix", () => ({ Socket: vi.fn().mockImplementation(() => ({ connect: vi.fn(), @@ -48,22 +48,8 @@ vi.mock("phoenix", () => ({ })), })); -// Apply module-level mocks -mockAxios(); - -/** - * Modern API Client Test Suite - * - * This test suite demonstrates modern testing practices including: - * - Realistic network simulation - * - Environment-specific testing (browser vs server) - * - Comprehensive error handling scenarios - * - Network resilience testing - * - Performance characteristics testing - */ describe("API Client", () => { beforeEach(() => { - // Clean slate for each test vi.clearAllMocks(); }); @@ -81,7 +67,6 @@ describe("API Client", () => { }); expect(apiClient).toBeInstanceOf(ApiClient); - // Don't test private properties directly - just verify it was created }); test("handles user token in configuration", () => { @@ -91,15 +76,13 @@ describe("API Client", () => { userToken: "user_token_456", }); - expect(apiClient).toBeInstanceOf(ApiClient); - // Don't test private properties directly - just verify it was created + expect(getDefaultHeaders(apiClient)["X-Knock-User-Token"]).toBe( + "user_token_456", + ); }); test("initializes WebSocket in browser environment", () => { - // Store original window value const originalWindow = (global as GlobalWithWindow).window; - - // Mock window to simulate browser environment (global as GlobalWithWindow).window = {} as Window; const apiClient = new ApiClient({ @@ -108,15 +91,12 @@ describe("API Client", () => { userToken: undefined, }); - // With mocked Phoenix Socket, socket should be defined in browser environment expect(apiClient.socket).toBeDefined(); - // Restore original window value (global as GlobalWithWindow).window = originalWindow; }); test("skips WebSocket in server environment", () => { - // Ensure window is undefined (server environment) const originalWindow = (global as GlobalWithWindow).window; (global as GlobalWithWindow).window = undefined; @@ -128,7 +108,6 @@ describe("API Client", () => { expect(apiClient.socket).toBeUndefined(); - // Restore original window value (global as GlobalWithWindow).window = originalWindow; }); @@ -169,19 +148,18 @@ describe("API Client", () => { }); describe("Request Handling", () => { - test("makes successful API requests", async () => { - const mockHttp = createAxiosMock(); + test("makes successful API requests with fetch", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); - - // Mock the internal axios client - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - mockHttp.mockSuccess({ data: "test response" }); + const fetchMock = vi.fn().mockResolvedValue( + createJsonResponse({ + data: "test response", + }), + ); + setFetchMock(apiClient, fetchMock); const response = await apiClient.makeRequest({ method: "GET", @@ -190,49 +168,56 @@ describe("API Client", () => { expect(response.statusCode).toBe("ok"); expect(response.body.data).toBe("test response"); + expect(fetchMock).toHaveBeenCalledWith( + "https://api.knock.app/test", + expect.objectContaining({ method: "GET" }), + ); }); - test("handles request with parameters", async () => { - const mockHttp = createAxiosMock(); + test("serializes request parameters with axios-compatible brackets", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi + .fn() + .mockResolvedValue(createJsonResponse({ ok: true })); + setFetchMock(apiClient, fetchMock); - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - mockHttp.mockSuccess({ received: true }); - - const response = await apiClient.makeRequest({ + await apiClient.makeRequest({ method: "GET", url: "/test", - params: { filter: "active" }, - }); - - expect(response.statusCode).toBe("ok"); - expect(mockHttp.axios).toHaveBeenCalledWith( - expect.objectContaining({ - params: { filter: "active" }, - }), - ); + params: { + filter: "active", + workflow_categories: ["billing", "security"], + nested: { enabled: true }, + ignored: undefined, + }, + }); + + const requestUrl = new URL(fetchMock.mock.calls[0]![0] as string); + expect(requestUrl.searchParams.get("filter")).toBe("active"); + expect(requestUrl.searchParams.getAll("workflow_categories[]")).toEqual([ + "billing", + "security", + ]); + expect(requestUrl.searchParams.get("nested[enabled]")).toBe("true"); + expect(requestUrl.searchParams.has("ignored")).toBe(false); }); - test("handles POST requests with data", async () => { - const mockHttp = createAxiosMock(); + test("sends POST requests with JSON data", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); - - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; + const fetchMock = vi + .fn() + .mockResolvedValue(createJsonResponse({ created: true })); + setFetchMock(apiClient, fetchMock); const testData = { name: "Test", value: 42 }; - mockHttp.mockSuccess({ created: true }); - const response = await apiClient.makeRequest({ method: "POST", url: "/test", @@ -240,122 +225,119 @@ describe("API Client", () => { }); expect(response.statusCode).toBe("ok"); - expect(mockHttp.axios).toHaveBeenCalledWith( + expect(fetchMock).toHaveBeenCalledWith( + "https://api.knock.app/test", expect.objectContaining({ - data: testData, + body: JSON.stringify(testData), }), ); }); + + test("parses empty and text responses", async () => { + const apiClient = new ApiClient({ + host: "https://api.knock.app", + apiKey: "pk_test_12345", + userToken: undefined, + }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce(new Response(null, { status: 204 })) + .mockResolvedValueOnce(new Response("accepted", { status: 202 })); + setFetchMock(apiClient, fetchMock); + + await expect( + apiClient.makeRequest({ url: "/empty" }), + ).resolves.toMatchObject({ + body: undefined, + status: 204, + statusCode: "ok", + }); + await expect( + apiClient.makeRequest({ url: "/text" }), + ).resolves.toMatchObject({ + body: "accepted", + status: 202, + statusCode: "ok", + }); + }); }); describe("Error Handling", () => { - test("handles network errors gracefully", async () => { - // Suppress console.error for this expected error test + test("returns a helpful error when fetch is unavailable", async () => { + const originalFetch = globalThis.fetch; + Object.defineProperty(globalThis, "fetch", { + configurable: true, + value: undefined, + }); + const consoleSpy = vi .spyOn(console, "error") .mockImplementation(() => {}); - try { - const mockHttp = createAxiosMock(); const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - // Mock network failure - this should not create unhandled rejections - const networkError = new Error("Network Error"); - ( - networkError as unknown as { code?: string; isAxiosError?: boolean } - ).code = "ECONNABORTED"; - (networkError as unknown as { isAxiosError?: boolean }).isAxiosError = - true; - mockHttp.axios.mockRejectedValue(networkError); - const response = await apiClient.makeRequest({ method: "GET", url: "/test", }); expect(response.statusCode).toBe("error"); - expect(response.error).toBe(networkError); + expect(response.error).toBeInstanceOf(Error); + expect(response.error.message).toBe( + "Fetch is not available in this environment. Please provide a native fetch implementation.", + ); } finally { + Object.defineProperty(globalThis, "fetch", { + configurable: true, + value: originalFetch, + }); consoleSpy.mockRestore(); } }); - test("handles different error types appropriately", async () => { - // Suppress console.error for this expected error test + test("handles network errors gracefully", async () => { const consoleSpy = vi .spyOn(console, "error") .mockImplementation(() => {}); - try { - const mockHttp = createAxiosMock(); + const networkError = new TypeError("Failed to fetch"); const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + setFetchMock(apiClient, vi.fn().mockRejectedValue(networkError)); + skipRetryDelays(apiClient); - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - const errorScenarios = [ - { - name: "timeout", - error: { - message: "timeout of 5000ms exceeded", - code: "ECONNABORTED", - }, - }, - { - name: "server error", - error: { - message: "Server Error", - response: { status: 500, data: { error: "Internal Error" } }, - }, - }, - { - name: "not found", - error: { - message: "Not Found", - response: { status: 404, data: { error: "Resource not found" } }, - }, - }, - ]; - - for (const scenario of errorScenarios) { - mockHttp.axios.mockRejectedValueOnce(scenario.error); - - const response = await apiClient.makeRequest({ - method: "GET", - url: "/test", - }); - - expect(response.statusCode).toBe("error"); - expect(response.error).toBe(scenario.error); - } + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", + }); + + expect(response.statusCode).toBe("error"); + expect(response.status).toBe(500); + expect(response.error).toBe(networkError); } finally { consoleSpy.mockRestore(); } }); - test("handles API error responses", async () => { - const mockHttp = createAxiosMock(); + test("handles API error responses with response metadata", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); - - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - mockHttp.mockError(500, "Internal Server Error"); + setFetchMock( + apiClient, + vi + .fn() + .mockResolvedValue(createJsonResponse({ error: "Not found" }, 404)), + ); const response = await apiClient.makeRequest({ method: "GET", @@ -363,177 +345,167 @@ describe("API Client", () => { }); expect(response.statusCode).toBe("error"); - expect(response.status).toBe(500); + expect(response.status).toBe(404); + expect(response.body).toEqual({ error: "Not found" }); + expect(response.error.response.status).toBe(404); + expect(response.error.response.data).toEqual({ error: "Not found" }); }); }); describe("Retry and Resilience", () => { - test("implements error handling for transient failures", async () => { - // Suppress console.error for this expected error test - const consoleSpy = vi - .spyOn(console, "error") - .mockImplementation(() => {}); - - try { - const mockHttp = createAxiosMock(); - const apiClient = new ApiClient({ - host: "https://api.knock.app", - apiKey: "pk_test_12345", - userToken: undefined, - }); - - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - // Mock network timeout - should not create unhandled rejections - mockHttp.axios.mockRejectedValueOnce(new Error("Network timeout")); - - const response = await apiClient.makeRequest({ - method: "GET", - url: "/test", - }); - - // Should handle the error gracefully - expect(response.statusCode).toBe("error"); - expect(response.error).toBeInstanceOf(Error); - expect(response.error.message).toBe("Network timeout"); - } finally { - consoleSpy.mockRestore(); - } - }); - test("retries on network errors", async () => { - // Configure the mock to return true for network errors - mockIsNetworkError.mockReturnValue(true); - const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi + .fn() + .mockRejectedValueOnce(new TypeError("Failed to fetch")) + .mockResolvedValue(createJsonResponse({ success: true })); + setFetchMock(apiClient, fetchMock); + skipRetryDelays(apiClient); - // Mock network error - const networkError = new Error("Network Error"); - ( - networkError as unknown as { code?: string; isAxiosError?: boolean } - ).code = "ECONNABORTED"; - (networkError as unknown as { isAxiosError?: boolean }).isAxiosError = - true; + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", + }); - const canRetry = (apiClient as unknown as Record) - .canRetryRequest as (error: unknown) => boolean; - expect(canRetry(networkError)).toBe(true); - expect(mockIsNetworkError).toHaveBeenCalledWith(networkError); + expect(response.statusCode).toBe("ok"); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - test("retries on 5xx server errors", async () => { + test("does not retry aborted requests", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const abortError = new DOMException( + "The operation was aborted.", + "AbortError", + ); + const fetchMock = vi.fn().mockRejectedValue(abortError); + setFetchMock(apiClient, fetchMock); + skipRetryDelays(apiClient); - const serverErrors = [500, 501, 502, 503, 504, 599]; - - serverErrors.forEach((status) => { - const serverError = { - response: { status }, - isAxiosError: true, - }; - - // Mock axiosRetry.isNetworkError to return false for server errors - mockIsNetworkError.mockReturnValue(false); + const consoleSpy = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + try { + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", + }); - const canRetry = (apiClient as unknown as Record) - .canRetryRequest as (error: unknown) => boolean; - expect(canRetry(serverError)).toBe(true); - }); + expect(response.statusCode).toBe("error"); + expect(response.error).toBe(abortError); + expect(fetchMock).toHaveBeenCalledTimes(1); + } finally { + consoleSpy.mockRestore(); + } }); - test("retries on rate limit errors (429)", async () => { + test("retries on 5xx server errors", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + createJsonResponse({ error: "Server Error" }, 500), + ) + .mockResolvedValue(createJsonResponse({ success: true })); + setFetchMock(apiClient, fetchMock); + skipRetryDelays(apiClient); - const rateLimitError = { - response: { status: 429 }, - isAxiosError: true, - }; - - mockIsNetworkError.mockReturnValue(false); + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", + }); - const canRetry = (apiClient as unknown as Record) - .canRetryRequest as (error: unknown) => boolean; - expect(canRetry(rateLimitError)).toBe(true); + expect(response.statusCode).toBe("ok"); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - test("does not retry on client errors (4xx except 429)", async () => { + test("returns the final retryable response after retries are exhausted", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi + .fn() + .mockResolvedValue(createJsonResponse({ error: "Server Error" }, 500)); + setFetchMock(apiClient, fetchMock); + skipRetryDelays(apiClient); - const clientErrors = [400, 401, 403, 404, 422]; - - clientErrors.forEach((status) => { - const clientError = { - response: { status }, - isAxiosError: true, - }; - - // Mock axiosRetry.isNetworkError to return false - mockIsNetworkError.mockReturnValue(false); + const consoleSpy = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + try { + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", + }); - const canRetry = (apiClient as unknown as Record) - .canRetryRequest as (error: unknown) => boolean; - expect(canRetry(clientError)).toBe(false); - }); + expect(response.statusCode).toBe("error"); + expect(response.status).toBe(500); + expect(response.body).toEqual({ error: "Server Error" }); + expect(response.error.response.status).toBe(500); + expect(fetchMock).toHaveBeenCalledTimes(4); + } finally { + consoleSpy.mockRestore(); + } }); - test("does not retry when response is undefined", async () => { + test("retries on rate limit errors (429)", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi + .fn() + .mockResolvedValueOnce( + createJsonResponse({ error: "Rate limited" }, 429), + ) + .mockResolvedValue(createJsonResponse({ success: true })); + setFetchMock(apiClient, fetchMock); + skipRetryDelays(apiClient); - const errorWithoutResponse = { - isAxiosError: true, - response: undefined, - }; - - mockIsNetworkError.mockReturnValue(false); + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", + }); - const canRetry = (apiClient as unknown as Record) - .canRetryRequest as (error: unknown) => boolean; - expect(canRetry(errorWithoutResponse)).toBe(false); + expect(response.statusCode).toBe("ok"); + expect(fetchMock).toHaveBeenCalledTimes(2); }); - test("does not retry on successful 2xx responses", async () => { + test("does not retry on client errors (4xx except 429)", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi + .fn() + .mockResolvedValue( + createJsonResponse({ error: "Resource not found" }, 404), + ); + setFetchMock(apiClient, fetchMock); + skipRetryDelays(apiClient); - const successResponses = [200, 201, 204]; - - successResponses.forEach((status) => { - const successError = { - response: { status }, - isAxiosError: true, - }; - - // Mock axiosRetry.isNetworkError to return false - mockIsNetworkError.mockReturnValue(false); - - const canRetry = (apiClient as unknown as Record) - .canRetryRequest as (error: unknown) => boolean; - expect(canRetry(successError)).toBe(false); + const response = await apiClient.makeRequest({ + method: "GET", + url: "/test", }); + + expect(response.statusCode).toBe("error"); + expect(fetchMock).toHaveBeenCalledTimes(1); }); }); @@ -545,11 +517,7 @@ describe("API Client", () => { userToken: undefined, }); - // Access the private axios client to check headers - const axiosClient = (apiClient as unknown as Record) - .axiosClient as { defaults: { headers: Record } }; - - expect(axiosClient.defaults.headers["X-Knock-Client"]).toBe( + expect(getDefaultHeaders(apiClient)["X-Knock-Client"]).toBe( `Knock/ClientJS ${packageJson.version}`, ); }); @@ -562,75 +530,52 @@ describe("API Client", () => { branch: TEST_BRANCH_SLUG, }); - const axiosClient = (apiClient as unknown as Record) - .axiosClient as { defaults: { headers: Record } }; - - expect(axiosClient.defaults.headers["X-Knock-Branch"]).toBe( + expect(getDefaultHeaders(apiClient)["X-Knock-Branch"]).toBe( TEST_BRANCH_SLUG, ); }); - test("supports various HTTP methods", async () => { - const mockHttp = createAxiosMock(); + test("omits optional headers when values are not configured", () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; - - const methods = ["GET", "POST", "PUT", "DELETE", "PATCH"]; - - for (const method of methods) { - mockHttp.mockSuccess({ method }); - - const response = await apiClient.makeRequest({ - method: method as "GET" | "POST" | "PUT" | "DELETE" | "PATCH", - url: "/test", - }); - - expect(response.statusCode).toBe("ok"); - expect(response.body.method).toBe(method); - } + expect( + getDefaultHeaders(apiClient)["X-Knock-User-Token"], + ).toBeUndefined(); + expect(getDefaultHeaders(apiClient)["X-Knock-Branch"]).toBeUndefined(); }); - test("handles request parameters correctly", async () => { - const mockHttp = createAxiosMock(); + test("supports various HTTP methods", async () => { const apiClient = new ApiClient({ host: "https://api.knock.app", apiKey: "pk_test_12345", userToken: undefined, }); + const fetchMock = vi.fn((_, init?: RequestInit) => + Promise.resolve(createJsonResponse({ method: init?.method })), + ); + setFetchMock(apiClient, fetchMock); - (apiClient as unknown as Record).axiosClient = - mockHttp.axios; + const methods = ["GET", "POST", "PUT", "DELETE", "PATCH"]; - mockHttp.axios.mockImplementation((config: unknown) => { - return Promise.resolve({ - status: 200, - data: { receivedConfig: config }, + for (const method of methods) { + const response = await apiClient.makeRequest({ + method, + url: "/test", }); - }); - - const response = await apiClient.makeRequest({ - method: "GET", - url: "/test", - params: { filter: "active", limit: 10 }, - }); - expect(response.statusCode).toBe("ok"); - expect(response.body.receivedConfig).toBeDefined(); + expect(response.statusCode).toBe("ok"); + expect(response.body.method).toBe(method); + } }); }); describe("Socket Connection Management", () => { test("provides socket interface in browser environment", () => { - // Store original window value const originalWindow = (global as GlobalWithWindow).window; - - // Mock window to simulate browser environment (global as GlobalWithWindow).window = {}; const apiClient = new ApiClient({ @@ -641,7 +586,6 @@ describe("API Client", () => { expect(apiClient.socket).toBeDefined(); - // Restore original window value (global as GlobalWithWindow).window = originalWindow; }); @@ -687,14 +631,12 @@ describe("API Client", () => { tries: number, ) => number; - // Call it many times to verify the range holds for (let i = 0; i < 50; i++) { const delay = reconnectAfterMs(1); expect(delay).toBeGreaterThanOrEqual(250); expect(delay).toBeLessThanOrEqual(1000); } - // At high tries, should be capped at 30_000 for (let i = 0; i < 50; i++) { const delay = reconnectAfterMs(100); expect(delay).toBeGreaterThanOrEqual(250); @@ -724,14 +666,12 @@ describe("API Client", () => { tries: number, ) => number; - // Call it many times to verify the range holds for (let i = 0; i < 50; i++) { const delay = rejoinAfterMs(1); expect(delay).toBeGreaterThanOrEqual(250); expect(delay).toBeLessThanOrEqual(1000); } - // At high tries, should be capped at 60_000 for (let i = 0; i < 50; i++) { const delay = rejoinAfterMs(100); expect(delay).toBeGreaterThanOrEqual(250); @@ -742,7 +682,6 @@ describe("API Client", () => { }); test("gracefully handles missing WebSocket in server environment", () => { - // Store original window value const originalWindow = (global as GlobalWithWindow).window; (global as GlobalWithWindow).window = undefined; @@ -752,10 +691,8 @@ describe("API Client", () => { userToken: undefined, }); - // In server environment, socket should be undefined expect(apiClient.socket).toBeUndefined(); - // Restore original window value (global as GlobalWithWindow).window = originalWindow; }); }); diff --git a/packages/client/test/setup.ts b/packages/client/test/setup.ts index 2b68ae122..8b4d475c9 100644 --- a/packages/client/test/setup.ts +++ b/packages/client/test/setup.ts @@ -87,7 +87,6 @@ console.warn = (message: unknown, ...args: unknown[]) => { }; // Completely suppress console.error during tests since we're testing error scenarios -// This prevents axios errors from showing up in test output console.error = noOp; // Suppress console.log during tests to prevent Knock debug messages diff --git a/packages/client/test/test-utils/mocks.ts b/packages/client/test/test-utils/mocks.ts index ebc0c865a..9153d9951 100644 --- a/packages/client/test/test-utils/mocks.ts +++ b/packages/client/test/test-utils/mocks.ts @@ -211,41 +211,6 @@ export const setupBrowserEnvironment = () => { }; }; -// Simple axios mock for API testing -export const createAxiosMock = () => { - const mockAxios = vi.fn(); - - // Default successful response - mockAxios.mockResolvedValue({ - status: 200, - data: { success: true }, - }); - - // Helper methods for common scenarios - const mockSuccess = (data: unknown, status = 200) => { - mockAxios.mockResolvedValue({ status, data }); - }; - - const mockError = (status: number, message = "Error") => { - mockAxios.mockResolvedValue({ - status, - data: { error: message }, - }); - }; - - const mockFailure = (error: Error) => { - mockAxios.mockRejectedValue(error); - }; - - return { - axios: mockAxios, - mockSuccess, - mockError, - mockFailure, - reset: () => mockAxios.mockClear(), - }; -}; - // Simple WebSocket mock export const createWebSocketMock = () => { const mockWebSocket = { @@ -311,23 +276,6 @@ export const createEventEmitterMock = () => ({ removeAllListeners: vi.fn(), }); -// Module-level mocks for common dependencies -export const mockAxios = () => { - vi.mock("axios", () => ({ - default: { - create: vi.fn(() => createAxiosMock().axios), - }, - })); -}; - -export const mockAxiosRetry = () => { - vi.mock("axios-retry", () => ({ - default: vi.fn(), - isNetworkError: vi.fn().mockReturnValue(false), - exponentialDelay: vi.fn().mockReturnValue(1000), - })); -}; - export const mockJwtDecode = () => { vi.mock("jwt-decode", () => ({ jwtDecode: vi.fn().mockReturnValue({ diff --git a/yarn.lock b/yarn.lock index 1d51a7738..8c3a1edc4 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4810,8 +4810,6 @@ __metadata: "@types/phoenix": "npm:^1.6.7" "@typescript-eslint/eslint-plugin": "npm:^8.59.4" "@typescript-eslint/parser": "npm:^8.59.4" - axios: "npm:^1.15.1" - axios-retry: "npm:^4.5.0" cross-env: "npm:^7.0.3" crypto: "npm:^1.0.1" eslint: "npm:^8.56.0" @@ -10113,13 +10111,6 @@ __metadata: languageName: node linkType: hard -"asynckit@npm:^0.4.0": - version: 0.4.0 - resolution: "asynckit@npm:0.4.0" - checksum: 10c0/d73e2ddf20c4eb9337e1b3df1a0f6159481050a5de457c55b14ea2e5cb6d90bb69e004c9af54737a5ee0917fcf2c9e25de67777bbe58261847846066ba75bc9d - languageName: node - linkType: hard - "available-typed-arrays@npm:^1.0.7": version: 1.0.7 resolution: "available-typed-arrays@npm:1.0.7" @@ -10136,28 +10127,6 @@ __metadata: languageName: node linkType: hard -"axios-retry@npm:^4.5.0": - version: 4.5.0 - resolution: "axios-retry@npm:4.5.0" - dependencies: - is-retry-allowed: "npm:^2.2.0" - peerDependencies: - axios: 0.x || 1.x - checksum: 10c0/574e7b1bf24aad99b560042d232a932d51bfaa29b5a6d4612d748ed799a6f11a5afb2582792492c55d95842200cbdfbe3454027a8c1b9a2d3e895d13c3d03c10 - languageName: node - linkType: hard - -"axios@npm:^1.15.1": - version: 1.15.2 - resolution: "axios@npm:1.15.2" - dependencies: - follow-redirects: "npm:^1.15.11" - form-data: "npm:^4.0.5" - proxy-from-env: "npm:^2.1.0" - checksum: 10c0/4eeae0feeaa7fdc1ef24f81f8b378fdadedf4aebdd6bf224484675160f8744cf17b9b0d1c215279979940f7e8ce463beffa2f713099612e428eac238515c81d5 - languageName: node - linkType: hard - "axobject-query@npm:^4.1.0": version: 4.1.0 resolution: "axobject-query@npm:4.1.0" @@ -11161,15 +11130,6 @@ __metadata: languageName: node linkType: hard -"combined-stream@npm:^1.0.8": - version: 1.0.8 - resolution: "combined-stream@npm:1.0.8" - dependencies: - delayed-stream: "npm:~1.0.0" - checksum: 10c0/0dbb829577e1b1e839fa82b40c07ffaf7de8a09b935cadd355a73652ae70a88b4320db322f6634a4ad93424292fa80973ac6480986247f1734a1137debf271d5 - languageName: node - linkType: hard - "commander@npm:^12.0.0": version: 12.1.0 resolution: "commander@npm:12.1.0" @@ -11658,13 +11618,6 @@ __metadata: languageName: node linkType: hard -"delayed-stream@npm:~1.0.0": - version: 1.0.0 - resolution: "delayed-stream@npm:1.0.0" - checksum: 10c0/d758899da03392e6712f042bec80aa293bbe9e9ff1b2634baae6a360113e708b91326594c8a486d475c69d6259afb7efacdc3537bfcda1c6c648e390ce601b19 - languageName: node - linkType: hard - "depd@npm:2.0.0": version: 2.0.0 resolution: "depd@npm:2.0.0" @@ -13916,16 +13869,6 @@ __metadata: languageName: node linkType: hard -"follow-redirects@npm:^1.15.11": - version: 1.15.11 - resolution: "follow-redirects@npm:1.15.11" - peerDependenciesMeta: - debug: - optional: true - checksum: 10c0/d301f430542520a54058d4aeeb453233c564aaccac835d29d15e050beb33f339ad67d9bddbce01739c5dc46a6716dbe3d9d0d5134b1ca203effa11a7ef092343 - languageName: node - linkType: hard - "fontfaceobserver@npm:^2.1.0": version: 2.3.0 resolution: "fontfaceobserver@npm:2.3.0" @@ -13952,19 +13895,6 @@ __metadata: languageName: node linkType: hard -"form-data@npm:^4.0.5": - version: 4.0.5 - resolution: "form-data@npm:4.0.5" - dependencies: - asynckit: "npm:^0.4.0" - combined-stream: "npm:^1.0.8" - es-set-tostringtag: "npm:^2.1.0" - hasown: "npm:^2.0.2" - mime-types: "npm:^2.1.12" - checksum: 10c0/dd6b767ee0bbd6d84039db12a0fa5a2028160ffbfaba1800695713b46ae974a5f6e08b3356c3195137f8530dcd9dfcb5d5ae1eeff53d0db1e5aad863b619ce3b - languageName: node - linkType: hard - "framer-motion@npm:^12.34.3": version: 12.34.3 resolution: "framer-motion@npm:12.34.3" @@ -15050,13 +14980,6 @@ __metadata: languageName: node linkType: hard -"is-retry-allowed@npm:^2.2.0": - version: 2.2.0 - resolution: "is-retry-allowed@npm:2.2.0" - checksum: 10c0/013be4f8a0a06a49ed1fe495242952e898325d496202a018f6f9fb3fb9ac8fe3b957a9bd62463d68299ae35dbbda680473c85a9bcefca731b49d500d3ccc08ff - languageName: node - linkType: hard - "is-set@npm:^2.0.3": version: 2.0.3 resolution: "is-set@npm:2.0.3" @@ -16726,7 +16649,7 @@ __metadata: languageName: node linkType: hard -"mime-types@npm:^2.1.12, mime-types@npm:^2.1.27, mime-types@npm:~2.1.34": +"mime-types@npm:^2.1.27, mime-types@npm:~2.1.34": version: 2.1.35 resolution: "mime-types@npm:2.1.35" dependencies: @@ -18231,13 +18154,6 @@ __metadata: languageName: node linkType: hard -"proxy-from-env@npm:^2.1.0": - version: 2.1.0 - resolution: "proxy-from-env@npm:2.1.0" - checksum: 10c0/ed01729fd4d094eab619cd7e17ce3698b3413b31eb102c4904f9875e677cd207392795d5b4adee9cec359dfd31c44d5ad7595a3a3ad51c40250e141512281c58 - languageName: node - linkType: hard - "punycode@npm:^2.1.0, punycode@npm:^2.3.1": version: 2.3.1 resolution: "punycode@npm:2.3.1"