From 1dc1a341c0938874baae872e150079c7d646399b Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Mon, 2 Sep 2024 16:54:24 +0100 Subject: [PATCH 1/6] FFM-11972 Add timeout to auth call + update readme for API middleware --- README.md | 48 +++++++++++++++++++ src/__tests__/stream.test.ts | 92 ++++++++++++++++++------------------ src/index.ts | 52 ++++++++++++++++++-- src/types.ts | 6 +++ src/utils.ts | 1 + 5 files changed, 150 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 95d0d77..f2bd278 100644 --- a/README.md +++ b/README.md @@ -330,6 +330,54 @@ interface Evaluation { } ``` +## Authentication Request Timeout + +The `authRequestReadTimeout` option allows you to specify a timeout in milliseconds for the authentication request. If the request takes longer than this timeout, it will be aborted. This is useful for preventing hanging requests due to network issues or slow responses. + +If the request is aborted due to this timeout, an ERROR_AUTH event will be emitted, indicating that the authentication failed. Additionally, an ERROR event will be emitted to signal that a general error has occurred. + +**This only applies to the authentiaction request. If you wish to set a read timeout on the remaining requests made by the SDK, you may register [API Middleware](#api-middleware) + +```typescript +const options = { + authRequestReadTimeout: 30000, // Timeout in milliseconds (default: 30000) +}; + +const client = initialize( + 'YOUR_API_KEY', + { + identifier: 'Harness1', + attributes: { + lastUpdated: Date(), + host: location.href, + }, + }, + options +); +``` + +## API Middleware +The `registerAPIRequestMiddleware` function allows you to register a middleware function to manipulate the payload (URL, body and headers) of API requests after the AUTH call has successfully completed + +```typescript +function abortControllerMiddleware([url, options]) { + if (window.AbortController) { + const abortController = new AbortController(); + options.signal = abortController.signal; + + // Set a timeout to automatically abort the request after 30 seconds + setTimeout(() => abortController.abort(), 30000); + } + + return [url, options]; // Return the modified or original arguments +} + +// Register the middleware +client.registerAPIRequestMiddleware(abortControllerMiddleware); +``` +This middleware will automatically attach an AbortController to each request, which will abort the request if it takes longer than the specified timeout. You can also customize the middleware to perform other actions, such as logging or modifying headers. + + ## Logging By default, the Javascript Client SDK will log errors and debug messages using the `console` object. In some cases, it can be useful to instead log to a service or silently fail without logging errors. diff --git a/src/__tests__/stream.test.ts b/src/__tests__/stream.test.ts index c3bca50..3663a69 100644 --- a/src/__tests__/stream.test.ts +++ b/src/__tests__/stream.test.ts @@ -3,7 +3,7 @@ import type { Options } from '../types' import { Event } from '../types' import { getRandom } from '../utils' import type { Emitter } from 'mitt' -import type Poller from "../poller"; +import type Poller from '../poller' jest.useFakeTimers() @@ -49,16 +49,16 @@ const getStreamer = (overrides: Partial = {}, maxRetries: number = Infi } return new Streamer( - mockEventBus, - options, - `${options.baseUrl}/stream`, - 'test-api-key', - { 'Test-Header': 'value' }, - { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller, - logDebug, - logError, - jest.fn(), - maxRetries + mockEventBus, + options, + `${options.baseUrl}/stream`, + 'test-api-key', + { 'Test-Header': 'value' }, + { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller, + logDebug, + logError, + jest.fn(), + maxRetries ) } @@ -130,16 +130,16 @@ describe('Streamer', () => { it('should fallback to polling on stream failure', () => { const poller = { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller const streamer = new Streamer( - mockEventBus, - { baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, - 'http://test/stream', - 'test-api-key', - { 'Test-Header': 'value' }, - poller, - logDebug, - logError, - jest.fn(), - Infinity + mockEventBus, + { baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, + 'http://test/stream', + 'test-api-key', + { 'Test-Header': 'value' }, + poller, + logDebug, + logError, + jest.fn(), + Infinity ) streamer.start() @@ -154,21 +154,19 @@ describe('Streamer', () => { it('should stop polling when close is called if in fallback polling mode', () => { const poller = { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn() } as unknown as Poller - ;(poller.isPolling as jest.Mock) - .mockImplementationOnce(() => false) - .mockImplementationOnce(() => true) + ;(poller.isPolling as jest.Mock).mockImplementationOnce(() => false).mockImplementationOnce(() => true) const streamer = new Streamer( - mockEventBus, - { baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, - 'http://test/stream', - 'test-api-key', - { 'Test-Header': 'value' }, - poller, - logDebug, - logError, - jest.fn(), - 3 + mockEventBus, + { baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, + 'http://test/stream', + 'test-api-key', + { 'Test-Header': 'value' }, + poller, + logDebug, + logError, + jest.fn(), + 3 ) streamer.start() @@ -190,18 +188,22 @@ describe('Streamer', () => { }) it('should stop streaming but not call poller.stop if not in fallback polling mode when close is called', () => { - const poller = { start: jest.fn(), stop: jest.fn(), isPolling: jest.fn().mockReturnValue(false) } as unknown as Poller + const poller = { + start: jest.fn(), + stop: jest.fn(), + isPolling: jest.fn().mockReturnValue(false) + } as unknown as Poller const streamer = new Streamer( - mockEventBus, - { baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, - 'http://test/stream', - 'test-api-key', - { 'Test-Header': 'value' }, - poller, - logDebug, - logError, - jest.fn(), - 3 + mockEventBus, + { baseUrl: 'http://test', eventUrl: 'http://event', pollingEnabled: true, streamEnabled: true, debug: true }, + 'http://test/stream', + 'test-api-key', + { 'Test-Header': 'value' }, + poller, + logDebug, + logError, + jest.fn(), + 3 ) streamer.start() diff --git a/src/index.ts b/src/index.ts index 2a3b038..14518c7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -64,6 +64,10 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = configurations.logger.error(`[FF-SDK] ${message}`, ...args) } + const logWarn = (message: string, ...args: any[]) => { + configurations.logger.warn(`[FF-SDK] ${message}`, ...args) + } + const convertValue = (evaluation: Evaluation) => { let { value } = evaluation @@ -143,18 +147,58 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = } const authenticate = async (clientID: string, configuration: Options): Promise => { - const response = await fetch(`${configuration.baseUrl}/client/auth`, { + const url = `${configuration.baseUrl}/client/auth` + const requestOptions: RequestInit = { method: 'POST', headers: { 'Content-Type': 'application/json', 'Harness-SDK-Info': SDK_INFO }, body: JSON.stringify({ apiKey: clientID, target: { ...target, identifier: String(target.identifier) } }) - }) + } - const data: { authToken: string } = await response.json() + let timeoutId: any; + + if (window.AbortController) { + const abortController = new AbortController() + requestOptions.signal = abortController.signal + + if (configurations.authRequestReadTimeout > 0){ + timeoutId = setTimeout(() => abortController.abort(), configuration.authRequestReadTimeout) + } - return data.authToken + try { + const response = await fetch(url, requestOptions) + + if (!response.ok) { + throw new Error(`Http error: ${response.status}: ${response.statusText}`) + } + + const data: { authToken: string } = await response.json() + return data.authToken + } catch (error) { + if (abortController.signal.aborted) { + throw new Error('Request timed out') + } + + throw error + } finally { + if (timeoutId) { + clearTimeout(timeoutId); + } + } + } else { + logWarn('AbortController is not available, auth request will not timeout') + + const response = await fetch(url, requestOptions) + + if (!response.ok) { + throw new Error(`Http error: ${response.status}: ${response.statusText}`) + } + + const data: { authToken: string } = await response.json() + return data.authToken + } } let failedMetricsCallCount = 0 diff --git a/src/types.ts b/src/types.ts index 127e7d7..8ad658e 100644 --- a/src/types.ts +++ b/src/types.ts @@ -133,6 +133,12 @@ export interface Options { * Whether to enable debug logging. * @default false */ + authRequestReadTimeout?: number + /** + * The timeout in milliseconds for the authentication request to read the response. + * If the request takes longer than this timeout, it will be aborted and the SDK will fail to initialize, and `ERROR_AUTH` and `ERROR` events will be emitted. + * @default 0 (no timeout) + */ debug?: boolean /** * Whether to enable caching. diff --git a/src/utils.ts b/src/utils.ts index f543727..21b4723 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -11,6 +11,7 @@ export const defaultOptions: Options = { pollingInterval: MIN_POLLING_INTERVAL, streamEnabled: true, cache: false, + authRequestReadTimeout: 0, maxStreamRetries: Infinity } From a46766c83d351ae51a8526213fe0da219c52fde8 Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Mon, 2 Sep 2024 18:40:13 +0100 Subject: [PATCH 2/6] FFM-11972 Patch micromatch --- package-lock.json | 20 ++++++++++---------- package.json | 2 +- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index ae915b2..d8b55ae 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@harnessio/ff-javascript-client-sdk", - "version": "1.27.0", + "version": "1.28.0", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@harnessio/ff-javascript-client-sdk", - "version": "1.27.0", + "version": "1.28.0", "license": "Apache-2.0", "dependencies": { "jwt-decode": "^3.1.2", @@ -4963,12 +4963,12 @@ "dev": true }, "node_modules/micromatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", - "integrity": "sha512-DMy+ERcEW2q8Z2Po+WNXuw3c5YaUSFjAO5GsJqfEl7UjvtIuFKO6ZrKvcItdy98dwFI2N1tg3zNIdKaQT+aNdA==", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, "dependencies": { - "braces": "^3.0.2", + "braces": "^3.0.3", "picomatch": "^2.3.1" }, "engines": { @@ -9788,12 +9788,12 @@ "dev": true }, "micromatch": { - "version": "4.0.5", - "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz", - "integrity": "sha512-DMy+ERcEW2q8Z2Po+WNXuw3c5YaUSFjAO5GsJqfEl7UjvtIuFKO6ZrKvcItdy98dwFI2N1tg3zNIdKaQT+aNdA==", + "version": "4.0.8", + "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.8.tgz", + "integrity": "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA==", "dev": true, "requires": { - "braces": "^3.0.2", + "braces": "^3.0.3", "picomatch": "^2.3.1" } }, diff --git a/package.json b/package.json index 8a79ce4..9ebfd1f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@harnessio/ff-javascript-client-sdk", - "version": "1.27.0", + "version": "1.28.0", "author": "Harness", "license": "Apache-2.0", "main": "dist/sdk.cjs.js", From 537245b4951d3572e23d61b593f6443eaebc9f27 Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Mon, 2 Sep 2024 18:41:32 +0100 Subject: [PATCH 3/6] FFM-11972 Only log warning if config used --- src/index.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/index.ts b/src/index.ts index 14518c7..559c0e6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -157,14 +157,14 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = }) } - let timeoutId: any; + let timeoutId: any if (window.AbortController) { const abortController = new AbortController() requestOptions.signal = abortController.signal - if (configurations.authRequestReadTimeout > 0){ - timeoutId = setTimeout(() => abortController.abort(), configuration.authRequestReadTimeout) + if (configurations.authRequestReadTimeout > 0) { + timeoutId = setTimeout(() => abortController.abort(), configuration.authRequestReadTimeout) } try { @@ -184,11 +184,13 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = throw error } finally { if (timeoutId) { - clearTimeout(timeoutId); + clearTimeout(timeoutId) } } } else { - logWarn('AbortController is not available, auth request will not timeout') + if (configurations.authRequestReadTimeout > 0) { + logWarn('AbortController is not available, auth request will not timeout') + } const response = await fetch(url, requestOptions) From 07e61bf547d146bb861f1863eb06de1901029e00 Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Mon, 2 Sep 2024 18:51:53 +0100 Subject: [PATCH 4/6] FFM-11972 Update README --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index f2bd278..0a3364a 100644 --- a/README.md +++ b/README.md @@ -334,7 +334,7 @@ interface Evaluation { The `authRequestReadTimeout` option allows you to specify a timeout in milliseconds for the authentication request. If the request takes longer than this timeout, it will be aborted. This is useful for preventing hanging requests due to network issues or slow responses. -If the request is aborted due to this timeout, an ERROR_AUTH event will be emitted, indicating that the authentication failed. Additionally, an ERROR event will be emitted to signal that a general error has occurred. +If the request is aborted due to this timeout the SDK will fail to initialize and an `ERROR_AUTH` and `ERROR` event will be emitted. **This only applies to the authentiaction request. If you wish to set a read timeout on the remaining requests made by the SDK, you may register [API Middleware](#api-middleware) @@ -375,7 +375,7 @@ function abortControllerMiddleware([url, options]) { // Register the middleware client.registerAPIRequestMiddleware(abortControllerMiddleware); ``` -This middleware will automatically attach an AbortController to each request, which will abort the request if it takes longer than the specified timeout. You can also customize the middleware to perform other actions, such as logging or modifying headers. +This example middleware will automatically attach an AbortController to each request, which will abort the request if it takes longer than the specified timeout. You can also customize the middleware to perform other actions, such as logging or modifying headers. ## Logging From 2783dc9b02e690be86ab984f8941ef05d01c325e Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Tue, 3 Sep 2024 11:30:05 +0100 Subject: [PATCH 5/6] FFM-11972 Refactor to only call fetch once --- src/index.ts | 49 ++++++++++++++++++------------------------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/src/index.ts b/src/index.ts index 559c0e6..457d57c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -157,41 +157,19 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = }) } - let timeoutId: any + let timeoutId: number | undefined + let abortController: AbortController | undefined - if (window.AbortController) { - const abortController = new AbortController() + if (window.AbortController && configurations.authRequestReadTimeout > 0) { + abortController = new AbortController() requestOptions.signal = abortController.signal - if (configurations.authRequestReadTimeout > 0) { - timeoutId = setTimeout(() => abortController.abort(), configuration.authRequestReadTimeout) - } - - try { - const response = await fetch(url, requestOptions) - - if (!response.ok) { - throw new Error(`Http error: ${response.status}: ${response.statusText}`) - } - - const data: { authToken: string } = await response.json() - return data.authToken - } catch (error) { - if (abortController.signal.aborted) { - throw new Error('Request timed out') - } - - throw error - } finally { - if (timeoutId) { - clearTimeout(timeoutId) - } - } - } else { - if (configurations.authRequestReadTimeout > 0) { - logWarn('AbortController is not available, auth request will not timeout') - } + timeoutId = window.setTimeout(() => abortController.abort(), configuration.authRequestReadTimeout) + } else if (configuration.authRequestReadTimeout > 0) { + logWarn('AbortController is not available, auth request will not timeout') + } + try { const response = await fetch(url, requestOptions) if (!response.ok) { @@ -200,6 +178,15 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = const data: { authToken: string } = await response.json() return data.authToken + } catch (error) { + if (abortController && abortController.signal.aborted) { + throw new Error(`Request to ${url} failed: Request timeout via configured authRequestTimeout of ${configurations.authRequestReadTimeout}`) + } + throw new Error(`Request to ${url} failed: ${error.message}`); + } finally { + if (timeoutId) { + clearTimeout(timeoutId) + } } } From 7e1b72e1c4b1868f40d0d0f9f7cd20945dd1b751 Mon Sep 17 00:00:00 2001 From: Erdi Rowlands Date: Tue, 3 Sep 2024 12:14:18 +0100 Subject: [PATCH 6/6] FFM-11972 Tweak error log for failed auth requests --- src/index.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/index.ts b/src/index.ts index 457d57c..ccaced9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -173,16 +173,19 @@ const initialize = (apiKey: string, target: Target, options?: Options): Result = const response = await fetch(url, requestOptions) if (!response.ok) { - throw new Error(`Http error: ${response.status}: ${response.statusText}`) + throw new Error(`${response.status}: ${response.statusText}`) } const data: { authToken: string } = await response.json() return data.authToken } catch (error) { if (abortController && abortController.signal.aborted) { - throw new Error(`Request to ${url} failed: Request timeout via configured authRequestTimeout of ${configurations.authRequestReadTimeout}`) + throw new Error( + `Request to ${url} failed: Request timeout via configured authRequestTimeout of ${configurations.authRequestReadTimeout}` + ) } - throw new Error(`Request to ${url} failed: ${error.message}`); + const errorMessage = error instanceof Error ? error.message : String(error) + throw new Error(`Request to ${url} failed: ${errorMessage}`) } finally { if (timeoutId) { clearTimeout(timeoutId)