From ad1a027bd6796e819d94d892e5a39eaf6a1a6d83 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Thu, 2 Jan 2025 22:58:39 +0800 Subject: [PATCH 1/8] remove unused cache --- index.ts | 7 ------- 1 file changed, 7 deletions(-) diff --git a/index.ts b/index.ts index 0dbaa40..ada00cb 100644 --- a/index.ts +++ b/index.ts @@ -11,13 +11,7 @@ interface UserContext { } const createRocketflagClient = (version = "v1", apiUrl = "https://api.rocketflag.app") => { - const cache: { [key: string]: FlagStatus } = {}; - const getFlag = async (flagId: string, userContext: UserContext = {}): Promise => { - if (cache[flagId]) { - return cache[flagId]; - } - const url = new URL(`${apiUrl}/${version}/flags/${flagId}`); Object.entries(userContext).forEach(([key, value]) => { url.searchParams.append(key, value.toString()); @@ -35,7 +29,6 @@ const createRocketflagClient = (version = "v1", apiUrl = "https://api.rocketflag if (typeof response !== "object") throw new Error("Invalid response from server"); if (!validateFlag(response)) throw new Error("Invalid response from server"); - cache[flagId] = response; return response; }; From 499d5bed69ecb2daea04cd027370286dd64e0770 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Thu, 2 Jan 2025 23:36:33 +0800 Subject: [PATCH 2/8] improve test cases and add new ones --- index.test.ts | 66 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/index.test.ts b/index.test.ts index 4ba0d74..074deb6 100644 --- a/index.test.ts +++ b/index.test.ts @@ -7,29 +7,28 @@ global.fetch = jest.fn() as jest.Mock>; describe("createRocketflagClient", () => { const apiUrl = "https://api.rocketflag.app"; const flagId = "test-flag"; - const userContext = { userId: "user123" }; + const userContext = { cohort: "user123" }; beforeEach(() => { jest.resetAllMocks(); jest.spyOn(console, "error").mockImplementation(jest.fn()); }); - it("should create a client with default options", () => { - const client = createRocketflagClient(); - expect(client).toBeDefined(); - expect(client.getFlag).toBeDefined(); - expect(client.getFlag).toBeInstanceOf(Function); - }); + describe("getFlag", () => { + it("should fetch a flag", async () => { + const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); - it("should create a client with custom options", () => { - const client = createRocketflagClient("v2", "https://example.com/api"); - expect(client).toBeDefined(); - expect(client.getFlag).toBeDefined(); - expect(client.getFlag).toBeInstanceOf(Function); - }); + const client = createRocketflagClient(); + const flag = await client.getFlag(flagId, userContext); - describe("getFlag", () => { - it("should fetch and cache a flag", async () => { + expect(fetch).toHaveBeenCalledTimes(1); + expect(fetch).toHaveBeenCalledWith(new URL(`${apiUrl}/v1/flags/${flagId}?cohort=${userContext.cohort}`), { method: "GET" }); + expect(flag).toEqual(mockFlag); + }); + + it("should fetch a flag with special characters in the query", async () => { + userContext.cohort = "user+testing_rocketflag@example.com"; const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); @@ -37,13 +36,40 @@ describe("createRocketflagClient", () => { const flag = await client.getFlag(flagId, userContext); expect(fetch).toHaveBeenCalledTimes(1); - expect(fetch).toHaveBeenCalledWith(new URL(`${apiUrl}/v1/flags/${flagId}?userId=${userContext.userId}`), { method: "GET" }); + + const expectedUrl = `${apiUrl}/v1/flags/${flagId}?cohort=${"user%2Btesting_rocketflag%40example.com"}`; + const expectedURLObject = new URL(expectedUrl); + + expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); expect(flag).toEqual(mockFlag); + }); + + describe("custom client options", () => { + it("can create a client with a custom version", async () => { + const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); + + const client = createRocketflagClient("v2"); + await client.getFlag(flagId); + + const expectedUrl = `${apiUrl}/v2/flags/${flagId}`; + const expectedURLObject = new URL(expectedUrl); + + expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); + }); + + it("can create a client with a custom url", async () => { + const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); + + const client = createRocketflagClient("v2", "https://example.com"); + await client.getFlag(flagId); + + const expectedUrl = `https://example.com/v2/flags/${flagId}`; + const expectedURLObject = new URL(expectedUrl); - // Fetch again to test caching - const cachedFlag = await client.getFlag(flagId, userContext); - expect(fetch).toHaveBeenCalledTimes(1); // Still only called once - expect(cachedFlag).toEqual(mockFlag); + expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); + }); }); it("should handle non-ok responses from the server", async () => { From 8676433ab693165064802545bba5d7f7dc4a9226 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Thu, 2 Jan 2025 23:59:31 +0800 Subject: [PATCH 3/8] add better test coverage and custom errors --- errors.ts | 20 +++++++++ index.test.ts | 114 ++++++++++++++++++++++++++++++++++++-------------- index.ts | 40 ++++++++++++++---- 3 files changed, 133 insertions(+), 41 deletions(-) create mode 100644 errors.ts diff --git a/errors.ts b/errors.ts new file mode 100644 index 0000000..7ba85fd --- /dev/null +++ b/errors.ts @@ -0,0 +1,20 @@ +export class NetworkError extends Error { + constructor(message: string) { + super(message); + this.name = "NetworkError"; + } +} + +export class APIError extends Error { + constructor(message: string, public status: number, public statusText: string) { + super(message); + this.name = "APIError"; + } +} + +export class InvalidResponseError extends Error { + constructor(message: string) { + super(message); + this.name = "InvalidResponseError"; + } +} diff --git a/index.test.ts b/index.test.ts index 074deb6..1b31229 100644 --- a/index.test.ts +++ b/index.test.ts @@ -1,3 +1,4 @@ +import { APIError, InvalidResponseError, NetworkError } from "./errors"; import createRocketflagClient from "./index"; import { FlagStatus } from "./index"; @@ -14,6 +15,34 @@ describe("createRocketflagClient", () => { jest.spyOn(console, "error").mockImplementation(jest.fn()); }); + describe("custom client options", () => { + it("can create a client with a custom version", async () => { + const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); + + const client = createRocketflagClient("v2"); + await client.getFlag(flagId); + + const expectedUrl = `${apiUrl}/v2/flags/${flagId}`; + const expectedURLObject = new URL(expectedUrl); + + expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); + }); + + it("can create a client with a custom url", async () => { + const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); + + const client = createRocketflagClient("v2", "https://example.com"); + await client.getFlag(flagId); + + const expectedUrl = `https://example.com/v2/flags/${flagId}`; + const expectedURLObject = new URL(expectedUrl); + + expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); + }); + }); + describe("getFlag", () => { it("should fetch a flag", async () => { const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; @@ -23,7 +52,11 @@ describe("createRocketflagClient", () => { const flag = await client.getFlag(flagId, userContext); expect(fetch).toHaveBeenCalledTimes(1); - expect(fetch).toHaveBeenCalledWith(new URL(`${apiUrl}/v1/flags/${flagId}?cohort=${userContext.cohort}`), { method: "GET" }); + + const expectedUrl = `${apiUrl}/v1/flags/${flagId}?cohort=user123`; + const expectedURLObject = new URL(expectedUrl); + + expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); expect(flag).toEqual(mockFlag); }); @@ -44,39 +77,25 @@ describe("createRocketflagClient", () => { expect(flag).toEqual(mockFlag); }); - describe("custom client options", () => { - it("can create a client with a custom version", async () => { - const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; - (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); - - const client = createRocketflagClient("v2"); - await client.getFlag(flagId); - - const expectedUrl = `${apiUrl}/v2/flags/${flagId}`; - const expectedURLObject = new URL(expectedUrl); - - expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); - }); - - it("can create a client with a custom url", async () => { - const mockFlag: FlagStatus = { name: "Test Flag", enabled: true, id: flagId }; - (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve(mockFlag) }); - - const client = createRocketflagClient("v2", "https://example.com"); - await client.getFlag(flagId); - - const expectedUrl = `https://example.com/v2/flags/${flagId}`; - const expectedURLObject = new URL(expectedUrl); - - expect(fetch).toHaveBeenCalledWith(expect.objectContaining({ href: expectedURLObject.href }), { method: "GET" }); + it("should throw an APIError on non-ok response with correct status and statusText", async () => { + (fetch as jest.Mock).mockResolvedValue({ + ok: false, + status: 404, + statusText: "Flag Not Found", }); - }); - - it("should handle non-ok responses from the server", async () => { - (fetch as jest.Mock).mockResolvedValue({ ok: false, statusText: "Not Found" }); - const client = createRocketflagClient(); - await expect(client.getFlag(flagId, userContext)).rejects.toThrow("Not Found"); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow(APIError); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow("API request failed with status 404"); + + try { + await client.getFlag(flagId, userContext); + } catch (error) { + expect(error).toBeInstanceOf(APIError); + if (error instanceof APIError) { + expect(error.status).toBe(404); + expect(error.statusText).toBe("Flag Not Found"); + } + } }); it("should handle invalid responses from the server", async () => { @@ -92,5 +111,36 @@ describe("createRocketflagClient", () => { const client = createRocketflagClient(); await expect(client.getFlag(flagId, userContext)).rejects.toThrow("Network error"); }); + + it("should throw an error if flagId is empty", async () => { + const client = createRocketflagClient(); + await expect(client.getFlag("", userContext)).rejects.toThrow("flagId is required"); + }); + + it("should throw an error if flagId is not a string", async () => { + const client = createRocketflagClient(); + await expect(client.getFlag(123 as any, userContext)).rejects.toThrow("flagId must be a string"); + }); + + it("should throw a NetworkError on network error", async () => { + (fetch as jest.Mock).mockRejectedValue(new Error("Some network error")); + const client = createRocketflagClient(); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow(NetworkError); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow("Some network error"); + }); + + it("should throw an InvalidResponseError on invalid JSON response", async () => { + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.reject(new Error("Syntax error")) }); + const client = createRocketflagClient(); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow(InvalidResponseError); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow("Failed to parse JSON response"); + }); + + it("should throw an InvalidResponseError if response is not an object", async () => { + (fetch as jest.Mock).mockResolvedValue({ ok: true, json: () => Promise.resolve("not an object") }); + const client = createRocketflagClient(); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow(InvalidResponseError); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow("Invalid response format: response is not an object"); + }); }); }); diff --git a/index.ts b/index.ts index ada00cb..b313afe 100644 --- a/index.ts +++ b/index.ts @@ -1,5 +1,10 @@ +import { APIError, InvalidResponseError, NetworkError } from "./errors"; import { validateFlag } from "./validateFlag"; +const GET_METHOD = "GET"; +const DEFAULT_API_URL = "https://api.rocketflag.app"; +const DEFAULT_VERSION = "v1"; + export type FlagStatus = { name: string; enabled: boolean; @@ -7,26 +12,43 @@ export type FlagStatus = { }; interface UserContext { - [key: string]: string | number | boolean; + cohort?: string | number | boolean; } -const createRocketflagClient = (version = "v1", apiUrl = "https://api.rocketflag.app") => { +const createRocketflagClient = (version = DEFAULT_VERSION, apiUrl = DEFAULT_API_URL) => { const getFlag = async (flagId: string, userContext: UserContext = {}): Promise => { + if (!flagId) { + throw new Error("flagId is required"); + } + if (typeof flagId !== "string") { + throw new Error("flagId must be a string"); + } + if (typeof userContext !== "object") { + throw new Error("userContext must be an object"); + } + const url = new URL(`${apiUrl}/${version}/flags/${flagId}`); Object.entries(userContext).forEach(([key, value]) => { url.searchParams.append(key, value.toString()); }); - const raw = await fetch(url, { - method: "GET", - }); + let raw: Response; + try { + raw = await fetch(url, { method: GET_METHOD }); + } catch (error) { + throw new NetworkError(`Network error: ${error instanceof Error ? error.message : "Unknown error"}`); + } - if (!raw.ok) throw new Error(raw.statusText); + if (!raw.ok) throw new APIError(`API request failed with status ${raw.status}`, raw.status, raw.statusText); - const response: unknown = await raw.json(); + let response: unknown; + try { + response = await raw.json(); + } catch (error) { + throw new InvalidResponseError("Failed to parse JSON response"); + } - if (!response) throw new Error("Invalid response from server"); - if (typeof response !== "object") throw new Error("Invalid response from server"); + if (!response || typeof response !== "object") throw new InvalidResponseError("Invalid response format: response is not an object"); if (!validateFlag(response)) throw new Error("Invalid response from server"); return response; From 079edb263bd4ea817cbe67359c4e1cb1eb072e48 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Fri, 3 Jan 2025 00:02:16 +0800 Subject: [PATCH 4/8] additional validation --- index.test.ts | 14 ++++++++++++++ index.ts | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/index.test.ts b/index.test.ts index 1b31229..af6a782 100644 --- a/index.test.ts +++ b/index.test.ts @@ -142,5 +142,19 @@ describe("createRocketflagClient", () => { await expect(client.getFlag(flagId, userContext)).rejects.toThrow(InvalidResponseError); await expect(client.getFlag(flagId, userContext)).rejects.toThrow("Invalid response format: response is not an object"); }); + + it("should throw an InvalidResponseError if validateFlag fails", async () => { + (fetch as jest.Mock).mockResolvedValue({ + ok: true, + json: () => + Promise.resolve({ + name: "Test Flag", + enabled: true, + // id: flagId, // Missing ID to make it fail validation + }), + }); + const client = createRocketflagClient(); + await expect(client.getFlag(flagId, userContext)).rejects.toThrow(InvalidResponseError); + }); }); }); diff --git a/index.ts b/index.ts index b313afe..a3fb6e5 100644 --- a/index.ts +++ b/index.ts @@ -49,7 +49,7 @@ const createRocketflagClient = (version = DEFAULT_VERSION, apiUrl = DEFAULT_API_ } if (!response || typeof response !== "object") throw new InvalidResponseError("Invalid response format: response is not an object"); - if (!validateFlag(response)) throw new Error("Invalid response from server"); + if (!validateFlag(response)) throw new InvalidResponseError("Invalid response from server"); return response; }; From 934195e36fb63b1146fcd240db51d2c7ce11d5e0 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Fri, 3 Jan 2025 00:02:51 +0800 Subject: [PATCH 5/8] version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 52f4e47..f43f876 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@rocketflag/node-sdk", - "version": "0.1.5", + "version": "0.1.6", "author": "RocketFlag Developers (https://rocketflag.app)", "main": "index.js", "scripts": { From b79003676fb8bee4ddf610978fbdcf9054015963 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Fri, 3 Jan 2025 00:12:58 +0800 Subject: [PATCH 6/8] add test runner --- .github/workflows/test-runner.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 .github/workflows/test-runner.yml diff --git a/.github/workflows/test-runner.yml b/.github/workflows/test-runner.yml new file mode 100644 index 0000000..94760d0 --- /dev/null +++ b/.github/workflows/test-runner.yml @@ -0,0 +1,30 @@ +# This workflow will do a clean installation of node dependencies, cache/restore them, build the source code and run tests across different versions of node +# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-nodejs + +name: Node.js CI + +on: + push: + branches: ["main"] + pull_request: + branches: ["main"] + +jobs: + build: + runs-on: ubuntu-latest + + strategy: + matrix: + node-version: [20.x, 22.x] + # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ + + steps: + - uses: actions/checkout@v4 + - name: Use Node.js ${{ matrix.node-version }} + uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node-version }} + cache: "npm" + - run: npm ci + - run: npm run build --if-present + - run: npm test From 1fdfbfdad38972611983711677aca29585ec00c2 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Fri, 3 Jan 2025 00:18:29 +0800 Subject: [PATCH 7/8] support node 22 --- .github/workflows/test-runner.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test-runner.yml b/.github/workflows/test-runner.yml index 94760d0..dd96310 100644 --- a/.github/workflows/test-runner.yml +++ b/.github/workflows/test-runner.yml @@ -15,7 +15,7 @@ jobs: strategy: matrix: - node-version: [20.x, 22.x] + node-version: [22.x] # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ steps: From df7796da80c4ab49a3ef9460a0043d604dd7d3c9 Mon Sep 17 00:00:00 2001 From: JK Gunnink Date: Fri, 3 Jan 2025 00:33:57 +0800 Subject: [PATCH 8/8] moved ci runner to another branch --- .github/workflows/test-runner.yml | 30 ------------------------------ 1 file changed, 30 deletions(-) delete mode 100644 .github/workflows/test-runner.yml diff --git a/.github/workflows/test-runner.yml b/.github/workflows/test-runner.yml deleted file mode 100644 index dd96310..0000000 --- a/.github/workflows/test-runner.yml +++ /dev/null @@ -1,30 +0,0 @@ -# This workflow will do a clean installation of node dependencies, cache/restore them, build the source code and run tests across different versions of node -# For more information see: https://docs.github.com/en/actions/automating-builds-and-tests/building-and-testing-nodejs - -name: Node.js CI - -on: - push: - branches: ["main"] - pull_request: - branches: ["main"] - -jobs: - build: - runs-on: ubuntu-latest - - strategy: - matrix: - node-version: [22.x] - # See supported Node.js release schedule at https://nodejs.org/en/about/releases/ - - steps: - - uses: actions/checkout@v4 - - name: Use Node.js ${{ matrix.node-version }} - uses: actions/setup-node@v4 - with: - node-version: ${{ matrix.node-version }} - cache: "npm" - - run: npm ci - - run: npm run build --if-present - - run: npm test