diff --git a/package.json b/package.json index afea1608..68924544 100644 --- a/package.json +++ b/package.json @@ -1,68 +1,68 @@ { - "name": "@schematichq/schematic-typescript-node", - "version": "1.3.1", - "private": false, - "repository": { - "type": "git", - "url": "git+https://github.com/schematichq/schematic-node.git" - }, - "main": "./dist/index.js", - "types": "./dist/index.d.ts", - "scripts": { - "format": "biome format --write --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", - "format:check": "biome format --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", - "lint": "biome lint --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", - "lint:fix": "biome lint --fix --unsafe --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", - "check": "biome check --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", - "check:fix": "biome check --fix --unsafe --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", - "build": "node build.js", - "prepack": "cp -rv dist/. .", - "test": "jest --config jest.config.mjs", - "test:unit": "jest --selectProjects unit", - "test:wire": "jest --selectProjects wire", - "build:legacy": "tsc" - }, - "dependencies": { - "form-data": "^4.0.4", - "formdata-node": "^6.0.3", - "node-fetch": "^2.7.0", - "readable-stream": "^4.5.2" - }, - "devDependencies": { - "@types/node-fetch": "^2.6.12", - "@types/readable-stream": "^4.0.18", - "webpack": "^5.97.1", - "ts-loader": "^9.5.1", - "jest": "^29.7.0", - "@jest/globals": "^29.7.0", - "@types/jest": "^29.5.14", - "ts-jest": "^29.3.4", - "jest-environment-jsdom": "^29.7.0", - "msw": "2.11.2", - "@types/node": "^18.19.70", - "typescript": "~5.7.2", - "@biomejs/biome": "2.3.1", - "esbuild": "^0.25.9" - }, - "browser": { - "fs": false, - "os": false, - "path": false, - "stream": false, - "crypto": false, - "timers": false - }, - "packageManager": "yarn@1.22.22", - "engines": { - "node": ">=18.0.0" - }, - "sideEffects": false, - "license": "MIT", - "files": [ - "dist/**/*.js", - "dist/**/*.d.ts", - "!dist/**/*.js.map", - "!dist/**/*.test.*", - "README.md" - ] + "name": "@schematichq/schematic-typescript-node", + "version": "1.3.2", + "private": false, + "repository": { + "type": "git", + "url": "git+https://github.com/schematichq/schematic-node.git" + }, + "main": "./dist/index.js", + "types": "./dist/index.d.ts", + "scripts": { + "format": "biome format --write --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", + "format:check": "biome format --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", + "lint": "biome lint --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", + "lint:fix": "biome lint --fix --unsafe --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", + "check": "biome check --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", + "check:fix": "biome check --fix --unsafe --skip-parse-errors --no-errors-on-unmatched --max-diagnostics=none", + "build": "node build.js", + "prepack": "cp -rv dist/. .", + "test": "jest --config jest.config.mjs", + "test:unit": "jest --selectProjects unit", + "test:wire": "jest --selectProjects wire", + "build:legacy": "tsc" + }, + "dependencies": { + "form-data": "^4.0.4", + "formdata-node": "^6.0.3", + "node-fetch": "^2.7.0", + "readable-stream": "^4.5.2" + }, + "devDependencies": { + "@types/node-fetch": "^2.6.12", + "@types/readable-stream": "^4.0.18", + "webpack": "^5.97.1", + "ts-loader": "^9.5.1", + "jest": "^29.7.0", + "@jest/globals": "^29.7.0", + "@types/jest": "^29.5.14", + "ts-jest": "^29.3.4", + "jest-environment-jsdom": "^29.7.0", + "msw": "2.11.2", + "@types/node": "^18.19.70", + "typescript": "~5.7.2", + "@biomejs/biome": "2.3.1", + "esbuild": "^0.25.9" + }, + "browser": { + "fs": false, + "os": false, + "path": false, + "stream": false, + "crypto": false, + "timers": false + }, + "packageManager": "yarn@1.22.22", + "engines": { + "node": ">=18.0.0" + }, + "sideEffects": false, + "license": "MIT", + "files": [ + "dist/**/*.js", + "dist/**/*.d.ts", + "!dist/**/*.js.map", + "!dist/**/*.test.*", + "README.md" + ] } diff --git a/src/wrapper.ts b/src/wrapper.ts index e5fdbedb..7b6f315e 100644 --- a/src/wrapper.ts +++ b/src/wrapper.ts @@ -123,16 +123,22 @@ export class SchematicClient extends BaseClient { return getDefault(); } - try { - const cacheKey = JSON.stringify({ evalCtx, key }); - for (const provider of this.flagCheckCacheProviders) { + const cacheKey = JSON.stringify({ evalCtx, key }); + + for (const provider of this.flagCheckCacheProviders) { + try { const cachedValue = await provider.get(cacheKey); if (cachedValue !== undefined) { this.logger.debug(`${provider.constructor.name} cache hit for flag ${key}`); return cachedValue; } + } catch (err) { + this.logger.warn(`Cache read error for flag ${key} from ${provider.constructor.name}: ${err}`); } + } + let value: boolean; + try { const response = await this.features.checkFlag(key, evalCtx, { timeoutInSeconds: options?.timeoutMs !== undefined ? options.timeoutMs / 1000 : undefined, }); @@ -140,18 +146,23 @@ export class SchematicClient extends BaseClient { this.logger.debug(`No value returned from feature flag API for flag ${key}, falling back to default`); return getDefault(); } - - for (const provider of this.flagCheckCacheProviders) { - this.logger.debug(`Caching value for flag ${key} in ${provider.constructor.name}`); - await provider.set(cacheKey, response.data.value); - } - + value = response.data.value; this.logger.debug(`Feature flag API response for ${key}: ${JSON.stringify(response.data)}`); - return response.data.value; } catch (err) { this.logger.error(`Error checking flag ${key}: ${err}`); return getDefault(); } + + for (const provider of this.flagCheckCacheProviders) { + try { + this.logger.debug(`Caching value for flag ${key} in ${provider.constructor.name}`); + await provider.set(cacheKey, value); + } catch (err) { + this.logger.warn(`Cache write error for flag ${key} to ${provider.constructor.name}: ${err}`); + } + } + + return value; } /** @@ -193,16 +204,20 @@ export class SchematicClient extends BaseClient { let foundInCache = false; for (const provider of this.flagCheckCacheProviders) { - const cachedValue = await provider.get(cacheKey); - if (cachedValue !== undefined) { - this.logger.debug(`${provider.constructor.name} cache hit for flag ${key}`); - cachedResults.set(key, { - flag: key, - value: cachedValue, - reason: 'Retrieved from cache' - }); - foundInCache = true; - break; + try { + const cachedValue = await provider.get(cacheKey); + if (cachedValue !== undefined) { + this.logger.debug(`${provider.constructor.name} cache hit for flag ${key}`); + cachedResults.set(key, { + flag: key, + value: cachedValue, + reason: 'Retrieved from cache' + }); + foundInCache = true; + break; + } + } catch (err) { + this.logger.warn(`Cache read error for flag ${key} from ${provider.constructor.name}: ${err}`); } } @@ -233,8 +248,12 @@ export class SchematicClient extends BaseClient { // Cache the fresh result const cacheKey = JSON.stringify({ evalCtx, key }); for (const provider of this.flagCheckCacheProviders) { - this.logger.debug(`Caching value for flag ${cacheKey} in ${provider.constructor.name}`); - await provider.set(cacheKey, apiResult.value); + try { + this.logger.debug(`Caching value for flag ${cacheKey} in ${provider.constructor.name}`); + await provider.set(cacheKey, apiResult.value); + } catch (err) { + this.logger.warn(`Cache write error for flag ${key} to ${provider.constructor.name}: ${err}`); + } } results.push(apiResult); } else { @@ -260,7 +279,7 @@ export class SchematicClient extends BaseClient { } catch (err) { this.logger.error(`Error checking flags ${keys ? keys.join(', ') : 'all'}: ${err}`); - + // Return default values for all requested keys if (keys && keys.length > 0) { return keys.map(key => ({ diff --git a/src/cache.test.ts b/tests/cache.test.ts similarity index 98% rename from src/cache.test.ts rename to tests/cache.test.ts index fe31f5e3..498c9ff2 100644 --- a/src/cache.test.ts +++ b/tests/cache.test.ts @@ -1,6 +1,6 @@ /* eslint @typescript-eslint/no-explicit-any: 0 */ -import { LocalCache } from "./cache"; +import { LocalCache } from "../src/cache"; jest.useFakeTimers(); diff --git a/src/events.test.ts b/tests/events.test.ts similarity index 96% rename from src/events.test.ts rename to tests/events.test.ts index 684ccbf2..24b3b9ea 100644 --- a/src/events.test.ts +++ b/tests/events.test.ts @@ -1,9 +1,9 @@ /* eslint @typescript-eslint/no-explicit-any: 0 */ -import { EventBuffer } from "./events"; -import { EventsClient } from "./api/resources/events/client/Client"; -import { CreateEventRequestBody } from "./api"; -import { Logger } from "./logger"; +import { EventBuffer } from "../src/events"; +import { EventsClient } from "../src/api/resources/events/client/Client"; +import { CreateEventRequestBody } from "../src/api"; +import { Logger } from "../src/logger"; process.env.NODE_ENV = "test"; diff --git a/tests/wrapper.test.ts b/tests/wrapper.test.ts new file mode 100644 index 00000000..01df624f --- /dev/null +++ b/tests/wrapper.test.ts @@ -0,0 +1,214 @@ +/* eslint @typescript-eslint/no-explicit-any: 0 */ + +import { SchematicClient } from "../src/wrapper"; +import { CacheProvider } from "../src/cache"; +import { Logger } from "../src/logger"; + +jest.useFakeTimers(); + +class ThrowOnWriteCacheProvider implements CacheProvider { + async get(): Promise { + return undefined; + } + async set(): Promise { + throw new Error("429 Too Many Requests"); + } +} + +class ThrowOnReadCacheProvider implements CacheProvider { + async get(): Promise { + throw new Error("cache read failure"); + } + async set(): Promise { + // no-op + } +} + +function createMockLogger(): jest.Mocked { + return { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }; +} + +function mockFeatures(client: SchematicClient, mock: Record): void { + Object.defineProperty(client, "features", { + value: mock, + writable: true, + configurable: true, + }); +} + +describe("SchematicClient cache provider error handling", () => { + let mockLogger: jest.Mocked; + let client: SchematicClient; + + beforeEach(() => { + mockLogger = createMockLogger(); + }); + + afterEach(async () => { + if (client) { + await client.close(); + } + }); + + describe("checkFlag", () => { + it("should return API value when cache provider throws on set()", async () => { + client = new SchematicClient({ + apiKey: "test-api-key", + cacheProviders: { + flagChecks: [new ThrowOnWriteCacheProvider()], + }, + logger: mockLogger, + }); + + mockFeatures(client, { + checkFlag: jest.fn().mockResolvedValue({ + data: { value: true }, + }), + }); + + const result = await client.checkFlag({}, "api_enabled"); + + expect(result).toBe(true); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining("Cache write error"), + ); + }); + + it("should fall through to API when cache provider throws on get()", async () => { + client = new SchematicClient({ + apiKey: "test-api-key", + cacheProviders: { + flagChecks: [new ThrowOnReadCacheProvider()], + }, + logger: mockLogger, + }); + + mockFeatures(client, { + checkFlag: jest.fn().mockResolvedValue({ + data: { value: true }, + }), + }); + + const result = await client.checkFlag({}, "api_enabled"); + + expect(result).toBe(true); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining("Cache read error"), + ); + }); + + it("should skip failing provider and use next provider on read", async () => { + const workingProvider: CacheProvider = { + get: jest.fn().mockResolvedValue(true), + set: jest.fn(), + }; + + client = new SchematicClient({ + apiKey: "test-api-key", + cacheProviders: { + flagChecks: [new ThrowOnReadCacheProvider(), workingProvider], + }, + logger: mockLogger, + }); + + const result = await client.checkFlag({}, "my_flag"); + + expect(result).toBe(true); + expect(workingProvider.get).toHaveBeenCalled(); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining("Cache read error"), + ); + }); + + it("should still write to working providers when one provider throws on set()", async () => { + const workingProvider: CacheProvider = { + get: jest.fn().mockResolvedValue(undefined), + set: jest.fn(), + }; + + client = new SchematicClient({ + apiKey: "test-api-key", + cacheProviders: { + flagChecks: [new ThrowOnWriteCacheProvider(), workingProvider], + }, + logger: mockLogger, + }); + + mockFeatures(client, { + checkFlag: jest.fn().mockResolvedValue({ + data: { value: true }, + }), + }); + + const result = await client.checkFlag({}, "my_flag"); + + expect(result).toBe(true); + expect(workingProvider.set).toHaveBeenCalled(); + }); + }); + + describe("checkFlags", () => { + it("should return API values when cache provider throws on set()", async () => { + client = new SchematicClient({ + apiKey: "test-api-key", + cacheProviders: { + flagChecks: [new ThrowOnWriteCacheProvider()], + }, + logger: mockLogger, + }); + + mockFeatures(client, { + checkFlags: jest.fn().mockResolvedValue({ + data: { + flags: [ + { flag: "flag_a", value: true, reason: "matched" }, + { flag: "flag_b", value: false, reason: "no match" }, + ], + }, + }), + }); + + const results = await client.checkFlags({}, ["flag_a", "flag_b"]); + + expect(results).toHaveLength(2); + expect(results[0].value).toBe(true); + expect(results[1].value).toBe(false); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining("Cache write error"), + ); + }); + + it("should fall through to API when cache provider throws on get()", async () => { + client = new SchematicClient({ + apiKey: "test-api-key", + cacheProviders: { + flagChecks: [new ThrowOnReadCacheProvider()], + }, + logger: mockLogger, + }); + + mockFeatures(client, { + checkFlags: jest.fn().mockResolvedValue({ + data: { + flags: [ + { flag: "flag_a", value: true, reason: "matched" }, + ], + }, + }), + }); + + const results = await client.checkFlags({}, ["flag_a"]); + + expect(results).toHaveLength(1); + expect(results[0].value).toBe(true); + expect(mockLogger.warn).toHaveBeenCalledWith( + expect.stringContaining("Cache read error"), + ); + }); + }); +});