From 0dd41b9c95b709cfa92ab7cece1dccfe2b474210 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 15 Apr 2025 12:02:32 -0700 Subject: [PATCH 01/22] feat: Add stream vardata --- packages/experiment-browser/package.json | 1 + packages/experiment-browser/src/config.ts | 14 + .../src/experimentClient.ts | 146 ++++------ .../src/transport/stream.ts | 17 ++ .../src/util/variantsUpdater.ts | 269 ++++++++++++++++++ .../experiment-browser/test/client.test.ts | 28 +- .../src/api/evaluation-stream-api.ts | 150 ++++++++++ packages/experiment-core/src/index.ts | 5 + .../experiment-core/src/transport/stream.ts | 159 +++++++++++ yarn.lock | 152 +--------- 10 files changed, 694 insertions(+), 247 deletions(-) create mode 100644 packages/experiment-browser/src/transport/stream.ts create mode 100644 packages/experiment-browser/src/util/variantsUpdater.ts create mode 100644 packages/experiment-core/src/api/evaluation-stream-api.ts create mode 100644 packages/experiment-core/src/transport/stream.ts diff --git a/packages/experiment-browser/package.json b/packages/experiment-browser/package.json index f29d002d..bd8526f7 100644 --- a/packages/experiment-browser/package.json +++ b/packages/experiment-browser/package.json @@ -38,6 +38,7 @@ "@amplitude/experiment-core": "^0.11.0", "@amplitude/ua-parser-js": "^0.7.31", "base64-js": "1.5.1", + "eventsource": "^2", "unfetch": "4.1.0" }, "devDependencies": { diff --git a/packages/experiment-browser/src/config.ts b/packages/experiment-browser/src/config.ts index b181b737..d669f367 100644 --- a/packages/experiment-browser/src/config.ts +++ b/packages/experiment-browser/src/config.ts @@ -94,6 +94,18 @@ export interface ExperimentConfig { */ flagConfigPollingIntervalMillis?: number; + /** + * If true, the client will stream updates for remote evaluation from the server. + * fetch() will update variants and initiate a connection to the server. + */ + streamVariants?: boolean; + + /** + * The URL to stream remote evaluation updates from. This is only used if + * `streamVariants` is `true`. + */ + streamVariantsServerUrl?: string; + /** * Explicitly enable or disable calling {@link fetch()} on {@link start()}: * @@ -190,6 +202,8 @@ export const Defaults: ExperimentConfig = { automaticExposureTracking: true, pollOnStart: true, flagConfigPollingIntervalMillis: 300000, + streamVariants: false, + streamVariantsServerUrl: 'https://stream.lab.amplitude.com', fetchOnStart: true, automaticFetchOnAmplitudeIdentityChange: false, userProvider: null, diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index eab54be9..59c7baf2 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -12,6 +12,7 @@ import { Poller, SdkEvaluationApi, SdkFlagApi, + SdkStreamEvaluationApi, TimeoutError, topologicalSort, } from '@amplitude/experiment-core'; @@ -29,6 +30,7 @@ import { import { LocalStorage } from './storage/local-storage'; import { SessionStorage } from './storage/session-storage'; import { FetchHttpClient, WrapperClient } from './transport/http'; +import { defaultSseProvider } from './transport/stream'; import { exposureEvent } from './types/analytics'; import { Client, FetchOptions } from './types/client'; import { Exposure, ExposureTrackingProvider } from './types/exposure'; @@ -49,20 +51,22 @@ import { convertUserToContext, convertVariant, } from './util/convert'; +import { + VariantsFetchUpdater, + VariantsRetryAndFallbackWrapperUpdater, + VariantsStreamUpdater, + VariantUpdater, +} from './util/variantsUpdater'; import { SessionAnalyticsProvider } from './util/sessionAnalyticsProvider'; import { SessionExposureTrackingProvider } from './util/sessionExposureTrackingProvider'; // Configs which have been removed from the public API. // May be added back in the future. -const fetchBackoffTimeout = 10000; -const fetchBackoffAttempts = 8; -const fetchBackoffMinMillis = 500; -const fetchBackoffMaxMillis = 10000; -const fetchBackoffScalar = 1.5; const minFlagPollerIntervalMillis = 60000; const euServerUrl = 'https://api.lab.eu.amplitude.com'; const euFlagsServerUrl = 'https://flag.lab.eu.amplitude.com'; +const euStreamVariantsServerUrl = 'https://stream.lab.eu.amplitude.com'; /** * The default {@link Client} used to fetch variations from Experiment's @@ -76,7 +80,7 @@ export class ExperimentClient implements Client { private readonly variants: LoadStoreCache; private readonly flags: LoadStoreCache; private readonly flagApi: FlagApi; - private readonly evaluationApi: EvaluationApi; + private readonly variantUpdater: VariantUpdater; private readonly engine: EvaluationEngine = new EvaluationEngine(); private user: ExperimentUser | undefined; private userProvider: ExperimentUserProvider | undefined; @@ -117,6 +121,11 @@ export class ExperimentClient implements Client { (config?.serverZone?.toLowerCase() === 'eu' ? euFlagsServerUrl : Defaults.flagsServerUrl), + streamVariantsServerUrl: + config?.streamVariantsServerUrl || + (config?.serverZone?.toLowerCase() === 'eu' + ? euStreamVariantsServerUrl + : Defaults.streamVariantsServerUrl), // Force minimum flag config polling interval. flagConfigPollingIntervalMillis: config.flagConfigPollingIntervalMillis < minFlagPollerIntervalMillis @@ -161,11 +170,28 @@ export class ExperimentClient implements Client { this.config.flagsServerUrl, httpClient, ); - this.evaluationApi = new SdkEvaluationApi( + const evaluationApi = new SdkEvaluationApi( this.apiKey, this.config.serverUrl, httpClient, ); + this.variantUpdater = new VariantsFetchUpdater(evaluationApi); + if (config.streamVariants) { + const streamEvaluationApi = new SdkStreamEvaluationApi( + this.apiKey, + this.config.streamVariantsServerUrl, + defaultSseProvider, + 3000, + // config.streamVariantsConnTimeoutMillis, + evaluationApi, + ); + const streamUpdater = new VariantsStreamUpdater(streamEvaluationApi); + this.variantUpdater = new VariantsRetryAndFallbackWrapperUpdater( + streamUpdater, + this.variantUpdater, + 2 * 60 * 1000, + ); + } // Storage & Caching let storage: Storage; const storageInstanceName = internalInstanceName @@ -267,23 +293,26 @@ export class ExperimentClient implements Client { user: ExperimentUser = this.user, options?: FetchOptions, ): Promise { + // Don't even try to fetch variants if API key is not set + if (!this.apiKey) { + throw Error('Experiment API key is empty'); + } this.setUser(user || {}); - try { - await this.fetchInternal( - user, - this.config.fetchTimeoutMillis, - this.config.retryFetchOnFailure, - options, - ); - } catch (e) { - if (this.config.debug) { - if (e instanceof TimeoutError) { - console.debug(e); - } else { - console.error(e); + user = await this.addContextOrWait(user); + user = this.cleanUserPropsForFetch(user); + await this.variantUpdater.start( + async (results: Record) => { + const variants: Variants = {}; + for (const key of Object.keys(results)) { + variants[key] = convertEvaluationVariantToVariant(results[key]); } - } - } + await this.storeVariants(variants, options); + }, + (err) => { + console.error(err); + }, + { user, options, config: this.config }, + ); return this; } @@ -665,63 +694,12 @@ export class ExperimentClient implements Client { return defaultSourceVariant; } - private async fetchInternal( - user: ExperimentUser, - timeoutMillis: number, - retry: boolean, - options?: FetchOptions, - ): Promise { - // Don't even try to fetch variants if API key is not set - if (!this.apiKey) { - throw Error('Experiment API key is empty'); - } - - this.debug(`[Experiment] Fetch all: retry=${retry}`); - - // Proactively cancel retries if active in order to avoid unnecessary API - // requests. A new failure will restart the retries. - if (retry) { - this.stopRetries(); - } - - try { - const variants = await this.doFetch(user, timeoutMillis, options); - await this.storeVariants(variants, options); - return variants; - } catch (e) { - if (retry && this.shouldRetryFetch(e)) { - void this.startRetries(user, options); - } - throw e; - } - } - private cleanUserPropsForFetch(user: ExperimentUser): ExperimentUser { const cleanedUser = { ...user }; delete cleanedUser.cookie; return cleanedUser; } - private async doFetch( - user: ExperimentUser, - timeoutMillis: number, - options?: FetchOptions, - ): Promise { - user = await this.addContextOrWait(user); - user = this.cleanUserPropsForFetch(user); - this.debug('[Experiment] Fetch variants for user: ', user); - const results = await this.evaluationApi.getVariants(user, { - timeoutMillis: timeoutMillis, - ...options, - }); - const variants: Variants = {}; - for (const key of Object.keys(results)) { - variants[key] = convertEvaluationVariantToVariant(results[key]); - } - this.debug('[Experiment] Received variants: ', variants); - return variants; - } - private async doFlags(): Promise { try { let user: ExperimentUser; @@ -780,28 +758,6 @@ export class ExperimentClient implements Client { this.debug('[Experiment] Stored variants: ', variants); } - private async startRetries( - user: ExperimentUser, - options: FetchOptions, - ): Promise { - this.debug('[Experiment] Retry fetch'); - this.retriesBackoff = new Backoff( - fetchBackoffAttempts, - fetchBackoffMinMillis, - fetchBackoffMaxMillis, - fetchBackoffScalar, - ); - void this.retriesBackoff.start(async () => { - await this.fetchInternal(user, fetchBackoffTimeout, false, options); - }); - } - - private stopRetries(): void { - if (this.retriesBackoff) { - this.retriesBackoff.cancel(); - } - } - private addContext(user: ExperimentUser): ExperimentUser { const providedUser = this.userProvider?.getUser(); const integrationUser = this.integrationManager.getUser(); diff --git a/packages/experiment-browser/src/transport/stream.ts b/packages/experiment-browser/src/transport/stream.ts new file mode 100644 index 00000000..c368ec78 --- /dev/null +++ b/packages/experiment-browser/src/transport/stream.ts @@ -0,0 +1,17 @@ +/** + * @packageDocumentation + * @internal + */ + +import { SSE } from '@amplitude/experiment-core'; +import EventSource from 'eventsource'; + +export const defaultSseProvider = ( + url: string, + headers: Record, +): SSE => { + const es = new EventSource(url, { + headers, + }); + return es; +}; diff --git a/packages/experiment-browser/src/util/variantsUpdater.ts b/packages/experiment-browser/src/util/variantsUpdater.ts new file mode 100644 index 00000000..272a8589 --- /dev/null +++ b/packages/experiment-browser/src/util/variantsUpdater.ts @@ -0,0 +1,269 @@ +import { + EvaluationApi, + EvaluationVariant, + FetchError, + GetVariantsOptions, + StreamEvaluationApi, + TimeoutError, +} from '@amplitude/experiment-core'; + +import { ExperimentConfig } from '../config'; +import { FetchOptions } from '../types/client'; +import { ExperimentUser } from '../types/user'; +import { Variant } from '../types/variant'; + +import { Backoff } from './backoff'; + +export interface Updater { + start( + onUpdate: (data: unknown) => void, + onError: (err: Error) => void, + params?: Record, + ): Promise; + stop(): Promise; +} + +type VariantUpdateCallback = (data: Record) => void; +type VariantErrorCallback = (err: Error) => void; +type VariantUpdaterParams = { + user: ExperimentUser; + config: ExperimentConfig; + options: GetVariantsOptions; +}; +export interface VariantUpdater extends Updater { + start( + onUpdate: VariantUpdateCallback, + onError: VariantErrorCallback, + params: VariantUpdaterParams, + ): Promise; + stop(): Promise; +} + +export class VariantsStreamUpdater implements VariantUpdater { + private evaluationApi: StreamEvaluationApi; + + constructor(evaluationApi: StreamEvaluationApi) { + this.evaluationApi = evaluationApi; + } + + async start( + onUpdate: VariantUpdateCallback, + onError: VariantErrorCallback, + params: VariantUpdaterParams, + ): Promise { + this.evaluationApi.streamVariants( + params.user, + params.options, + onUpdate, + onError, + ); + } + + async stop(): Promise { + this.evaluationApi.close(); + } +} + +const fetchBackoffTimeout = 10000; +const fetchBackoffAttempts = 8; +const fetchBackoffMinMillis = 500; +const fetchBackoffMaxMillis = 10000; +const fetchBackoffScalar = 1.5; + +export class VariantsFetchUpdater implements Updater { + private evaluationApi: EvaluationApi; + retriesBackoff: Backoff; + constructor(evaluationApi: EvaluationApi) { + this.evaluationApi = evaluationApi; + } + + async start( + onUpdate: VariantUpdateCallback, + onError: VariantErrorCallback, + params: VariantUpdaterParams, + ): Promise { + const { user, config, options } = params; + + try { + if (config.retryFetchOnFailure) { + this.stopRetries(); + } + + try { + await this.fetchInternal( + user, + options, + config.fetchTimeoutMillis, + onUpdate, + ); + } catch (e) { + if (config.retryFetchOnFailure && this.shouldRetryFetch(e)) { + void this.startRetries(user, options, onUpdate); + } + throw e; + } + } catch (e) { + if (config.debug) { + if (e instanceof TimeoutError) { + console.debug(e); + } else { + console.error(e); + } + } + } + } + + private async fetchInternal( + user: ExperimentUser, + options: FetchOptions, + timeoutMillis: number, + onUpdate: (data: Record) => void, + ): Promise { + const results = await this.evaluationApi.getVariants(user, { + timeoutMillis: timeoutMillis, + ...options, + }); + onUpdate(results); + } + + private shouldRetryFetch(e: Error): boolean { + if (e instanceof FetchError) { + const fetchErr = e as FetchError; + return ( + fetchErr.statusCode < 400 || + fetchErr.statusCode >= 500 || + fetchErr.statusCode === 429 + ); + } + return true; + } + + private async startRetries( + user: ExperimentUser, + options: FetchOptions, + onUpdate: (data: Record) => void, + ): Promise { + // this.debug('[Experiment] Retry fetch'); + this.retriesBackoff = new Backoff( + fetchBackoffAttempts, + fetchBackoffMinMillis, + fetchBackoffMaxMillis, + fetchBackoffScalar, + ); + void this.retriesBackoff.start(async () => { + await this.fetchInternal(user, options, fetchBackoffTimeout, onUpdate); + }); + } + + private stopRetries(): void { + if (this.retriesBackoff) { + this.retriesBackoff.cancel(); + } + } + async stop(): Promise { + this.stopRetries(); + } +} + +/** + * This class retries the main updater and, if it fails, falls back to the fallback updater. + * The main updater will keep retrying every set interval and, if succeeded, the fallback updater will be stopped. + */ + +export class RetryAndFallbackWrapperUpdater implements Updater { + private readonly mainUpdater: Updater; + private readonly fallbackUpdater: Updater; + private readonly retryIntervalMillis: number; + private retryTimer: NodeJS.Timeout | null = null; + + constructor( + mainUpdater: Updater, + fallbackUpdater: Updater, + retryIntervalMillis: number, + ) { + this.mainUpdater = mainUpdater; + this.fallbackUpdater = fallbackUpdater; + this.retryIntervalMillis = retryIntervalMillis; + } + + async start( + onUpdate: VariantUpdateCallback, + onError: VariantErrorCallback, + params: VariantUpdaterParams, + ): Promise { + this.stop(); + + try { + await this.mainUpdater.start( + onUpdate, + (err) => { + this.fallbackUpdater.start(onUpdate, onError, params); + this.startRetryTimer(onUpdate, onError, params); + }, + params, + ); + } catch (error) { + await this.fallbackUpdater.start(onUpdate, onError, params); + this.startRetryTimer(onUpdate, onError, params); + } + } + + async startRetryTimer(onUpdate, onError, params) { + if (this.retryTimer) { + clearInterval(this.retryTimer); + this.retryTimer = null; + } + + const retryTimer = setInterval(async () => { + try { + await this.mainUpdater.start( + onUpdate, + (err) => { + this.fallbackUpdater.start(onUpdate, onError, params); + this.startRetryTimer(onUpdate, onError, params); + }, + params, + ); + this.fallbackUpdater.stop(); + clearInterval(retryTimer); + if (this.retryTimer) { + clearInterval(this.retryTimer); + this.retryTimer = null; + } + } catch (error) { + console.error('Retry failed', error); + } + }, this.retryIntervalMillis); + this.retryTimer = retryTimer; + } + + async stop(): Promise { + if (this.retryTimer) { + clearInterval(this.retryTimer); + this.retryTimer = null; + } + await this.mainUpdater.stop(); + await this.fallbackUpdater.stop(); + } +} + +export class VariantsRetryAndFallbackWrapperUpdater + extends RetryAndFallbackWrapperUpdater + implements VariantUpdater +{ + constructor( + mainUpdater: VariantUpdater, + fallbackUpdater: VariantUpdater, + retryIntervalMillis: number, + ) { + super(mainUpdater, fallbackUpdater, retryIntervalMillis); + } + + async start( + onUpdate: VariantUpdateCallback, + onError: VariantErrorCallback, + params: VariantUpdaterParams, + ): Promise { + super.start(onUpdate, onError, params); + } +} diff --git a/packages/experiment-browser/test/client.test.ts b/packages/experiment-browser/test/client.test.ts index c4f10617..d85f3138 100644 --- a/packages/experiment-browser/test/client.test.ts +++ b/packages/experiment-browser/test/client.test.ts @@ -1,5 +1,10 @@ import { AnalyticsConnector } from '@amplitude/analytics-connector'; -import { FetchError, safeGlobal } from '@amplitude/experiment-core'; +import { + EvaluationVariant, + FetchError, + safeGlobal, + SdkEvaluationApi, +} from '@amplitude/experiment-core'; import { Defaults } from 'src/config'; import { ExperimentEvent, IntegrationPlugin } from 'src/types/plugin'; @@ -20,6 +25,7 @@ import { HttpClient, SimpleResponse } from '../src/types/transport'; import { randomString } from '../src/util/randomstring'; import { mockClientStorage } from './util/mock'; +import { VariantsFetchUpdater } from '../src/util/variantsUpdater'; const delay = (ms: number) => new Promise((res) => setTimeout(res, ms)); @@ -1129,20 +1135,22 @@ describe('fetch retry with different response codes', () => { mockClientStorage(client); jest - .spyOn(ExperimentClient.prototype as any, 'doFetch') + .spyOn(SdkEvaluationApi.prototype as any, 'getVariants') .mockImplementation( async (user?: ExperimentUser, options?: FetchOptions) => { - return new Promise((resolve, reject) => { - if (responseCode === 0) { - reject(new Error(errorMessage)); - } else { - reject(new FetchError(responseCode, errorMessage)); - } - }); + return new Promise>( + (resolve, reject) => { + if (responseCode === 0) { + reject(new Error(errorMessage)); + } else { + reject(new FetchError(responseCode, errorMessage)); + } + }, + ); }, ); const retryMock = jest.spyOn( - ExperimentClient.prototype as any, + VariantsFetchUpdater.prototype as any, 'startRetries', ); try { diff --git a/packages/experiment-core/src/api/evaluation-stream-api.ts b/packages/experiment-core/src/api/evaluation-stream-api.ts new file mode 100644 index 00000000..d77f967e --- /dev/null +++ b/packages/experiment-core/src/api/evaluation-stream-api.ts @@ -0,0 +1,150 @@ +import { Base64 } from 'js-base64'; +import { DEFAULT_EVENT_TYPE, SSEProvider, SSEStream } from 'transport/stream'; + +import { EvaluationVariant } from '../evaluation/flag'; + +import { EvaluationApi, GetVariantsOptions } from './evaluation-api'; + +const STREAM_CONNECTION_TIMEOUT_MILLIS = 3000; + +export interface StreamEvaluationApi { + streamVariants( + user: Record, + options?: GetVariantsOptions, + onUpdate?: (variants: Record) => void, + onError?: (error: Error) => void, + ): Promise; + close(): Promise; +} + +export class SdkStreamEvaluationApi implements StreamEvaluationApi { + private readonly deploymentKey: string; + private readonly serverUrl: string; + private readonly sseProvider: SSEProvider; + private readonly fetchEvalApi?: EvaluationApi; + private readonly connectionTimeoutMillis: number; + + private stream?: SSEStream; + + constructor( + deploymentKey: string, + serverUrl: string, + sseProvider: SSEProvider, + connectionTimeoutMillis = STREAM_CONNECTION_TIMEOUT_MILLIS, + fetchEvalApi?: EvaluationApi, + ) { + this.deploymentKey = deploymentKey; + this.serverUrl = serverUrl; + this.sseProvider = sseProvider; + this.connectionTimeoutMillis = connectionTimeoutMillis; + this.fetchEvalApi = fetchEvalApi; + } + + async streamVariants( + user: Record, + options?: GetVariantsOptions, + onUpdate?: (variants: Record) => void, + onError?: (error: Error) => void, + ): Promise { + if (this.stream) { + this.close(); + } + + const userJsonBase64 = Base64.encodeURL(JSON.stringify(user)); + const headers: Record = { + Authorization: `Api-Key ${this.deploymentKey}`, + 'X-Amp-Exp-User': userJsonBase64, + }; + if (options?.flagKeys) { + headers['X-Amp-Exp-Flag-Keys'] = Base64.encodeURL( + JSON.stringify(options.flagKeys), + ); + } + if (options?.trackingOption) { + headers['X-Amp-Exp-Track'] = options.trackingOption; + } + + const url = new URL(`${this.serverUrl}/sdk/stream/v1/vardata`); + if (options?.evaluationMode) { + url.searchParams.append('eval_mode', options?.evaluationMode); + } + if (options?.deliveryMethod) { + url.searchParams.append('delivery_method', options?.deliveryMethod); + } + + return new Promise((resolve, reject) => { + this.stream = new SSEStream(this.sseProvider, url.toString(), headers); + let isConnecting = true; + + const connectionTimeout = setTimeout(() => { + if (isConnecting) { + this.close(); + reject(new Error('Connection timed out.')); + } + }, this.connectionTimeoutMillis); + + const onErrorSseCb = (error: Error) => { + if (isConnecting) { + isConnecting = false; + this.close(); + clearTimeout(connectionTimeout); + reject(error); + } else { + this.close(); + onError?.(error); + } + }; + const onDataUpdateSseCb = (data: string) => { + if (isConnecting) { + isConnecting = false; + const variants = JSON.parse(data); + clearTimeout(connectionTimeout); + resolve(); + onUpdate?.(variants); + } else { + onUpdate?.(JSON.parse(data)); + } + }; + const onSignalUpdateCb = async () => { + // Signaled that there's a push. + if (isConnecting) { + isConnecting = false; + clearTimeout(connectionTimeout); + resolve(); + } + if (!this.fetchEvalApi) { + onErrorSseCb( + new Error( + 'No fetchEvalApi provided for variant data that is too large to push.', + ), + ); + return; + } + + let variants; + try { + variants = await this.fetchEvalApi.getVariants(user, options); + } catch (error) { + onErrorSseCb( + new Error('Error fetching variants on signal: ' + error), + ); + } + onUpdate?.(variants || {}); + }; + + this.stream.connect( + { + push_data: onDataUpdateSseCb, + push_signal: onSignalUpdateCb, + [DEFAULT_EVENT_TYPE]: onDataUpdateSseCb, + }, + onErrorSseCb, + ); + }); + } + + async close(): Promise { + this.stream?.close(); + this.stream = undefined; + } +} diff --git a/packages/experiment-core/src/index.ts b/packages/experiment-core/src/index.ts index d5f1bb83..60cd3c53 100644 --- a/packages/experiment-core/src/index.ts +++ b/packages/experiment-core/src/index.ts @@ -17,6 +17,11 @@ export { } from './api/evaluation-api'; export { FlagApi, SdkFlagApi, GetFlagsOptions } from './api/flag-api'; export { HttpClient, HttpRequest, HttpResponse } from './transport/http'; +export { + StreamEvaluationApi, + SdkStreamEvaluationApi, +} from './api/evaluation-stream-api'; +export { SSE, SSEStream, SSEProvider } from './transport/stream'; export { Poller } from './util/poller'; export { safeGlobal, diff --git a/packages/experiment-core/src/transport/stream.ts b/packages/experiment-core/src/transport/stream.ts new file mode 100644 index 00000000..6fe04f2e --- /dev/null +++ b/packages/experiment-core/src/transport/stream.ts @@ -0,0 +1,159 @@ +const KEEP_ALIVE_DATA = ' '; +const KEEP_ALIVE_INTERVAL_MILLIS = 33000; // 30 seconds with a 3 seconds buffer +const RECONNECTION_INTERVAL_MILLIS = 30 * 60 * 1000; +export const DEFAULT_EVENT_TYPE = 'message'; + +export interface ErrorEvent { + code?: number; + message?: string; +} + +export interface SSE { + addEventListener( + event: string, + callback: (event: MessageEvent | ErrorEvent) => void, + ): void; + close(): void; +} +export type SSEProvider = (url: string, headers: Record) => SSE; + +export class SSEStream { + streamProvider: SSEProvider; + url: string; + headers: Record; + + es?: SSE; + onEventTypeUpdate: Record void> = {}; + onError?: (error: Error) => void; + keepAliveInterval: number; + keepAliveTimer?: NodeJS.Timeout; + reconnectionIntervalMillisMin: number; + reconnectionIntervalMillisRange: number; + reconnectionTimeout?: NodeJS.Timeout; + + constructor( + streamProvider: SSEProvider, + url: string, + headers: Record, + keepAliveInterval = KEEP_ALIVE_INTERVAL_MILLIS, + reconnectionIntervalMillis = RECONNECTION_INTERVAL_MILLIS, + ) { + this.streamProvider = streamProvider; + this.url = url; + this.headers = headers; + this.keepAliveInterval = keepAliveInterval; + // Make the jitter range to be 0.9 to 1.1 of the reconnection interval. + this.reconnectionIntervalMillisMin = Math.floor( + reconnectionIntervalMillis * 0.9, + ); + this.reconnectionIntervalMillisRange = + Math.ceil(reconnectionIntervalMillis * 1.1) - + this.reconnectionIntervalMillisMin; + } + + /** + * Connect to the stream and listen for updates. + * Autonmatically handles keep alive packets and reconnection after a long period of time. + * Whenever there's an error, the stream is closed, then onError callback is called. + * @param onUpdate If onUpdate is provided, it will be called with the data received from the stream with DEFAULT_EVENT_TYPE. + * @param onEventTypeUpdate A mapping from event type to callbacks. Routes the event data for different event types received from the stream to different callbacks. + * @param onError If onError is provided, it will be called with the error received from the stream. + */ + connect( + onUpdate: (data: string) => void, + onError?: (error: Error) => void, + ): void; + connect( + onEventTypeUpdate: Record void>, + onError?: (error: Error) => void, + ): void; + connect( + onUpdateCb: + | ((data: string) => void) + | Record void>, + onError?: (error: Error) => void, + ): void { + if (typeof onUpdateCb === 'function') { + // onUpdate: (data: string) => void, + this.onEventTypeUpdate = { [DEFAULT_EVENT_TYPE]: onUpdateCb }; + } else { + // onEventTypeUpdate: Record void>, + this.onEventTypeUpdate = onUpdateCb as Record< + string, + (data: string) => void + >; + if (!(DEFAULT_EVENT_TYPE in this.onEventTypeUpdate)) { + // Ensure there's always a default to receive keep alive data. + // eslint-disable-next-line @typescript-eslint/no-empty-function + this.onEventTypeUpdate[DEFAULT_EVENT_TYPE] = () => {}; + } + } + this.onError = onError; + if (this.es) { + this.close(); + } + + this.es = this.streamProvider(this.url, this.headers); + for (const eventType in this.onEventTypeUpdate) { + this.es.addEventListener(eventType, (event) => { + this.resetKeepAlive(); + event = event as MessageEvent; + if (event.data === KEEP_ALIVE_DATA) { + // This is a keep-alive message, ignore it + return; + } + this.onEventTypeUpdate[eventType](event.data); + }); + } + this.es.addEventListener('error', (error) => { + this.close(); + error = error as ErrorEvent; + this.onError?.(Error(`Error in stream: ${JSON.stringify(error)}`)); + }); + this.resetKeepAlive(); + this.setReconnectionTimeout(); + } + + resetKeepAlive(): void { + if (this.keepAliveTimer) { + clearTimeout(this.keepAliveTimer); + this.keepAliveTimer = undefined; + } + if (this.es) { + this.keepAliveTimer = setTimeout(() => { + this.close(); + this.onError?.(Error('Keep-alive timeout')); + }, this.keepAliveInterval); + } + } + + setReconnectionTimeout(): void { + if (this.reconnectionTimeout) { + clearTimeout(this.reconnectionTimeout); + this.reconnectionTimeout = undefined; + } + if (this.es) { + if ( + this.reconnectionIntervalMillisMin > 0 && + this.reconnectionIntervalMillisRange > 0 + ) { + this.reconnectionTimeout = setTimeout(() => { + this.connect(this.onEventTypeUpdate, this.onError); + }, Math.ceil(Math.random() * this.reconnectionIntervalMillisRange + this.reconnectionIntervalMillisMin)); + } + } + } + + close(): void { + if (this.keepAliveTimer) { + clearTimeout(this.keepAliveTimer); + this.keepAliveTimer = undefined; + } + if (this.reconnectionTimeout) { + clearTimeout(this.reconnectionTimeout); + this.reconnectionTimeout = undefined; + } + this.es?.close(); + this.es = undefined; + } +} diff --git a/yarn.lock b/yarn.lock index 368d5ae4..52533d9d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -47,14 +47,6 @@ dependencies: "@babel/highlight" "^7.22.5" -"@babel/code-frame@^7.10.4": - version "7.22.13" - resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.22.13.tgz#e3c1c099402598483b7a8c46a721d1038803755e" - integrity sha512-XktuhWlJ5g+3TJXc5upd9Ks1HutSArik6jf2eAjYFyIOf4ej3RN+184cZbzDvbPnuTJIUhPKKJE3cIsYTiAT3w== - dependencies: - "@babel/highlight" "^7.22.13" - chalk "^2.4.2" - "@babel/compat-data@^7.22.5", "@babel/compat-data@^7.22.6", "@babel/compat-data@^7.22.9": version "7.22.9" resolved "https://registry.yarnpkg.com/@babel/compat-data/-/compat-data-7.22.9.tgz#71cdb00a1ce3a329ce4cbec3a44f9fef35669730" @@ -289,15 +281,6 @@ chalk "^2.0.0" js-tokens "^4.0.0" -"@babel/highlight@^7.22.13": - version "7.22.13" - resolved "https://registry.yarnpkg.com/@babel/highlight/-/highlight-7.22.13.tgz#9cda839e5d3be9ca9e8c26b6dd69e7548f0cbf16" - integrity sha512-C/BaXcnnvBCmHTpz/VGZ8jgtE2aYlW4hxDhseJAWZb7gqGM/qtCK6iZUb0TyKFf7BOUsBH7Q7fkRsDRhg1XklQ== - dependencies: - "@babel/helper-validator-identifier" "^7.22.5" - chalk "^2.4.2" - js-tokens "^4.0.0" - "@babel/parser@^7.1.0", "@babel/parser@^7.14.7", "@babel/parser@^7.20.7", "@babel/parser@^7.22.5", "@babel/parser@^7.22.7": version "7.22.7" resolved "https://registry.yarnpkg.com/@babel/parser/-/parser-7.22.7.tgz#df8cf085ce92ddbdbf668a7f186ce848c9036cae" @@ -991,21 +974,6 @@ resolved "https://registry.yarnpkg.com/@babel/regjsgen/-/regjsgen-0.8.0.tgz#f0ba69b075e1f05fb2825b7fad991e7adbb18310" integrity sha512-x/rqGMdzj+fWZvCOYForTghzbtqPDZ5gPwaoNGHdgDfF2QA/XZbCBp4Moo5scrkAMPhB7z26XM/AaHuIJdgauA== -"@babel/runtime-corejs3@^7.10.2": - version "7.22.11" - resolved "https://registry.yarnpkg.com/@babel/runtime-corejs3/-/runtime-corejs3-7.22.11.tgz#bf65b846cb4a03e1594dba9850c4632a992ddc04" - integrity sha512-NhfzUbdWbiE6fCFypbWCPu6AR8xre31EOPF7wwAIJEvGQ2avov04eymayWinCuyXmV1b0+jzoXP/HYzzUYdvwg== - dependencies: - core-js-pure "^3.30.2" - regenerator-runtime "^0.14.0" - -"@babel/runtime@^7.10.2", "@babel/runtime@^7.10.3": - version "7.22.11" - resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.22.11.tgz#7a9ba3bbe406ad6f9e8dd4da2ece453eb23a77a4" - integrity sha512-ee7jVNlWN09+KftVOu9n7S8gQzD/Z6hN/I8VBRXW4P1+Xe7kJGXMwu8vds4aGIMHZnNbdpSWCfZZtinytpcAvA== - dependencies: - regenerator-runtime "^0.14.0" - "@babel/runtime@^7.11.2", "@babel/runtime@^7.21.0", "@babel/runtime@^7.8.4": version "7.22.6" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.22.6.tgz#57d64b9ae3cff1d67eb067ae117dac087f5bd438" @@ -1345,17 +1313,6 @@ slash "^3.0.0" write-file-atomic "^4.0.2" -"@jest/types@^26.6.2": - version "26.6.2" - resolved "https://registry.yarnpkg.com/@jest/types/-/types-26.6.2.tgz#bef5a532030e1d88a2f5a6d933f84e97226ed48e" - integrity sha512-fC6QCp7Sc5sX6g8Tvbmj4XUTbyrik0akgRy03yjXbQaBWWNWGE7SGtJk98m0N8nzegD/7SggrUlivxo5ax4KWQ== - dependencies: - "@types/istanbul-lib-coverage" "^2.0.0" - "@types/istanbul-reports" "^3.0.0" - "@types/node" "*" - "@types/yargs" "^15.0.0" - chalk "^4.0.0" - "@jest/types@^29.6.1": version "29.6.1" resolved "https://registry.yarnpkg.com/@jest/types/-/types-29.6.1.tgz#ae79080278acff0a6af5eb49d063385aaa897bf2" @@ -2155,20 +2112,6 @@ dependencies: "@sinonjs/commons" "^3.0.0" -"@testing-library/dom@7.26.0": - version "7.26.0" - resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-7.26.0.tgz#da4d052dc426a4ccc916303369c6e7552126f680" - integrity sha512-fyKFrBbS1IigaE3FV21LyeC7kSGF84lqTlSYdKmGaHuK2eYQ/bXVPM5vAa2wx/AU1iPD6oQHsxy2QQ17q9AMCg== - dependencies: - "@babel/code-frame" "^7.10.4" - "@babel/runtime" "^7.10.3" - "@types/aria-query" "^4.2.0" - aria-query "^4.2.2" - chalk "^4.1.0" - dom-accessibility-api "^0.5.1" - lz-string "^1.4.4" - pretty-format "^26.4.2" - "@tootallnate/once@2": version "2.0.0" resolved "https://registry.yarnpkg.com/@tootallnate/once/-/once-2.0.0.tgz#f544a148d3ab35801c1f633a7441fd87c2e484bf" @@ -2192,11 +2135,6 @@ resolved "https://registry.yarnpkg.com/@types/amplitude-js/-/amplitude-js-8.16.2.tgz#2cfb850a01a4171e632498821f92e45bef23c952" integrity sha512-a+tb/CEQOlrHRvEvAuYNOcoUy1POERANnAhfKgiTmsy0eACj3eukGP0ucA9t115QOPzVUhbnUfZqtyHp99IZyA== -"@types/aria-query@^4.2.0": - version "4.2.2" - resolved "https://registry.yarnpkg.com/@types/aria-query/-/aria-query-4.2.2.tgz#ed4e0ad92306a704f9fb132a0cfcf77486dbe2bc" - integrity sha512-HnYpAE1Y6kRyKM/XkEuiRQhTHvkzMBurTHnpFLYLBGPIylZNPs9jJcuOOYWxPLJCSEtmZT0Y8rHDokKN7rRTig== - "@types/babel__core@^7.1.14": version "7.20.1" resolved "https://registry.yarnpkg.com/@types/babel__core/-/babel__core-7.20.1.tgz#916ecea274b0c776fec721e333e55762d3a9614b" @@ -2350,13 +2288,6 @@ resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-21.0.0.tgz#0c60e537fa790f5f9472ed2776c2b71ec117351b" integrity sha512-iO9ZQHkZxHn4mSakYV0vFHAVDyEOIJQrV2uZ06HxEPcx+mt8swXoZHIbaaJ2crJYFfErySgktuTZ3BeLz+XmFA== -"@types/yargs@^15.0.0": - version "15.0.15" - resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-15.0.15.tgz#e609a2b1ef9e05d90489c2f5f45bbfb2be092158" - integrity sha512-IziEYMU9XoVj8hWg7k+UJrXALkGFjWJhn5QFEv9q4p+v40oZhSuC135M38st8XPjICL7Ey4TV64ferBGUoJhBg== - dependencies: - "@types/yargs-parser" "*" - "@types/yargs@^17.0.8": version "17.0.24" resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-17.0.24.tgz#b3ef8d50ad4aa6aecf6ddc97c580a00f5aa11902" @@ -2604,7 +2535,7 @@ ansi-escapes@^4.2.1: dependencies: type-fest "^0.21.3" -ansi-regex@^5.0.0, ansi-regex@^5.0.1: +ansi-regex@^5.0.1: version "5.0.1" resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-5.0.1.tgz#082cb2c89c9fe8659a311a53bd6a4dc5301db304" integrity sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ== @@ -2684,14 +2615,6 @@ argparse@^2.0.1: resolved "https://registry.yarnpkg.com/argparse/-/argparse-2.0.1.tgz#246f50f3ca78a3240f6c997e8a9bd1eac49e4b38" integrity sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q== -aria-query@^4.2.2: - version "4.2.2" - resolved "https://registry.yarnpkg.com/aria-query/-/aria-query-4.2.2.tgz#0d2ca6c9aceb56b8977e9fed6aed7e15bbd2f83b" - integrity sha512-o/HelwhuKpTj/frsOsbNLNgnNGVIFsVP/SW2BSF14gVl7kAfMOJ6/8wUAUvG1R1NHKrfG+2sHZTu0yauT1qBrA== - dependencies: - "@babel/runtime" "^7.10.2" - "@babel/runtime-corejs3" "^7.10.2" - array-buffer-byte-length@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/array-buffer-byte-length/-/array-buffer-byte-length-1.0.0.tgz#fabe8bc193fea865f317fe7807085ee0dee5aead" @@ -3106,7 +3029,7 @@ chalk@4.1.0: ansi-styles "^4.1.0" supports-color "^7.1.0" -chalk@^2.0.0, chalk@^2.4.2: +chalk@^2.0.0: version "2.4.2" resolved "https://registry.yarnpkg.com/chalk/-/chalk-2.4.2.tgz#cd42541677a54333cf541a49108c1432b44c9424" integrity sha512-Mti+f9lpJNcwF4tWV8/OrTTtF1gZi+f8FqlyAdouralcFWFQWF2+NgCHShjkCb+IFBLq9buZwE1xckQU4peSuQ== @@ -3433,11 +3356,6 @@ core-js-compat@^3.31.0: dependencies: browserslist "^4.21.9" -core-js-pure@^3.30.2: - version "3.32.1" - resolved "https://registry.yarnpkg.com/core-js-pure/-/core-js-pure-3.32.1.tgz#5775b88f9062885f67b6d7edce59984e89d276f3" - integrity sha512-f52QZwkFVDPf7UEQZGHKx6NYxsxmVGJe5DIvbzOdRMJlmT6yv0KDjR8rmy3ngr/t5wU54c7Sp/qIJH0ppbhVpQ== - core-util-is@~1.0.0: version "1.0.3" resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.3.tgz#a6042d3634c2b27e9328f837b965fac83808db85" @@ -3656,11 +3574,6 @@ doctrine@^3.0.0: dependencies: esutils "^2.0.2" -dom-accessibility-api@^0.5.1: - version "0.5.16" - resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz#5a7429e6066eb3664d911e33fb0e45de8eb08453" - integrity sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg== - "dom-mutator@git+ssh://git@github.com:amplitude/dom-mutator#main": version "0.7.1" resolved "git+ssh://git@github.com:amplitude/dom-mutator#20ef4fa76e5e35107efd74cac93baf659cd4cabb" @@ -4092,6 +4005,11 @@ events@^3.3.0: resolved "https://registry.yarnpkg.com/events/-/events-3.3.0.tgz#31a95ad0a924e2d2c419a813aeb2c4e878ea7400" integrity sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q== +eventsource@^2: + version "2.0.2" + resolved "https://registry.yarnpkg.com/eventsource/-/eventsource-2.0.2.tgz#76dfcc02930fb2ff339520b6d290da573a9e8508" + integrity sha512-IzUmBGPR3+oUG9dUeXynyNmf91/3zUSJg1lCktzKw47OXuhco54U3r9B7O4XX+Rb1Itm9OZ2b0RkTs10bICOxA== + execa@5.0.0: version "5.0.0" resolved "https://registry.yarnpkg.com/execa/-/execa-5.0.0.tgz#4029b0007998a841fbd1032e5f4de86a3c1e3376" @@ -6086,11 +6004,6 @@ lunr@^2.3.9: resolved "https://registry.yarnpkg.com/lunr/-/lunr-2.3.9.tgz#18b123142832337dd6e964df1a5a7707b25d35e1" integrity sha512-zTU3DaZaF3Rt9rhN3uBMGQD3dD2/vFQqnvZCDv4dl5iOzq2IZQqTxu90r4E5J+nP70J3ilqVCrbho2eWaeW8Ow== -lz-string@^1.4.4: - version "1.5.0" - resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.5.0.tgz#c1ab50f77887b712621201ba9fd4e3a6ed099941" - integrity sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ== - magic-string@^0.25.7: version "0.25.9" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.9.tgz#de7f9faf91ef8a1c91d02c2e5314c8277dbcdd1c" @@ -7218,16 +7131,6 @@ pretty-format@29.4.3: ansi-styles "^5.0.0" react-is "^18.0.0" -pretty-format@^26.4.2: - version "26.6.2" - resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-26.6.2.tgz#e35c2705f14cb7fe2fe94fa078345b444120fc93" - integrity sha512-7AeGuCYNGmycyQbCqd/3PWH4eOoX/OiCa0uphp57NVTeAGdJGaAliecxwBDHYQCIvrW7aDBZCYeNTP/WX69mkg== - dependencies: - "@jest/types" "^26.6.2" - ansi-regex "^5.0.0" - ansi-styles "^4.0.0" - react-is "^17.0.1" - pretty-format@^29.0.0, pretty-format@^29.6.2: version "29.6.2" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-29.6.2.tgz#3d5829261a8a4d89d8b9769064b29c50ed486a47" @@ -7375,11 +7278,6 @@ randombytes@^2.1.0: dependencies: safe-buffer "^5.1.0" -react-is@^17.0.1: - version "17.0.2" - resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0" - integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== - react-is@^18.0.0: version "18.2.0" resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.2.0.tgz#199431eeaaa2e09f86427efbb4f1473edb47609b" @@ -7542,11 +7440,6 @@ regenerator-runtime@^0.13.11: resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.13.11.tgz#f6dca3e7ceec20590d07ada785636a90cdca17f9" integrity sha512-kY1AZVr2Ra+t+piVaJ4gxaFaReZVH40AKNo7UCX6W+dEwBo/2oZJzqfuN1qLq1oL45o56cPaTXELwrTh8Fpggg== -regenerator-runtime@^0.14.0: - version "0.14.0" - resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.14.0.tgz#5e19d68eb12d486f797e15a3c6a918f7cec5eb45" - integrity sha512-srw17NI0TUWHuGa5CFGGmhfNIeja30WMBfbslPNhf6JrqQlLN5gcrvig1oqPxiVaXb0oW0XRKtH6Nngs5lKCIA== - regenerator-transform@^0.15.1: version "0.15.1" resolved "https://registry.yarnpkg.com/regenerator-transform/-/regenerator-transform-0.15.1.tgz#f6c4e99fc1b4591f780db2586328e4d9a9d8dc56" @@ -8040,16 +7933,7 @@ string-length@^4.0.1: char-regex "^1.0.2" strip-ansi "^6.0.0" -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -"string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", "string-width@^1.0.2 || 2 || 3 || 4", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -8108,14 +7992,7 @@ string_decoder@~1.1.1: dependencies: safe-buffer "~5.1.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -8856,16 +8733,7 @@ wordwrap@^1.0.0: resolved "https://registry.yarnpkg.com/wordwrap/-/wordwrap-1.0.0.tgz#27584810891456a4171c8d0226441ade90cbcaeb" integrity sha512-gvVzJFlPycKc5dZN4yPkP8w7Dc37BtP1yczEneOb4uq34pXZcvrtRTmWV8W+Ume+XCxKgbjM+nevkyFPMybd4Q== -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - -wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== From c207f4109cfdc713096a2d5ea4985a73cbf69173 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 16 Apr 2025 03:34:00 -0700 Subject: [PATCH 02/22] fix: error handling, fix types, fix awaits --- .../src/experimentClient.ts | 11 ++- .../src/util/variantsUpdater.ts | 76 ++++++++++++------- .../experiment-browser/test/client.test.ts | 13 ++++ .../src/api/evaluation-stream-api.ts | 2 +- .../experiment-core/src/transport/stream.ts | 36 ++++++--- 5 files changed, 97 insertions(+), 41 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 59c7baf2..199e183c 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -7,6 +7,7 @@ import { EvaluationApi, EvaluationEngine, EvaluationFlag, + EvaluationVariant, FetchError, FlagApi, Poller, @@ -51,18 +52,19 @@ import { convertUserToContext, convertVariant, } from './util/convert'; +import { SessionAnalyticsProvider } from './util/sessionAnalyticsProvider'; +import { SessionExposureTrackingProvider } from './util/sessionExposureTrackingProvider'; import { VariantsFetchUpdater, VariantsRetryAndFallbackWrapperUpdater, VariantsStreamUpdater, VariantUpdater, } from './util/variantsUpdater'; -import { SessionAnalyticsProvider } from './util/sessionAnalyticsProvider'; -import { SessionExposureTrackingProvider } from './util/sessionExposureTrackingProvider'; // Configs which have been removed from the public API. // May be added back in the future. const minFlagPollerIntervalMillis = 60000; +const streamRetryIntervalMillis = 10 * 60 * 1000; const euServerUrl = 'https://api.lab.eu.amplitude.com'; const euFlagsServerUrl = 'https://flag.lab.eu.amplitude.com'; @@ -189,7 +191,7 @@ export class ExperimentClient implements Client { this.variantUpdater = new VariantsRetryAndFallbackWrapperUpdater( streamUpdater, this.variantUpdater, - 2 * 60 * 1000, + streamRetryIntervalMillis, ); } // Storage & Caching @@ -259,6 +261,7 @@ export class ExperimentClient implements Client { * Stop the local flag configuration poller. */ public stop() { + this.variantUpdater.stop(); if (!this.isRunning) { return; } @@ -301,7 +304,7 @@ export class ExperimentClient implements Client { user = await this.addContextOrWait(user); user = this.cleanUserPropsForFetch(user); await this.variantUpdater.start( - async (results: Record) => { + async (results: Record) => { const variants: Variants = {}; for (const key of Object.keys(results)) { variants[key] = convertEvaluationVariantToVariant(results[key]); diff --git a/packages/experiment-browser/src/util/variantsUpdater.ts b/packages/experiment-browser/src/util/variantsUpdater.ts index 272a8589..30d7b347 100644 --- a/packages/experiment-browser/src/util/variantsUpdater.ts +++ b/packages/experiment-browser/src/util/variantsUpdater.ts @@ -10,7 +10,6 @@ import { import { ExperimentConfig } from '../config'; import { FetchOptions } from '../types/client'; import { ExperimentUser } from '../types/user'; -import { Variant } from '../types/variant'; import { Backoff } from './backoff'; @@ -23,7 +22,7 @@ export interface Updater { stop(): Promise; } -type VariantUpdateCallback = (data: Record) => void; +type VariantUpdateCallback = (data: Record) => void; type VariantErrorCallback = (err: Error) => void; type VariantUpdaterParams = { user: ExperimentUser; @@ -39,8 +38,20 @@ export interface VariantUpdater extends Updater { stop(): Promise; } +function isErrorRetriable(e: Error | ErrorEvent): boolean { + if (e instanceof FetchError) { + const ferr = e as FetchError; + return ( + ferr.statusCode < 400 || ferr.statusCode >= 500 || ferr.statusCode === 429 + ); + } + + return true; +} + export class VariantsStreamUpdater implements VariantUpdater { private evaluationApi: StreamEvaluationApi; + private hasNonretriableError = false; constructor(evaluationApi: StreamEvaluationApi) { this.evaluationApi = evaluationApi; @@ -51,16 +62,36 @@ export class VariantsStreamUpdater implements VariantUpdater { onError: VariantErrorCallback, params: VariantUpdaterParams, ): Promise { - this.evaluationApi.streamVariants( - params.user, - params.options, - onUpdate, - onError, - ); + if (this.hasNonretriableError) { + throw new Error('Stream updater has non-retriable error, not starting'); + } + try { + await this.evaluationApi.streamVariants( + params.user, + params.options, + onUpdate, + (error) => { + this.handleError(error); + onError(error); + }, + ); + } catch (error) { + this.handleError(error); + throw error; + } + } + + handleError(error: Error) { + if (!isErrorRetriable(error)) { + this.hasNonretriableError = true; + console.error( + '[Experiment] Stream updater has non-retriable error: ' + error, + ); + } } async stop(): Promise { - this.evaluationApi.close(); + await this.evaluationApi.close(); } } @@ -97,7 +128,7 @@ export class VariantsFetchUpdater implements Updater { onUpdate, ); } catch (e) { - if (config.retryFetchOnFailure && this.shouldRetryFetch(e)) { + if (config.retryFetchOnFailure && isErrorRetriable(e)) { void this.startRetries(user, options, onUpdate); } throw e; @@ -126,18 +157,6 @@ export class VariantsFetchUpdater implements Updater { onUpdate(results); } - private shouldRetryFetch(e: Error): boolean { - if (e instanceof FetchError) { - const fetchErr = e as FetchError; - return ( - fetchErr.statusCode < 400 || - fetchErr.statusCode >= 500 || - fetchErr.statusCode === 429 - ); - } - return true; - } - private async startRetries( user: ExperimentUser, options: FetchOptions, @@ -173,7 +192,8 @@ export class VariantsFetchUpdater implements Updater { export class RetryAndFallbackWrapperUpdater implements Updater { private readonly mainUpdater: Updater; private readonly fallbackUpdater: Updater; - private readonly retryIntervalMillis: number; + private readonly retryIntervalMillisMin: number; + private readonly retryIntervalMillisRange: number; private retryTimer: NodeJS.Timeout | null = null; constructor( @@ -183,7 +203,9 @@ export class RetryAndFallbackWrapperUpdater implements Updater { ) { this.mainUpdater = mainUpdater; this.fallbackUpdater = fallbackUpdater; - this.retryIntervalMillis = retryIntervalMillis; + this.retryIntervalMillisMin = retryIntervalMillis * 0.8; + this.retryIntervalMillisRange = + retryIntervalMillis * 1.2 - this.retryIntervalMillisMin; } async start( @@ -191,7 +213,7 @@ export class RetryAndFallbackWrapperUpdater implements Updater { onError: VariantErrorCallback, params: VariantUpdaterParams, ): Promise { - this.stop(); + await this.stop(); try { await this.mainUpdater.start( @@ -233,7 +255,7 @@ export class RetryAndFallbackWrapperUpdater implements Updater { } catch (error) { console.error('Retry failed', error); } - }, this.retryIntervalMillis); + }, Math.ceil(this.retryIntervalMillisMin + Math.random() * this.retryIntervalMillisRange)); this.retryTimer = retryTimer; } @@ -264,6 +286,6 @@ export class VariantsRetryAndFallbackWrapperUpdater onError: VariantErrorCallback, params: VariantUpdaterParams, ): Promise { - super.start(onUpdate, onError, params); + return super.start(onUpdate, onError, params); } } diff --git a/packages/experiment-browser/test/client.test.ts b/packages/experiment-browser/test/client.test.ts index d85f3138..5bf4c0a7 100644 --- a/packages/experiment-browser/test/client.test.ts +++ b/packages/experiment-browser/test/client.test.ts @@ -1375,3 +1375,16 @@ describe('flag config polling interval config', () => { expect(client['config'].flagConfigPollingIntervalMillis).toEqual(900000); }); }); + +describe('client stream variants', () => { + test('stream variants, success', async () => { + const client = new ExperimentClient(API_KEY, { + streamVariants: true, + }); + mockClientStorage(client); + await client.fetch(testUser); + const variant = client.variant(serverKey); + expect(variant).toEqual(serverVariant); + client.stop(); + }); +}); diff --git a/packages/experiment-core/src/api/evaluation-stream-api.ts b/packages/experiment-core/src/api/evaluation-stream-api.ts index d77f967e..1fb5fa07 100644 --- a/packages/experiment-core/src/api/evaluation-stream-api.ts +++ b/packages/experiment-core/src/api/evaluation-stream-api.ts @@ -47,7 +47,7 @@ export class SdkStreamEvaluationApi implements StreamEvaluationApi { onError?: (error: Error) => void, ): Promise { if (this.stream) { - this.close(); + await this.close(); } const userJsonBase64 = Base64.encodeURL(JSON.stringify(user)); diff --git a/packages/experiment-core/src/transport/stream.ts b/packages/experiment-core/src/transport/stream.ts index 6fe04f2e..8b94ddcd 100644 --- a/packages/experiment-core/src/transport/stream.ts +++ b/packages/experiment-core/src/transport/stream.ts @@ -1,11 +1,25 @@ +import { FetchError } from 'evaluation/error'; + const KEEP_ALIVE_DATA = ' '; const KEEP_ALIVE_INTERVAL_MILLIS = 33000; // 30 seconds with a 3 seconds buffer const RECONNECTION_INTERVAL_MILLIS = 30 * 60 * 1000; export const DEFAULT_EVENT_TYPE = 'message'; -export interface ErrorEvent { - code?: number; - message?: string; +type ErrorEvent = + | { + status?: number; + message: string; + } + | Error; + +export class SSEError extends Error { + message: string; + status?: number; + constructor(message: string, status?: number) { + super(message); + this.message = message; + this.status = status; + } } export interface SSE { @@ -97,18 +111,22 @@ export class SSEStream { for (const eventType in this.onEventTypeUpdate) { this.es.addEventListener(eventType, (event) => { this.resetKeepAlive(); - event = event as MessageEvent; - if (event.data === KEEP_ALIVE_DATA) { + const msgEvent = event as MessageEvent; + if (msgEvent.data === KEEP_ALIVE_DATA) { // This is a keep-alive message, ignore it return; } - this.onEventTypeUpdate[eventType](event.data); + this.onEventTypeUpdate[eventType](msgEvent.data); }); } - this.es.addEventListener('error', (error) => { + this.es.addEventListener('error', (err) => { this.close(); - error = error as ErrorEvent; - this.onError?.(Error(`Error in stream: ${JSON.stringify(error)}`)); + const error = err as { status?: number; message: string }; + if (error.status) { + this.onError?.(new FetchError(error.status, error.message)); + } else { + this.onError?.(new Error(`Error in stream: ${JSON.stringify(error)}`)); + } }); this.resetKeepAlive(); this.setReconnectionTimeout(); From 9c4c95835667cd62cd1275ff94251f1931a31b0e Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 17 Apr 2025 11:41:08 -0700 Subject: [PATCH 03/22] fix: catch stream cb exception, add tests --- .../src/experimentClient.ts | 2 +- .../util/{variantsUpdater.ts => updaters.ts} | 8 + .../experiment-browser/test/client.test.ts | 2 +- .../test/util/updaters.test.ts | 18 ++ .../src/api/evaluation-stream-api.ts | 18 +- .../experiment-core/src/transport/stream.ts | 41 ++--- .../test/api/evaluation-stream-api.test.ts | 163 ++++++++++++++++++ .../test/transport/stream.test.ts | 147 ++++++++++++++++ packages/experiment-core/test/utils.ts | 4 + 9 files changed, 373 insertions(+), 30 deletions(-) rename packages/experiment-browser/src/util/{variantsUpdater.ts => updaters.ts} (93%) create mode 100644 packages/experiment-browser/test/util/updaters.test.ts create mode 100644 packages/experiment-core/test/api/evaluation-stream-api.test.ts create mode 100644 packages/experiment-core/test/transport/stream.test.ts create mode 100644 packages/experiment-core/test/utils.ts diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 199e183c..987f1a85 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -59,7 +59,7 @@ import { VariantsRetryAndFallbackWrapperUpdater, VariantsStreamUpdater, VariantUpdater, -} from './util/variantsUpdater'; +} from './util/updaters'; // Configs which have been removed from the public API. // May be added back in the future. diff --git a/packages/experiment-browser/src/util/variantsUpdater.ts b/packages/experiment-browser/src/util/updaters.ts similarity index 93% rename from packages/experiment-browser/src/util/variantsUpdater.ts rename to packages/experiment-browser/src/util/updaters.ts index 30d7b347..331e44c0 100644 --- a/packages/experiment-browser/src/util/variantsUpdater.ts +++ b/packages/experiment-browser/src/util/updaters.ts @@ -101,6 +101,14 @@ const fetchBackoffMinMillis = 500; const fetchBackoffMaxMillis = 10000; const fetchBackoffScalar = 1.5; +/** + * This updater fetches the variants from the server and calls the onUpdate callback with the results. + * This updater does not continuously poll the server, it only fetches the variants once. + * It will retry the fetch if it fails, with exponential backoff. + * The retry will stop if the fetch succeeds or if the max number of retries is reached. + * The retry will also stop if a non-retriable error is encountered. + * The retry will also stop if the user calls the stop method. + */ export class VariantsFetchUpdater implements Updater { private evaluationApi: EvaluationApi; retriesBackoff: Backoff; diff --git a/packages/experiment-browser/test/client.test.ts b/packages/experiment-browser/test/client.test.ts index 5bf4c0a7..2442f3f7 100644 --- a/packages/experiment-browser/test/client.test.ts +++ b/packages/experiment-browser/test/client.test.ts @@ -25,7 +25,7 @@ import { HttpClient, SimpleResponse } from '../src/types/transport'; import { randomString } from '../src/util/randomstring'; import { mockClientStorage } from './util/mock'; -import { VariantsFetchUpdater } from '../src/util/variantsUpdater'; +import { VariantsFetchUpdater } from '../src/util/updaters'; const delay = (ms: number) => new Promise((res) => setTimeout(res, ms)); diff --git a/packages/experiment-browser/test/util/updaters.test.ts b/packages/experiment-browser/test/util/updaters.test.ts new file mode 100644 index 00000000..a9e4be75 --- /dev/null +++ b/packages/experiment-browser/test/util/updaters.test.ts @@ -0,0 +1,18 @@ +describe('VariantsStreamUpdater tests', () => { + it('connect success and receive data', async () => {}); + it('connect error throws', async () => {}); + it('connect success then stream error', async () => {}); +}); +describe('VariantsFetchUpdater tests', () => { + it('fetches variant', async () => {}); + it('first fetch failed would retry but does not throw', async () => {}); + it('all fetches fails does nothing', async () => {}); +}); +describe('RetryAndFallbackWrapperUpdater tests', () => { + it('main start success, no fallback start, wrapper start success', async () => {}); + it('main start failed, fallback start success, wrapper start success', async () => {}); + it('main start failed, fallback start failed, wrapper start fail', async () => {}); + it('main start success, then failed, fallback starts, main retry success, fallback stopped', async () => {}); + it('main start success, then failed, fallback start failed, fallback retry success, main retry success, fallback stopped', async () => {}); + it('main start success, then failed, fallback start failed, main retry success, fallback stopped retrying', async () => {}); +}); diff --git a/packages/experiment-core/src/api/evaluation-stream-api.ts b/packages/experiment-core/src/api/evaluation-stream-api.ts index 1fb5fa07..21426e70 100644 --- a/packages/experiment-core/src/api/evaluation-stream-api.ts +++ b/packages/experiment-core/src/api/evaluation-stream-api.ts @@ -1,7 +1,11 @@ import { Base64 } from 'js-base64'; -import { DEFAULT_EVENT_TYPE, SSEProvider, SSEStream } from 'transport/stream'; import { EvaluationVariant } from '../evaluation/flag'; +import { + DEFAULT_EVENT_TYPE, + SSEProvider, + SSEStream, +} from '../transport/stream'; import { EvaluationApi, GetVariantsOptions } from './evaluation-api'; @@ -95,15 +99,19 @@ export class SdkStreamEvaluationApi implements StreamEvaluationApi { } }; const onDataUpdateSseCb = (data: string) => { + let variants; + try { + variants = JSON.parse(data); + } catch (error) { + onErrorSseCb(new Error('Error parsing variant data: ' + error)); + return; + } if (isConnecting) { isConnecting = false; - const variants = JSON.parse(data); clearTimeout(connectionTimeout); resolve(); - onUpdate?.(variants); - } else { - onUpdate?.(JSON.parse(data)); } + onUpdate?.(variants); }; const onSignalUpdateCb = async () => { // Signaled that there's a push. diff --git a/packages/experiment-core/src/transport/stream.ts b/packages/experiment-core/src/transport/stream.ts index 8b94ddcd..126319e3 100644 --- a/packages/experiment-core/src/transport/stream.ts +++ b/packages/experiment-core/src/transport/stream.ts @@ -1,26 +1,14 @@ -import { FetchError } from 'evaluation/error'; +import { FetchError } from '../evaluation/error'; const KEEP_ALIVE_DATA = ' '; const KEEP_ALIVE_INTERVAL_MILLIS = 33000; // 30 seconds with a 3 seconds buffer const RECONNECTION_INTERVAL_MILLIS = 30 * 60 * 1000; export const DEFAULT_EVENT_TYPE = 'message'; -type ErrorEvent = - | { - status?: number; - message: string; - } - | Error; - -export class SSEError extends Error { - message: string; +type ErrorEvent = { status?: number; - constructor(message: string, status?: number) { - super(message); - this.message = message; - this.status = status; - } -} + message: string; +}; export interface SSE { addEventListener( @@ -116,16 +104,23 @@ export class SSEStream { // This is a keep-alive message, ignore it return; } - this.onEventTypeUpdate[eventType](msgEvent.data); + try { + this.onEventTypeUpdate[eventType](msgEvent.data); + } catch { + // Don't care about errors in the callback. + } }); } this.es.addEventListener('error', (err) => { this.close(); - const error = err as { status?: number; message: string }; - if (error.status) { - this.onError?.(new FetchError(error.status, error.message)); - } else { - this.onError?.(new Error(`Error in stream: ${JSON.stringify(error)}`)); + const error = err as ErrorEvent; + const newError = error.status + ? new FetchError(error.status, error.message) + : new Error(`Error in stream: ${JSON.stringify(error)}`); + try { + this.onError?.(newError); + } catch { + // Don't care about errors in the callback. } }); this.resetKeepAlive(); @@ -141,7 +136,7 @@ export class SSEStream { this.keepAliveTimer = setTimeout(() => { this.close(); this.onError?.(Error('Keep-alive timeout')); - }, this.keepAliveInterval); + }, this.keepAliveInterval * 1.1); } } diff --git a/packages/experiment-core/test/api/evaluation-stream-api.test.ts b/packages/experiment-core/test/api/evaluation-stream-api.test.ts new file mode 100644 index 00000000..832f4c70 --- /dev/null +++ b/packages/experiment-core/test/api/evaluation-stream-api.test.ts @@ -0,0 +1,163 @@ +import { FetchError, SdkStreamEvaluationApi } from '../../src'; +import { sleep } from '../utils'; + +const VARDATA = { + 'peter-test-stream': { + key: 'treatment', + metadata: { + experimentKey: 'exp-1', + }, + value: 'treatment', + }, +}; +describe('EvaluationStreamApi tests', () => { + let registeredListeners: Record void>; + let mockStreamProvider: jest.Mock; + let api: SdkStreamEvaluationApi; + let mockOnDataUpdate: jest.Mock; + let mockOnError: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + registeredListeners = {}; + mockStreamProvider = jest.fn((url, header) => { + expect(url).toBe('https://url/sdk/stream/v1/vardata'); + expect(header['Authorization']).toBe('Api-Key apikey'); + return { + addEventListener: jest.fn( + (eventType: string, cb: (data: unknown) => void) => { + registeredListeners[eventType] = cb; + }, + ), + close: jest.fn(), + }; + }); + + api = new SdkStreamEvaluationApi( + 'apikey', + 'https://url', + mockStreamProvider, + 2000, + ); + mockOnDataUpdate = jest.fn(); + mockOnError = jest.fn(); + }); + + test('connect and receive data and parses', async () => { + const connectPromise = api.streamVariants( + {}, + undefined, + mockOnDataUpdate, + mockOnError, + ); + await sleep(1000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + registeredListeners['message']({ + data: JSON.stringify(VARDATA), + }); + await connectPromise; + expect(mockOnDataUpdate).toHaveBeenCalledTimes(1); + expect(mockOnDataUpdate).toHaveBeenCalledWith(VARDATA); + expect(mockOnError).not.toHaveBeenCalled(); + }); + test('connect http error', async () => { + const connectPromise = api.streamVariants( + {}, + undefined, + mockOnDataUpdate, + mockOnError, + ); + await sleep(1000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + registeredListeners['error']({ + status: 500, + message: 'Internal Server Error', + }); + await expect(connectPromise).rejects.toThrow( + new FetchError(500, 'Internal Server Error'), + ); + expect(mockOnDataUpdate).not.toHaveBeenCalled(); + expect(mockOnError).toHaveBeenCalledTimes(0); + }); + test('connect parsing error', async () => { + const connectPromise = api.streamVariants( + {}, + undefined, + mockOnDataUpdate, + mockOnError, + ); + await sleep(1000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + registeredListeners['message']({ + data: 'not json', + }); + await expect(connectPromise).rejects.toThrow(); + expect(mockOnDataUpdate).not.toHaveBeenCalled(); + expect(mockOnError).not.toHaveBeenCalled(); + }); + test('connect timeout', async () => { + const connectPromise = api.streamVariants( + {}, + undefined, + mockOnDataUpdate, + mockOnError, + ); + await Promise.race([ + expect(connectPromise).rejects.toThrow(), + sleep(2100).then(() => { + throw new Error('Timeout'); + }), + ]); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + expect(mockOnDataUpdate).not.toHaveBeenCalled(); + expect(mockOnError).toHaveBeenCalledTimes(0); + }); + test('stream error stops api', async () => { + const connectPromise = api.streamVariants( + {}, + undefined, + mockOnDataUpdate, + mockOnError, + ); + await sleep(1000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + registeredListeners['message']({ + data: JSON.stringify(VARDATA), + }); + await connectPromise; + expect(mockOnDataUpdate).toHaveBeenCalledTimes(1); + expect(mockOnDataUpdate).toHaveBeenCalledWith(VARDATA); + expect(mockOnError).not.toHaveBeenCalled(); + + registeredListeners['error']({ + message: 'Socket closed', + }); + expect(mockOnError).toHaveBeenCalledTimes(1); + expect(mockOnError).toHaveBeenCalledWith( + new Error('Error in stream: {"message":"Socket closed"}'), + ); + }); + test('stream error stops api', async () => { + const connectPromise = api.streamVariants( + {}, + undefined, + mockOnDataUpdate, + mockOnError, + ); + await sleep(1000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + registeredListeners['message']({ + data: JSON.stringify(VARDATA), + }); + await connectPromise; + expect(mockOnDataUpdate).toHaveBeenCalledTimes(1); + expect(mockOnDataUpdate).toHaveBeenCalledWith(VARDATA); + expect(mockOnError).not.toHaveBeenCalled(); + + registeredListeners['message']({ + data: 'not json', + }); + expect(mockOnDataUpdate).toHaveBeenCalledTimes(1); + expect(mockOnError).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/experiment-core/test/transport/stream.test.ts b/packages/experiment-core/test/transport/stream.test.ts new file mode 100644 index 00000000..caa824a8 --- /dev/null +++ b/packages/experiment-core/test/transport/stream.test.ts @@ -0,0 +1,147 @@ +import { SSEStream } from '../../src/transport/stream'; +import { sleep } from '../utils'; + +describe('SSEStream', () => { + let addEventListener: jest.Mock; + let close: jest.Mock; + let mockStreamProvider: jest.Mock; + + beforeEach(() => { + jest.clearAllMocks(); + addEventListener = jest.fn(); + close = jest.fn(); + mockStreamProvider = jest.fn(() => ({ + addEventListener, + close, + })); + }); + + test('should connect and receive data', async () => { + const stream = new SSEStream( + mockStreamProvider, + 'http://localhost:7999', + { + header1: 'value1', + }, + 4000, + 7000, + ); + const mockOnDataUpdate = jest.fn(); + const mockOnError = jest.fn(); + + // Test new connection makes a call to the stream provider. + stream.connect(mockOnDataUpdate, mockOnError); + expect(mockStreamProvider).toHaveBeenCalledWith('http://localhost:7999', { + header1: 'value1', + }); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + + // Test that the event listener is set up correctly. + addEventListener.mock.calls[0][1]({ + type: 'message', + data: 'apple', + }); + expect(addEventListener.mock.calls[0][0]).toBe('message'); + expect(mockOnDataUpdate).toHaveBeenCalledWith('apple'); + expect(addEventListener.mock.calls[1][0]).toBe('error'); + expect(mockOnError).not.toHaveBeenCalled(); + + // Test keepalive avoids new connection. + await sleep(4000); + addEventListener.mock.calls[0][1]({ + type: 'message', + data: ' ', + }); + await sleep(1000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + + // Test keepalive data is not passed to the callback. + expect(mockOnDataUpdate).not.toHaveBeenCalledWith(' '); + expect(mockOnError).not.toHaveBeenCalled(); + + // Test reconnection reconnects and receives data correctly. + await sleep(Math.ceil(4000)); + expect(mockStreamProvider).toHaveBeenCalledTimes(2); + addEventListener.mock.calls[0][1]({ + type: 'message', + data: 'banana', + }); + expect(addEventListener.mock.calls[2][0]).toBe('message'); + expect(mockOnDataUpdate).toHaveBeenCalledWith('banana'); + expect(addEventListener.mock.calls[3][0]).toBe('error'); + expect(mockOnError).not.toHaveBeenCalled(); + }, 12000); + + test('able to subscribe for multiple event types', async () => { + const stream = new SSEStream(mockStreamProvider, 'url', {}); + const mockOnData1Update = jest.fn(); + const mockOnData2Update = jest.fn(); + const mockOnError = jest.fn(); + + // Test new connection makes a call to the stream provider. + stream.connect( + { channel1: mockOnData1Update, channel2: mockOnData2Update }, + mockOnError, + ); + expect(mockStreamProvider).toHaveBeenCalledWith('url', {}); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + + // Test that the event listener is set up correctly. + const registeredListeners = addEventListener.mock.calls.reduce( + (acc, curr) => { + acc[curr[0]] = curr[1]; + return acc; + }, + {}, + ); + expect(registeredListeners['channel1']).toBeDefined(); + expect(registeredListeners['channel2']).toBeDefined(); + expect(registeredListeners['message']).toBeDefined(); // default is always added. + expect(registeredListeners['error']).toBeDefined(); + + // Send data for each channel. + registeredListeners['channel1']({ + type: 'channel1', + data: 'apple', + }); + expect(mockOnData1Update).toBeCalledTimes(1); + expect(mockOnData2Update).not.toHaveBeenCalled(); + expect(mockOnError).not.toHaveBeenCalled(); + expect(mockOnData1Update).toHaveBeenCalledWith('apple'); + registeredListeners['channel2']({ + type: 'channel2', + data: 'banana', + }); + expect(mockOnData1Update).toBeCalledTimes(1); + expect(mockOnData2Update).toBeCalledTimes(1); + expect(mockOnError).not.toHaveBeenCalled(); + expect(mockOnData2Update).toHaveBeenCalledWith('banana'); + }); + + test('keepalive failure causes error', async () => { + const stream = new SSEStream(mockStreamProvider, 'url', {}, 3000); + const mockOnDataUpdate = jest.fn(); + const mockOnError = jest.fn(); + stream.connect(mockOnDataUpdate, mockOnError); + await sleep(4000); + expect(mockStreamProvider).toHaveBeenCalledTimes(1); + expect(mockOnDataUpdate).not.toHaveBeenCalled(); + expect(mockOnError).toHaveBeenCalledTimes(1); + expect(mockOnError).toHaveBeenCalledWith(new Error('Keep-alive timeout')); + expect(close).toHaveBeenCalled(); + }); + + test('error event causes error callback', async () => { + const stream = new SSEStream(mockStreamProvider, 'url', {}); + const mockOnDataUpdate = jest.fn(); + const mockOnError = jest.fn(); + stream.connect(mockOnDataUpdate, mockOnError); + addEventListener.mock.calls[1][1]({ + message: 'error', + }); + expect(mockOnDataUpdate).not.toHaveBeenCalled(); + expect(mockOnError).toHaveBeenCalledWith( + new Error('Error in stream: {"message":"error"}'), + ); + }); +}); diff --git a/packages/experiment-core/test/utils.ts b/packages/experiment-core/test/utils.ts new file mode 100644 index 00000000..3ac327ce --- /dev/null +++ b/packages/experiment-core/test/utils.ts @@ -0,0 +1,4 @@ +export const sleep = async (millis: number) => + new Promise((r) => { + setTimeout(r, millis); + }); From 8142cf5d09359d6a5242292b9cb429c7531e253d Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 17 Apr 2025 16:55:55 -0700 Subject: [PATCH 04/22] fix: retry wrapper updater retry fallback start, add test --- .../experiment-browser/src/util/updaters.ts | 108 +++-- packages/experiment-browser/test/util/misc.ts | 6 + .../test/util/updaters.test.ts | 452 +++++++++++++++++- 3 files changed, 521 insertions(+), 45 deletions(-) diff --git a/packages/experiment-browser/src/util/updaters.ts b/packages/experiment-browser/src/util/updaters.ts index 331e44c0..28c2374c 100644 --- a/packages/experiment-browser/src/util/updaters.ts +++ b/packages/experiment-browser/src/util/updaters.ts @@ -65,23 +65,25 @@ export class VariantsStreamUpdater implements VariantUpdater { if (this.hasNonretriableError) { throw new Error('Stream updater has non-retriable error, not starting'); } + await this.stop(); try { await this.evaluationApi.streamVariants( params.user, params.options, onUpdate, - (error) => { - this.handleError(error); + async (error) => { + await this.handleError(error); onError(error); }, ); } catch (error) { - this.handleError(error); + await this.handleError(error); throw error; } } - handleError(error: Error) { + async handleError(error: Error): Promise { + await this.stop(); if (!isErrorRetriable(error)) { this.hasNonretriableError = true; console.error( @@ -112,6 +114,7 @@ const fetchBackoffScalar = 1.5; export class VariantsFetchUpdater implements Updater { private evaluationApi: EvaluationApi; retriesBackoff: Backoff; + constructor(evaluationApi: EvaluationApi) { this.evaluationApi = evaluationApi; } @@ -170,7 +173,6 @@ export class VariantsFetchUpdater implements Updater { options: FetchOptions, onUpdate: (data: Record) => void, ): Promise { - // this.debug('[Experiment] Retry fetch'); this.retriesBackoff = new Backoff( fetchBackoffAttempts, fetchBackoffMinMillis, @@ -195,14 +197,15 @@ export class VariantsFetchUpdater implements Updater { /** * This class retries the main updater and, if it fails, falls back to the fallback updater. * The main updater will keep retrying every set interval and, if succeeded, the fallback updater will be stopped. + * If it has falled back to fallback updater, if the fallback updated failed to start, it will retry starting the fallback updater. */ - export class RetryAndFallbackWrapperUpdater implements Updater { private readonly mainUpdater: Updater; private readonly fallbackUpdater: Updater; private readonly retryIntervalMillisMin: number; private readonly retryIntervalMillisRange: number; - private retryTimer: NodeJS.Timeout | null = null; + private mainRetryTimer: NodeJS.Timeout | null = null; // To retry main updater after initial start. + private fallbackRetryTimer: NodeJS.Timeout | null = null; // To make sure fallback start is retried if failed to start when main updater failed. constructor( mainUpdater: Updater, @@ -216,61 +219,100 @@ export class RetryAndFallbackWrapperUpdater implements Updater { retryIntervalMillis * 1.2 - this.retryIntervalMillisMin; } + /** + * If main start succeeded, return. + * If main start failed, start fallback updater. + * If fallback start failed, throw exception. + */ async start( - onUpdate: VariantUpdateCallback, - onError: VariantErrorCallback, - params: VariantUpdaterParams, + onUpdate: (data: unknown) => void, + onError: (err: Error) => void, + params: Record, ): Promise { await this.stop(); try { await this.mainUpdater.start( onUpdate, - (err) => { - this.fallbackUpdater.start(onUpdate, onError, params); - this.startRetryTimer(onUpdate, onError, params); + async (err) => { + this.fallbackUpdater + .start(onUpdate, onError, params) + .catch((error) => { + this.startFallbackRetryTimer(onUpdate, onError, params); + }); + this.startMainRetryTimer(onUpdate, onError, params); }, params, ); } catch (error) { await this.fallbackUpdater.start(onUpdate, onError, params); - this.startRetryTimer(onUpdate, onError, params); + this.startMainRetryTimer(onUpdate, onError, params); } } - async startRetryTimer(onUpdate, onError, params) { - if (this.retryTimer) { - clearInterval(this.retryTimer); - this.retryTimer = null; + startMainRetryTimer(onUpdate, onError, params) { + if (this.mainRetryTimer) { + clearTimeout(this.mainRetryTimer); + this.mainRetryTimer = null; } - const retryTimer = setInterval(async () => { + const retryTimer = setTimeout(async () => { try { await this.mainUpdater.start( onUpdate, (err) => { - this.fallbackUpdater.start(onUpdate, onError, params); - this.startRetryTimer(onUpdate, onError, params); + this.fallbackUpdater + .start(onUpdate, onError, params) + .catch((error) => { + this.startFallbackRetryTimer(onUpdate, onError, params); + }); + this.startMainRetryTimer(onUpdate, onError, params); }, params, ); - this.fallbackUpdater.stop(); - clearInterval(retryTimer); - if (this.retryTimer) { - clearInterval(this.retryTimer); - this.retryTimer = null; - } - } catch (error) { - console.error('Retry failed', error); + } catch { + this.startMainRetryTimer(onUpdate, onError, params); + return; + } + if (this.fallbackRetryTimer) { + clearTimeout(this.fallbackRetryTimer); + this.fallbackRetryTimer = null; } + this.fallbackUpdater.stop(); }, Math.ceil(this.retryIntervalMillisMin + Math.random() * this.retryIntervalMillisRange)); - this.retryTimer = retryTimer; + this.mainRetryTimer = retryTimer; + } + + startFallbackRetryTimer(onUpdate, onError, params) { + if (this.fallbackRetryTimer) { + clearTimeout(this.fallbackRetryTimer); + this.fallbackRetryTimer = null; + } + const retryTimer = setTimeout(async () => { + try { + await this.fallbackUpdater.start(onUpdate, onError, params); + } catch { + this.startFallbackRetryTimer(onUpdate, onError, params); + } + }, Math.ceil(this.retryIntervalMillisMin + Math.random() * this.retryIntervalMillisRange)); + this.fallbackRetryTimer = retryTimer; } async stop(): Promise { - if (this.retryTimer) { - clearInterval(this.retryTimer); - this.retryTimer = null; + /* + * No locks needed for await and asyncs. + * If stop is called, the intervals are cancelled. Callbacks are not called. + * If the callback has already started, the updater.start in callback is scheduled before the updater.stop in this stop() func. + * So either no updater.start() is performed, or the updater.start() is scheduled before the updater.stop(). + */ + // Cancelling timers must be done before stopping the updaters. + if (this.mainRetryTimer) { + clearTimeout(this.mainRetryTimer); + this.mainRetryTimer = null; + } + if (this.fallbackRetryTimer) { + clearTimeout(this.fallbackRetryTimer); + this.fallbackRetryTimer = null; } await this.mainUpdater.stop(); await this.fallbackUpdater.stop(); diff --git a/packages/experiment-browser/test/util/misc.ts b/packages/experiment-browser/test/util/misc.ts index 0076dd2b..25166c31 100644 --- a/packages/experiment-browser/test/util/misc.ts +++ b/packages/experiment-browser/test/util/misc.ts @@ -6,3 +6,9 @@ export const clearAllCookies = () => { document.cookie = `${cookieName}=; expires=Thu, 01 Jan 1970 00:00:00 GMT; path=/`; } }; + +export const sleep = async (millis: number): Promise => { + return new Promise((resolve) => { + setTimeout(resolve, millis); + }); +}; diff --git a/packages/experiment-browser/test/util/updaters.test.ts b/packages/experiment-browser/test/util/updaters.test.ts index a9e4be75..2abe0a9f 100644 --- a/packages/experiment-browser/test/util/updaters.test.ts +++ b/packages/experiment-browser/test/util/updaters.test.ts @@ -1,18 +1,446 @@ +import { + SdkEvaluationApi, + SdkStreamEvaluationApi, +} from '@amplitude/experiment-core'; +import { FetchError } from '@amplitude/experiment-core'; + +import { + RetryAndFallbackWrapperUpdater, + Updater, + VariantsFetchUpdater, + VariantsStreamUpdater, +} from '../..//src/util/updaters'; + +import { sleep } from './misc'; + describe('VariantsStreamUpdater tests', () => { - it('connect success and receive data', async () => {}); - it('connect error throws', async () => {}); - it('connect success then stream error', async () => {}); + test('connect success and receive data', async () => { + let registeredOnUpdate; + const mockStreamApi = { + streamVariants: jest.fn( + async (_: unknown, __: unknown, onUpdate: (data) => void) => { + registeredOnUpdate = onUpdate; + }, + ), + close: jest.fn(), + } as unknown as SdkStreamEvaluationApi; + const updater = new VariantsStreamUpdater(mockStreamApi); + const onUpdate = jest.fn(); + await updater.start(onUpdate, () => {}, { + user: {}, + config: {}, + options: {}, + }); + + expect(onUpdate).toHaveBeenCalledTimes(0); + await registeredOnUpdate({ + 'test-flag': {}, + }); + expect(onUpdate).toHaveBeenCalledTimes(1); + await registeredOnUpdate({ + 'test-flag': {}, + }); + expect(onUpdate).toHaveBeenCalledTimes(2); + }); + + test('connect error throws', async () => { + const mockStreamApi = { + streamVariants: jest.fn( + async (_: unknown, __: unknown, onUpdate: (data) => void) => { + throw new FetchError(413, 'Payload too large'); + }, + ), + close: jest.fn(), + } as unknown as SdkStreamEvaluationApi; + const updater = new VariantsStreamUpdater(mockStreamApi); + const onUpdate = jest.fn(); + await expect( + updater.start(onUpdate, () => {}, { + user: {}, + config: {}, + options: {}, + }), + ).rejects.toThrow('Payload too large'); + expect(onUpdate).toHaveBeenCalledTimes(0); + expect(updater['hasNonretriableError']).toBe(true); + }); + + test('connect success then stream error', async () => { + let registeredOnUpdate; + let registeredOnError; + const mockStreamApi = { + streamVariants: jest.fn( + async ( + _: unknown, + __: unknown, + onUpdate: (data) => void, + onError: (error) => void, + ) => { + registeredOnUpdate = onUpdate; + registeredOnError = onError; + }, + ), + close: jest.fn(), + } as unknown as SdkStreamEvaluationApi; + const updater = new VariantsStreamUpdater(mockStreamApi); + const onUpdate = jest.fn(); + const onError = jest.fn(); + await updater.start(onUpdate, onError, { + user: {}, + config: {}, + options: {}, + }); + + registeredOnUpdate({ 'test-flag': {} }); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(mockStreamApi.close).toHaveBeenCalledTimes(1); // Close called once on start. + await registeredOnError(new Error('Stream error')); + expect(onError).toHaveBeenCalledTimes(1); + expect(mockStreamApi.close).toHaveBeenCalledTimes(2); // Close called again on error. + expect(updater['hasNonretriableError']).toBe(false); + }); }); + describe('VariantsFetchUpdater tests', () => { - it('fetches variant', async () => {}); - it('first fetch failed would retry but does not throw', async () => {}); - it('all fetches fails does nothing', async () => {}); + test('fetches variant', async () => { + const mockEvalApi = { + getVariants: jest.fn(async () => { + return { + 'test-flag': {}, + }; + }), + } as unknown as SdkEvaluationApi; + const updater = new VariantsFetchUpdater(mockEvalApi); + const onUpdate = jest.fn(); + const onError = jest.fn(); + await updater.start(onUpdate, onError, { + user: {}, + config: {}, + options: {}, + }); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith({ + 'test-flag': {}, + }); + await updater.start(onUpdate, onError, { + user: {}, + config: {}, + options: {}, + }); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(2); + expect(onUpdate).toHaveBeenCalledTimes(2); + expect(onUpdate).toHaveBeenCalledWith({ + 'test-flag': {}, + }); + expect(onError).toHaveBeenCalledTimes(0); + }); + + test('first fetch failed retriable would retry but does not throw', async () => { + const mockEvalApi = { + getVariants: jest + .fn() + .mockRejectedValueOnce(new FetchError(500, 'Internal Server Error')) + .mockResolvedValueOnce({ + 'test-flag': {}, + }), + } as unknown as SdkEvaluationApi; + const updater = new VariantsFetchUpdater(mockEvalApi); + const onUpdate = jest.fn(); + const onError = jest.fn(); + await updater.start(onUpdate, onError, { + user: {}, + config: { retryFetchOnFailure: true }, + options: {}, + }); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledTimes(0); + await sleep(750); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(2); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith({ + 'test-flag': {}, + }); + expect(onError).toHaveBeenCalledTimes(0); + }); + + test('first fetch failed nonretriable would not retry and does not throw', async () => { + const mockEvalApi = { + getVariants: jest + .fn() + .mockRejectedValue(new FetchError(413, 'Payload too large')), + } as unknown as SdkEvaluationApi; + const updater = new VariantsFetchUpdater(mockEvalApi); + const onUpdate = jest.fn(); + const onError = jest.fn(); + await updater.start(onUpdate, onError, { + user: {}, + config: { retryFetchOnFailure: true }, + options: {}, + }); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledTimes(0); + await sleep(2000); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledTimes(0); + expect(onError).toHaveBeenCalledTimes(0); + }); + + test('all fetches fails does nothing', async () => { + const mockEvalApi = { + getVariants: jest + .fn() + .mockRejectedValue(new FetchError(500, 'Internal Server Error')), + } as unknown as SdkEvaluationApi; + const updater = new VariantsFetchUpdater(mockEvalApi); + const onUpdate = jest.fn(); + const onError = jest.fn(); + await updater.start(onUpdate, onError, { + user: {}, + config: { retryFetchOnFailure: true }, + options: {}, + }); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledTimes(0); + await sleep(20000); + expect(mockEvalApi.getVariants).toHaveBeenCalledTimes(8); + expect(onUpdate).toHaveBeenCalledTimes(0); + expect(onError).toHaveBeenCalledTimes(0); + }, 30000); }); + describe('RetryAndFallbackWrapperUpdater tests', () => { - it('main start success, no fallback start, wrapper start success', async () => {}); - it('main start failed, fallback start success, wrapper start success', async () => {}); - it('main start failed, fallback start failed, wrapper start fail', async () => {}); - it('main start success, then failed, fallback starts, main retry success, fallback stopped', async () => {}); - it('main start success, then failed, fallback start failed, fallback retry success, main retry success, fallback stopped', async () => {}); - it('main start success, then failed, fallback start failed, main retry success, fallback stopped retrying', async () => {}); + test('main start success, no fallback start, wrapper start success', async () => { + const mainUpdater = { + start: jest.fn(), + stop: jest.fn(), + }; + const fallbackUpdater = { + start: jest.fn(), + stop: jest.fn(), + }; + const wrapperUpdater = new RetryAndFallbackWrapperUpdater( + mainUpdater as Updater, + fallbackUpdater as Updater, + 2000, + ); + const onUpdate = jest.fn(); + await wrapperUpdater.start(onUpdate, jest.fn(), {}); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(0); + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('asdf'); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('asdf'); + // Reset stop function mocks to reset called times. + mainUpdater.stop = jest.fn(); + fallbackUpdater.stop = jest.fn(); + await wrapperUpdater.stop(); + expect(mainUpdater.stop).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.stop).toHaveBeenCalledTimes(1); + }); + + test('main start failed, fallback start success, wrapper start success', async () => { + const mainUpdater = { + start: jest.fn().mockRejectedValue(new Error('Main updater error')), + stop: jest.fn(), + }; + const fallbackUpdater = { + start: jest.fn(), + stop: jest.fn(), + }; + const wrapperUpdater = new RetryAndFallbackWrapperUpdater( + mainUpdater as Updater, + fallbackUpdater as Updater, + 2000, + ); + const onUpdate = jest.fn(); + await wrapperUpdater.start(onUpdate, jest.fn(), {}); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Verify data flows. + fallbackUpdater.start.mock.lastCall[0]('asdf'); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('asdf'); + // Reset stop function mocks to reset called times. + mainUpdater.stop = jest.fn(); + fallbackUpdater.stop = jest.fn(); + await wrapperUpdater.stop(); + expect(mainUpdater.stop).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.stop).toHaveBeenCalledTimes(1); + }); + + test('main start failed, fallback start failed, wrapper start fail', async () => { + const mainUpdater = { + start: jest.fn().mockRejectedValue(new Error('Main updater error')), + stop: jest.fn(), + }; + const fallbackUpdater = { + start: jest.fn().mockRejectedValue(new Error('Fallback updater error')), + stop: jest.fn(), + }; + const wrapperUpdater = new RetryAndFallbackWrapperUpdater( + mainUpdater, + fallbackUpdater, + 2000, + ); + await expect( + wrapperUpdater.start(jest.fn(), jest.fn(), {}), + ).rejects.toThrow('Fallback updater error'); + }); + + test('main start success, then failed, fallback starts, main retry success, fallback stopped', async () => { + const mainUpdater = { + start: jest.fn(), + stop: jest.fn(), + }; + const fallbackUpdater = { + start: jest.fn().mockResolvedValue(undefined), + stop: jest.fn(), + }; + const wrapperUpdater = new RetryAndFallbackWrapperUpdater( + mainUpdater, + fallbackUpdater, + 4000, + ); + const onUpdate = jest.fn(); + await wrapperUpdater.start(onUpdate, jest.fn(), {}); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(0); + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('asdf'); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('asdf'); + // Signal main updater to fail. + mainUpdater.start.mock.lastCall[1](new Error('Main updater error')); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Verify data flows. + fallbackUpdater.start.mock.lastCall[0]('fallback data'); + expect(onUpdate).toHaveBeenCalledTimes(2); + expect(onUpdate).toHaveBeenCalledWith('fallback data'); + // Wait for retry. + await sleep(5000); + expect(mainUpdater.start).toHaveBeenCalledTimes(2); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('main data'); + expect(onUpdate).toHaveBeenCalledTimes(3); + expect(onUpdate).toHaveBeenCalledWith('main data'); + // Reset stop function mocks to reset called times. + mainUpdater.stop = jest.fn(); + fallbackUpdater.stop = jest.fn(); + await wrapperUpdater.stop(); + expect(mainUpdater.stop).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.stop).toHaveBeenCalledTimes(1); + }); + + test('main start success, then failed, fallback start failed, fallback retry success, main retry success, fallback stopped', async () => { + const mainUpdater = { + start: jest + .fn() + .mockResolvedValueOnce(undefined) + .mockRejectedValueOnce(undefined) + .mockResolvedValueOnce(undefined), + stop: jest.fn(), + }; + const fallbackUpdater = { + start: jest + .fn() + .mockRejectedValueOnce(new Error('Fallback start error')) + .mockResolvedValueOnce(undefined), + stop: jest.fn(), + }; + const wrapperUpdater = new RetryAndFallbackWrapperUpdater( + mainUpdater, + fallbackUpdater, + 4000, + ); + const onUpdate = jest.fn(); + await wrapperUpdater.start(onUpdate, jest.fn(), {}); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(0); + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('asdf'); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('asdf'); + // Signal main updater to fail. + mainUpdater.start.mock.lastCall[1](new Error('Main updater error')); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Wait for main and fallback retry. + await sleep(5000); + expect(mainUpdater.start).toHaveBeenCalledTimes(2); // Main still failed to start. + expect(fallbackUpdater.start).toHaveBeenCalledTimes(2); // Fallback would start success. + // Verify data flows through fallback. + fallbackUpdater.start.mock.lastCall[0]('fallback data'); + expect(onUpdate).toHaveBeenCalledTimes(2); + expect(onUpdate).toHaveBeenCalledWith('fallback data'); + // Wait for main retry. + fallbackUpdater.stop = jest.fn(); // Reset fallback stop counter. + await sleep(5000); + expect(mainUpdater.start).toHaveBeenCalledTimes(3); // Main success. + expect(fallbackUpdater.start).toHaveBeenCalledTimes(2); // No more fallback retry. + expect(fallbackUpdater.stop).toHaveBeenCalledTimes(1); // Verify fallback stopped. + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('main data'); + expect(onUpdate).toHaveBeenCalledTimes(3); + expect(onUpdate).toHaveBeenCalledWith('main data'); + // Reset stop function mocks to reset called times. + mainUpdater.stop = jest.fn(); + fallbackUpdater.stop = jest.fn(); + await wrapperUpdater.stop(); + expect(mainUpdater.stop).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.stop).toHaveBeenCalledTimes(1); + }, 15000); + + test('main start success, then failed, fallback start failed, main retry success, fallback stopped retrying', async () => { + const mainUpdater = { + start: jest.fn(), + stop: jest.fn(), + }; + const fallbackUpdater = { + start: jest.fn(async () => { + await sleep(2500); + throw new Error('Fallback start error'); + }), + stop: jest.fn(), + }; + const wrapperUpdater = new RetryAndFallbackWrapperUpdater( + mainUpdater, + fallbackUpdater, + 4000, + ); + const onUpdate = jest.fn(); + await wrapperUpdater.start(onUpdate, jest.fn(), {}); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(0); + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('asdf'); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate).toHaveBeenCalledWith('asdf'); + // Signal main updater to fail. + mainUpdater.start.mock.lastCall[1](new Error('Main updater error')); + expect(mainUpdater.start).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Ensure fallback updater failed and enters retry. + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Wait for main retry. + await sleep(5000); + expect(mainUpdater.start).toHaveBeenCalledTimes(2); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Verify data flows. + mainUpdater.start.mock.lastCall[0]('main data'); + expect(onUpdate).toHaveBeenCalledTimes(2); + expect(onUpdate).toHaveBeenCalledWith('main data'); + // Make sure fallback stopped retrying. + await sleep(5000); + expect(fallbackUpdater.start).toHaveBeenCalledTimes(1); + // Reset stop function mocks to reset called times. + mainUpdater.stop = jest.fn(); + fallbackUpdater.stop = jest.fn(); + await wrapperUpdater.stop(); + expect(mainUpdater.stop).toHaveBeenCalledTimes(1); + expect(fallbackUpdater.stop).toHaveBeenCalledTimes(1); + }, 15000); }); From 2505cd07784f31b9e7ee575f43b085356851d002 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Sun, 20 Apr 2025 03:13:33 -0700 Subject: [PATCH 05/22] test: simplify test, add comments --- .../src/experimentClient.ts | 4 +- .../experiment-browser/src/util/updaters.ts | 14 +++++++ .../experiment-browser/test/client.test.ts | 2 +- .../test/util/updaters.test.ts | 41 ++++++++----------- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 987f1a85..f33991b2 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -64,6 +64,7 @@ import { // Configs which have been removed from the public API. // May be added back in the future. const minFlagPollerIntervalMillis = 60000; +const streamConnectionTimeoutMillis = 3000; const streamRetryIntervalMillis = 10 * 60 * 1000; const euServerUrl = 'https://api.lab.eu.amplitude.com'; @@ -183,8 +184,7 @@ export class ExperimentClient implements Client { this.apiKey, this.config.streamVariantsServerUrl, defaultSseProvider, - 3000, - // config.streamVariantsConnTimeoutMillis, + streamConnectionTimeoutMillis, evaluationApi, ); const streamUpdater = new VariantsStreamUpdater(streamEvaluationApi); diff --git a/packages/experiment-browser/src/util/updaters.ts b/packages/experiment-browser/src/util/updaters.ts index 28c2374c..6a5ba605 100644 --- a/packages/experiment-browser/src/util/updaters.ts +++ b/packages/experiment-browser/src/util/updaters.ts @@ -49,6 +49,11 @@ function isErrorRetriable(e: Error | ErrorEvent): boolean { return true; } +/** + * This updater streams the variants from the server and calls the onUpdate callback with the results. + * This updater does not retry the stream if it fails, it will call the onError callback with the error. + * If it encounters a non-retriable error, all future starts are errored. + */ export class VariantsStreamUpdater implements VariantUpdater { private evaluationApi: StreamEvaluationApi; private hasNonretriableError = false; @@ -119,6 +124,15 @@ export class VariantsFetchUpdater implements Updater { this.evaluationApi = evaluationApi; } + /** + * Perform a fetch to the server and call the onUpdate callback with the results. + * If the fetch fails, it will retry the fetch with exponential backoff. + * Always return success, even if the fetch fails. + * @param onUpdate This callback is called with the results of the fetch. + * @param onError This callback is ignored. + * @param params The params object contains the user, config, and options. + * @returns A promise that resolves when the first fetch finishes. Always resolves, even if the fetch fails. + */ async start( onUpdate: VariantUpdateCallback, onError: VariantErrorCallback, diff --git a/packages/experiment-browser/test/client.test.ts b/packages/experiment-browser/test/client.test.ts index 2442f3f7..e65265f8 100644 --- a/packages/experiment-browser/test/client.test.ts +++ b/packages/experiment-browser/test/client.test.ts @@ -23,9 +23,9 @@ import { } from '../src'; import { HttpClient, SimpleResponse } from '../src/types/transport'; import { randomString } from '../src/util/randomstring'; +import { VariantsFetchUpdater } from '../src/util/updaters'; import { mockClientStorage } from './util/mock'; -import { VariantsFetchUpdater } from '../src/util/updaters'; const delay = (ms: number) => new Promise((res) => setTimeout(res, ms)); diff --git a/packages/experiment-browser/test/util/updaters.test.ts b/packages/experiment-browser/test/util/updaters.test.ts index 2abe0a9f..c53f5aca 100644 --- a/packages/experiment-browser/test/util/updaters.test.ts +++ b/packages/experiment-browser/test/util/updaters.test.ts @@ -15,29 +15,24 @@ import { sleep } from './misc'; describe('VariantsStreamUpdater tests', () => { test('connect success and receive data', async () => { - let registeredOnUpdate; const mockStreamApi = { - streamVariants: jest.fn( - async (_: unknown, __: unknown, onUpdate: (data) => void) => { - registeredOnUpdate = onUpdate; - }, - ), + streamVariants: jest.fn(), close: jest.fn(), - } as unknown as SdkStreamEvaluationApi; + }; const updater = new VariantsStreamUpdater(mockStreamApi); const onUpdate = jest.fn(); - await updater.start(onUpdate, () => {}, { + await updater.start(onUpdate, jest.fn(), { user: {}, config: {}, options: {}, }); expect(onUpdate).toHaveBeenCalledTimes(0); - await registeredOnUpdate({ + await mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {}, }); expect(onUpdate).toHaveBeenCalledTimes(1); - await registeredOnUpdate({ + await mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {}, }); expect(onUpdate).toHaveBeenCalledTimes(2); @@ -45,17 +40,15 @@ describe('VariantsStreamUpdater tests', () => { test('connect error throws', async () => { const mockStreamApi = { - streamVariants: jest.fn( - async (_: unknown, __: unknown, onUpdate: (data) => void) => { - throw new FetchError(413, 'Payload too large'); - }, - ), + streamVariants: jest + .fn() + .mockRejectedValue(new FetchError(413, 'Payload too large')), close: jest.fn(), } as unknown as SdkStreamEvaluationApi; const updater = new VariantsStreamUpdater(mockStreamApi); const onUpdate = jest.fn(); await expect( - updater.start(onUpdate, () => {}, { + updater.start(onUpdate, jest.fn(), { user: {}, config: {}, options: {}, @@ -81,7 +74,7 @@ describe('VariantsStreamUpdater tests', () => { }, ), close: jest.fn(), - } as unknown as SdkStreamEvaluationApi; + }; const updater = new VariantsStreamUpdater(mockStreamApi); const onUpdate = jest.fn(); const onError = jest.fn(); @@ -91,10 +84,12 @@ describe('VariantsStreamUpdater tests', () => { options: {}, }); - registeredOnUpdate({ 'test-flag': {} }); + mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {} }); expect(onUpdate).toHaveBeenCalledTimes(1); expect(mockStreamApi.close).toHaveBeenCalledTimes(1); // Close called once on start. - await registeredOnError(new Error('Stream error')); + await mockStreamApi.streamVariants.mock.lastCall[3]( + new Error('Stream error'), + ); expect(onError).toHaveBeenCalledTimes(1); expect(mockStreamApi.close).toHaveBeenCalledTimes(2); // Close called again on error. expect(updater['hasNonretriableError']).toBe(false); @@ -109,7 +104,7 @@ describe('VariantsFetchUpdater tests', () => { 'test-flag': {}, }; }), - } as unknown as SdkEvaluationApi; + }; const updater = new VariantsFetchUpdater(mockEvalApi); const onUpdate = jest.fn(); const onError = jest.fn(); @@ -144,7 +139,7 @@ describe('VariantsFetchUpdater tests', () => { .mockResolvedValueOnce({ 'test-flag': {}, }), - } as unknown as SdkEvaluationApi; + }; const updater = new VariantsFetchUpdater(mockEvalApi); const onUpdate = jest.fn(); const onError = jest.fn(); @@ -169,7 +164,7 @@ describe('VariantsFetchUpdater tests', () => { getVariants: jest .fn() .mockRejectedValue(new FetchError(413, 'Payload too large')), - } as unknown as SdkEvaluationApi; + }; const updater = new VariantsFetchUpdater(mockEvalApi); const onUpdate = jest.fn(); const onError = jest.fn(); @@ -191,7 +186,7 @@ describe('VariantsFetchUpdater tests', () => { getVariants: jest .fn() .mockRejectedValue(new FetchError(500, 'Internal Server Error')), - } as unknown as SdkEvaluationApi; + }; const updater = new VariantsFetchUpdater(mockEvalApi); const onUpdate = jest.fn(); const onError = jest.fn(); From 35d3fc42df33d0e9106abe2f6a07d1606a7621a3 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 23 Apr 2025 15:45:18 -0700 Subject: [PATCH 06/22] feat: change provider params, add stream test --- .gitignore | 3 + .../src/transport/stream.ts | 8 +- .../experiment-browser/test/stream.test.ts | 144 ++++++++++++++++++ .../src/api/evaluation-stream-api.ts | 56 +++---- packages/experiment-core/src/index.ts | 7 +- .../experiment-core/src/transport/stream.ts | 7 +- .../test/api/evaluation-stream-api.test.ts | 4 +- .../test/transport/stream.test.ts | 6 +- yarn.lock | 111 +++++++++++++- 9 files changed, 306 insertions(+), 40 deletions(-) create mode 100644 packages/experiment-browser/test/stream.test.ts diff --git a/.gitignore b/.gitignore index 5757ec87..85f90cec 100644 --- a/.gitignore +++ b/.gitignore @@ -27,3 +27,6 @@ dist/ # Example Experiment tag script packages/experiment-tag/example/ + +# Environment variables +.env* \ No newline at end of file diff --git a/packages/experiment-browser/src/transport/stream.ts b/packages/experiment-browser/src/transport/stream.ts index c368ec78..f2efcebc 100644 --- a/packages/experiment-browser/src/transport/stream.ts +++ b/packages/experiment-browser/src/transport/stream.ts @@ -3,15 +3,13 @@ * @internal */ -import { SSE } from '@amplitude/experiment-core'; +import { SSE, SSEProviderParams } from '@amplitude/experiment-core'; import EventSource from 'eventsource'; export const defaultSseProvider = ( url: string, - headers: Record, + params: SSEProviderParams, ): SSE => { - const es = new EventSource(url, { - headers, - }); + const es = new EventSource(url, params); return es; }; diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts new file mode 100644 index 00000000..e3606d0e --- /dev/null +++ b/packages/experiment-browser/test/stream.test.ts @@ -0,0 +1,144 @@ +/* eslint-disable no-empty */ +import path from 'path'; + +import { + EvaluationVariant, + GetVariantsOptions, + SdkEvaluationApi, + SdkStreamEvaluationApi, + StreamEvaluationApi, +} from '@amplitude/experiment-core'; +import * as dotenv from 'dotenv'; +import EventSource from 'eventsource'; + +import { Defaults } from '../src/config'; +import { FetchHttpClient, WrapperClient } from '../src/transport/http'; + +import { sleep } from './util/misc'; + +dotenv.config({ + path: path.join( + __dirname, + '../', + process.env['ENVIRONMENT'] ? '.env.' + process.env['ENVIRONMENT'] : '.env', + ), +}); + +if (!process.env['MANAGEMENT_API_KEY']) { + throw new Error( + 'No env vars found. If running on local, have you created .env file correct environment variables? Checkout README.md', + ); +} + +const SERVER_URL = process.env['SERVER_URL'] || Defaults.serverUrl; +const STREAM_SERVER_URL = + process.env['STREAM_SERVER_URL'] || Defaults.streamVariantsServerUrl; +const MANAGEMENT_API_SERVER_URL = + process.env['MANAGEMENT_API_SERVER_URL'] || + 'https://experiment.amplitude.com'; +const DEPLOYMENT_KEY = + process.env['DEPLOYMENT_KEY'] || 'client-DvWljIjiiuqLbyjqdvBaLFfEBrAvGuA3'; +const MANAGEMENT_API_KEY = process.env['MANAGEMENT_API_KEY']; +const FLAG_KEY = 'sdk-ci-stream-vardata-test'; + +const USER = {}; +const OPTIONS: GetVariantsOptions = {}; + +// Test stream is successfully connected and data is valid. +// The main purpose is to test and ensure the SDK stream interface works with stream server. +// This test may be flaky if multiple edits to the flag happens simultaneously, +// i.e. multiple invocation of this test is run at the same time. +// If two edits are made in a very very very short period (few seconds), the first edit may not be streamed. +test('SDK stream is compatible with stream server (flaky possible, see comments)', async () => { + const api: StreamEvaluationApi = new SdkStreamEvaluationApi( + DEPLOYMENT_KEY, + STREAM_SERVER_URL, + (url, params) => { + return new EventSource(url, params); + }, + 5000, // A bit more generous timeout than the default. + ); + + const streamVariants: Record[] = []; + let streamError = undefined; + const connectedPromise = new Promise((resolve, reject) => { + api + .streamVariants( + USER, + OPTIONS, + (variants: Record) => { + streamVariants.push(variants); + resolve(); + }, + (err) => { + streamError = err; + reject(err); + }, + ) + .catch((err) => { + reject(err); + }); + }); + await connectedPromise; + + // Get variant from the fetch api to compare. + const httpClient = FetchHttpClient; + const fetchApi = new SdkEvaluationApi( + DEPLOYMENT_KEY, + SERVER_URL, + new WrapperClient(httpClient), + ); + const fetchVariants = await fetchApi.getVariants(USER, OPTIONS); + + // At least one vardata streamed should be the same as the one fetched. + // There can be other updates after stream establishment and before fetch. + await sleep(5000); // Assume there's an update right before fetch but after stream, wait for stream to receive that data. + expect( + // Find the one that match using payload of our test flag. + streamVariants.filter( + (f) => f[FLAG_KEY]['payload'] === fetchVariants[FLAG_KEY]['payload'], + )[0], + ).toStrictEqual(fetchVariants); + + // Test that stream is kept alive. + await sleep(40000); + expect(streamError).toBeUndefined(); + + // Get flag id using management-api. + const getFlagIdRequest = await httpClient.request( + `${MANAGEMENT_API_SERVER_URL}/api/1/flags?key=${FLAG_KEY}`, + 'GET', + { + Authorization: 'Bearer ' + MANAGEMENT_API_KEY, + 'Content-Type': 'application/json', + Accept: '*/*', + }, + '', + ); + expect(getFlagIdRequest.status).toBe(200); + const flagId = JSON.parse(getFlagIdRequest.body)['flags'][0]['id']; + + // Call management api to edit deployment. Then wait for stream to update. + const randNumber = Math.random(); + const modifyFlagReq = await httpClient.request( + `${MANAGEMENT_API_SERVER_URL}/api/1/flags/${flagId}/variants/on`, + 'PATCH', + { + Authorization: 'Bearer ' + MANAGEMENT_API_KEY, + 'Content-Type': 'application/json', + Accept: '*/*', + }, + `{"payload": ${randNumber}}`, + 10000, + ); + expect(modifyFlagReq.status).toBe(200); + await sleep(5000); // 5s is generous enough for update to stream. + + // Check that at least one of the updates happened during this time have the random number we generated. + // This means that the stream is working and we are getting updates. + expect( + streamVariants.filter((f) => f[FLAG_KEY]['payload'] === randNumber).length, + ).toBeGreaterThanOrEqual(1); + + api.close(); +}, 60000); diff --git a/packages/experiment-core/src/api/evaluation-stream-api.ts b/packages/experiment-core/src/api/evaluation-stream-api.ts index 21426e70..92100b78 100644 --- a/packages/experiment-core/src/api/evaluation-stream-api.ts +++ b/packages/experiment-core/src/api/evaluation-stream-api.ts @@ -113,37 +113,39 @@ export class SdkStreamEvaluationApi implements StreamEvaluationApi { } onUpdate?.(variants); }; - const onSignalUpdateCb = async () => { - // Signaled that there's a push. - if (isConnecting) { - isConnecting = false; - clearTimeout(connectionTimeout); - resolve(); - } - if (!this.fetchEvalApi) { - onErrorSseCb( - new Error( - 'No fetchEvalApi provided for variant data that is too large to push.', - ), - ); - return; - } - - let variants; - try { - variants = await this.fetchEvalApi.getVariants(user, options); - } catch (error) { - onErrorSseCb( - new Error('Error fetching variants on signal: ' + error), - ); - } - onUpdate?.(variants || {}); - }; + // The following is to support receiving a signal and fetching the variants. + // This would be helpful if the variant data is too large to be pushed. + // const onSignalUpdateCb = async () => { + // // Signaled that there's a push. + // if (isConnecting) { + // isConnecting = false; + // clearTimeout(connectionTimeout); + // resolve(); + // } + // if (!this.fetchEvalApi) { + // onErrorSseCb( + // new Error( + // 'No fetchEvalApi provided for variant data that is too large to push.', + // ), + // ); + // return; + // } + + // let variants; + // try { + // variants = await this.fetchEvalApi.getVariants(user, options); + // } catch (error) { + // onErrorSseCb( + // new Error('Error fetching variants on signal: ' + error), + // ); + // } + // onUpdate?.(variants || {}); + // }; this.stream.connect( { push_data: onDataUpdateSseCb, - push_signal: onSignalUpdateCb, + // push_signal: onSignalUpdateCb, [DEFAULT_EVENT_TYPE]: onDataUpdateSseCb, }, onErrorSseCb, diff --git a/packages/experiment-core/src/index.ts b/packages/experiment-core/src/index.ts index 60cd3c53..675e2e06 100644 --- a/packages/experiment-core/src/index.ts +++ b/packages/experiment-core/src/index.ts @@ -21,7 +21,12 @@ export { StreamEvaluationApi, SdkStreamEvaluationApi, } from './api/evaluation-stream-api'; -export { SSE, SSEStream, SSEProvider } from './transport/stream'; +export { + SSE, + SSEStream, + SSEProvider, + SSEProviderParams, +} from './transport/stream'; export { Poller } from './util/poller'; export { safeGlobal, diff --git a/packages/experiment-core/src/transport/stream.ts b/packages/experiment-core/src/transport/stream.ts index 126319e3..a58d0117 100644 --- a/packages/experiment-core/src/transport/stream.ts +++ b/packages/experiment-core/src/transport/stream.ts @@ -17,7 +17,10 @@ export interface SSE { ): void; close(): void; } -export type SSEProvider = (url: string, headers: Record) => SSE; +export type SSEProviderParams = { + headers: Record; +}; +export type SSEProvider = (url: string, params: SSEProviderParams) => SSE; export class SSEStream { streamProvider: SSEProvider; @@ -95,7 +98,7 @@ export class SSEStream { this.close(); } - this.es = this.streamProvider(this.url, this.headers); + this.es = this.streamProvider(this.url, { headers: this.headers }); for (const eventType in this.onEventTypeUpdate) { this.es.addEventListener(eventType, (event) => { this.resetKeepAlive(); diff --git a/packages/experiment-core/test/api/evaluation-stream-api.test.ts b/packages/experiment-core/test/api/evaluation-stream-api.test.ts index 832f4c70..fff24f77 100644 --- a/packages/experiment-core/test/api/evaluation-stream-api.test.ts +++ b/packages/experiment-core/test/api/evaluation-stream-api.test.ts @@ -20,9 +20,9 @@ describe('EvaluationStreamApi tests', () => { beforeEach(() => { jest.clearAllMocks(); registeredListeners = {}; - mockStreamProvider = jest.fn((url, header) => { + mockStreamProvider = jest.fn((url, param) => { expect(url).toBe('https://url/sdk/stream/v1/vardata'); - expect(header['Authorization']).toBe('Api-Key apikey'); + expect(param.headers['Authorization']).toBe('Api-Key apikey'); return { addEventListener: jest.fn( (eventType: string, cb: (data: unknown) => void) => { diff --git a/packages/experiment-core/test/transport/stream.test.ts b/packages/experiment-core/test/transport/stream.test.ts index caa824a8..7b269e6d 100644 --- a/packages/experiment-core/test/transport/stream.test.ts +++ b/packages/experiment-core/test/transport/stream.test.ts @@ -32,7 +32,9 @@ describe('SSEStream', () => { // Test new connection makes a call to the stream provider. stream.connect(mockOnDataUpdate, mockOnError); expect(mockStreamProvider).toHaveBeenCalledWith('http://localhost:7999', { - header1: 'value1', + headers: { + header1: 'value1', + }, }); expect(mockStreamProvider).toHaveBeenCalledTimes(1); @@ -83,7 +85,7 @@ describe('SSEStream', () => { { channel1: mockOnData1Update, channel2: mockOnData2Update }, mockOnError, ); - expect(mockStreamProvider).toHaveBeenCalledWith('url', {}); + expect(mockStreamProvider).toHaveBeenCalledWith('url', { headers: {} }); expect(mockStreamProvider).toHaveBeenCalledTimes(1); // Test that the event listener is set up correctly. diff --git a/yarn.lock b/yarn.lock index 52533d9d..888eeac8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -47,6 +47,15 @@ dependencies: "@babel/highlight" "^7.22.5" +"@babel/code-frame@^7.10.4": + version "7.26.2" + resolved "https://registry.yarnpkg.com/@babel/code-frame/-/code-frame-7.26.2.tgz#4b5fab97d33338eff916235055f0ebc21e573a85" + integrity sha512-RJlIHRueQgwWitWgF8OdFYGZX328Ax5BCemNGlqHfplnRT9ESi8JkFlvaVYbS+UubVY6dpv87Fs2u5M29iNFVQ== + dependencies: + "@babel/helper-validator-identifier" "^7.25.9" + js-tokens "^4.0.0" + picocolors "^1.0.0" + "@babel/compat-data@^7.22.5", "@babel/compat-data@^7.22.6", "@babel/compat-data@^7.22.9": version "7.22.9" resolved "https://registry.yarnpkg.com/@babel/compat-data/-/compat-data-7.22.9.tgz#71cdb00a1ce3a329ce4cbec3a44f9fef35669730" @@ -249,6 +258,11 @@ resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.22.5.tgz#9544ef6a33999343c8740fa51350f30eeaaaf193" integrity sha512-aJXu+6lErq8ltp+JhkJUfk1MTGyuA4v7f3pA+BJ5HLfNC6nAQ0Cpi9uOquUj8Hehg0aUiHzWQbOVJGao6ztBAQ== +"@babel/helper-validator-identifier@^7.25.9": + version "7.25.9" + resolved "https://registry.yarnpkg.com/@babel/helper-validator-identifier/-/helper-validator-identifier-7.25.9.tgz#24b64e2c3ec7cd3b3c547729b8d16871f22cbdc7" + integrity sha512-Ed61U6XJc3CVRfkERJWDz4dJwKe7iLmmJsbOGu9wSloNSFttHV0I8g6UAgb7qnK5ly5bGLPd4oXZlxCdANBOWQ== + "@babel/helper-validator-option@^7.22.5": version "7.22.5" resolved "https://registry.yarnpkg.com/@babel/helper-validator-option/-/helper-validator-option-7.22.5.tgz#de52000a15a177413c8234fa3a8af4ee8102d0ac" @@ -974,6 +988,21 @@ resolved "https://registry.yarnpkg.com/@babel/regjsgen/-/regjsgen-0.8.0.tgz#f0ba69b075e1f05fb2825b7fad991e7adbb18310" integrity sha512-x/rqGMdzj+fWZvCOYForTghzbtqPDZ5gPwaoNGHdgDfF2QA/XZbCBp4Moo5scrkAMPhB7z26XM/AaHuIJdgauA== +"@babel/runtime-corejs3@^7.10.2": + version "7.27.0" + resolved "https://registry.yarnpkg.com/@babel/runtime-corejs3/-/runtime-corejs3-7.27.0.tgz#c766df350ec7a2caf3ed64e3659b100954589413" + integrity sha512-UWjX6t+v+0ckwZ50Y5ShZLnlk95pP5MyW/pon9tiYzl3+18pkTHTFNTKr7rQbfRXPkowt2QAn30o1b6oswszew== + dependencies: + core-js-pure "^3.30.2" + regenerator-runtime "^0.14.0" + +"@babel/runtime@^7.10.2", "@babel/runtime@^7.10.3": + version "7.27.0" + resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.27.0.tgz#fbee7cf97c709518ecc1f590984481d5460d4762" + integrity sha512-VtPOkrdPHZsKc/clNqyi9WUA8TINkZ4cGk63UUE3u4pmB2k+ZMQRDuIOagv8UVd6j7k0T3+RRIb7beKTebNbcw== + dependencies: + regenerator-runtime "^0.14.0" + "@babel/runtime@^7.11.2", "@babel/runtime@^7.21.0", "@babel/runtime@^7.8.4": version "7.22.6" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.22.6.tgz#57d64b9ae3cff1d67eb067ae117dac087f5bd438" @@ -1313,6 +1342,17 @@ slash "^3.0.0" write-file-atomic "^4.0.2" +"@jest/types@^26.6.2": + version "26.6.2" + resolved "https://registry.yarnpkg.com/@jest/types/-/types-26.6.2.tgz#bef5a532030e1d88a2f5a6d933f84e97226ed48e" + integrity sha512-fC6QCp7Sc5sX6g8Tvbmj4XUTbyrik0akgRy03yjXbQaBWWNWGE7SGtJk98m0N8nzegD/7SggrUlivxo5ax4KWQ== + dependencies: + "@types/istanbul-lib-coverage" "^2.0.0" + "@types/istanbul-reports" "^3.0.0" + "@types/node" "*" + "@types/yargs" "^15.0.0" + chalk "^4.0.0" + "@jest/types@^29.6.1": version "29.6.1" resolved "https://registry.yarnpkg.com/@jest/types/-/types-29.6.1.tgz#ae79080278acff0a6af5eb49d063385aaa897bf2" @@ -2112,6 +2152,20 @@ dependencies: "@sinonjs/commons" "^3.0.0" +"@testing-library/dom@7.26.0": + version "7.26.0" + resolved "https://registry.yarnpkg.com/@testing-library/dom/-/dom-7.26.0.tgz#da4d052dc426a4ccc916303369c6e7552126f680" + integrity sha512-fyKFrBbS1IigaE3FV21LyeC7kSGF84lqTlSYdKmGaHuK2eYQ/bXVPM5vAa2wx/AU1iPD6oQHsxy2QQ17q9AMCg== + dependencies: + "@babel/code-frame" "^7.10.4" + "@babel/runtime" "^7.10.3" + "@types/aria-query" "^4.2.0" + aria-query "^4.2.2" + chalk "^4.1.0" + dom-accessibility-api "^0.5.1" + lz-string "^1.4.4" + pretty-format "^26.4.2" + "@tootallnate/once@2": version "2.0.0" resolved "https://registry.yarnpkg.com/@tootallnate/once/-/once-2.0.0.tgz#f544a148d3ab35801c1f633a7441fd87c2e484bf" @@ -2135,6 +2189,11 @@ resolved "https://registry.yarnpkg.com/@types/amplitude-js/-/amplitude-js-8.16.2.tgz#2cfb850a01a4171e632498821f92e45bef23c952" integrity sha512-a+tb/CEQOlrHRvEvAuYNOcoUy1POERANnAhfKgiTmsy0eACj3eukGP0ucA9t115QOPzVUhbnUfZqtyHp99IZyA== +"@types/aria-query@^4.2.0": + version "4.2.2" + resolved "https://registry.yarnpkg.com/@types/aria-query/-/aria-query-4.2.2.tgz#ed4e0ad92306a704f9fb132a0cfcf77486dbe2bc" + integrity sha512-HnYpAE1Y6kRyKM/XkEuiRQhTHvkzMBurTHnpFLYLBGPIylZNPs9jJcuOOYWxPLJCSEtmZT0Y8rHDokKN7rRTig== + "@types/babel__core@^7.1.14": version "7.20.1" resolved "https://registry.yarnpkg.com/@types/babel__core/-/babel__core-7.20.1.tgz#916ecea274b0c776fec721e333e55762d3a9614b" @@ -2288,6 +2347,13 @@ resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-21.0.0.tgz#0c60e537fa790f5f9472ed2776c2b71ec117351b" integrity sha512-iO9ZQHkZxHn4mSakYV0vFHAVDyEOIJQrV2uZ06HxEPcx+mt8swXoZHIbaaJ2crJYFfErySgktuTZ3BeLz+XmFA== +"@types/yargs@^15.0.0": + version "15.0.19" + resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-15.0.19.tgz#328fb89e46109ecbdb70c295d96ff2f46dfd01b9" + integrity sha512-2XUaGVmyQjgyAZldf0D0c14vvo/yv0MhQBSTJcejMMaitsn3nxCB6TmH4G0ZQf+uxROOa9mpanoSm8h6SG/1ZA== + dependencies: + "@types/yargs-parser" "*" + "@types/yargs@^17.0.8": version "17.0.24" resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-17.0.24.tgz#b3ef8d50ad4aa6aecf6ddc97c580a00f5aa11902" @@ -2535,7 +2601,7 @@ ansi-escapes@^4.2.1: dependencies: type-fest "^0.21.3" -ansi-regex@^5.0.1: +ansi-regex@^5.0.0, ansi-regex@^5.0.1: version "5.0.1" resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-5.0.1.tgz#082cb2c89c9fe8659a311a53bd6a4dc5301db304" integrity sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ== @@ -2615,6 +2681,14 @@ argparse@^2.0.1: resolved "https://registry.yarnpkg.com/argparse/-/argparse-2.0.1.tgz#246f50f3ca78a3240f6c997e8a9bd1eac49e4b38" integrity sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q== +aria-query@^4.2.2: + version "4.2.2" + resolved "https://registry.yarnpkg.com/aria-query/-/aria-query-4.2.2.tgz#0d2ca6c9aceb56b8977e9fed6aed7e15bbd2f83b" + integrity sha512-o/HelwhuKpTj/frsOsbNLNgnNGVIFsVP/SW2BSF14gVl7kAfMOJ6/8wUAUvG1R1NHKrfG+2sHZTu0yauT1qBrA== + dependencies: + "@babel/runtime" "^7.10.2" + "@babel/runtime-corejs3" "^7.10.2" + array-buffer-byte-length@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/array-buffer-byte-length/-/array-buffer-byte-length-1.0.0.tgz#fabe8bc193fea865f317fe7807085ee0dee5aead" @@ -3356,6 +3430,11 @@ core-js-compat@^3.31.0: dependencies: browserslist "^4.21.9" +core-js-pure@^3.30.2: + version "3.41.0" + resolved "https://registry.yarnpkg.com/core-js-pure/-/core-js-pure-3.41.0.tgz#349fecad168d60807a31e83c99d73d786fe80811" + integrity sha512-71Gzp96T9YPk63aUvE5Q5qP+DryB4ZloUZPSOebGM88VNw8VNfvdA7z6kGA8iGOTEzAomsRidp4jXSmUIJsL+Q== + core-util-is@~1.0.0: version "1.0.3" resolved "https://registry.yarnpkg.com/core-util-is/-/core-util-is-1.0.3.tgz#a6042d3634c2b27e9328f837b965fac83808db85" @@ -3574,6 +3653,11 @@ doctrine@^3.0.0: dependencies: esutils "^2.0.2" +dom-accessibility-api@^0.5.1: + version "0.5.16" + resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.5.16.tgz#5a7429e6066eb3664d911e33fb0e45de8eb08453" + integrity sha512-X7BJ2yElsnOJ30pZF4uIIDfBEVgF4XEBxL9Bxhy6dnrm5hkzqmsWHGTiHqRiITNhMyFLyAiWndIJP7Z1NTteDg== + "dom-mutator@git+ssh://git@github.com:amplitude/dom-mutator#main": version "0.7.1" resolved "git+ssh://git@github.com:amplitude/dom-mutator#20ef4fa76e5e35107efd74cac93baf659cd4cabb" @@ -6004,6 +6088,11 @@ lunr@^2.3.9: resolved "https://registry.yarnpkg.com/lunr/-/lunr-2.3.9.tgz#18b123142832337dd6e964df1a5a7707b25d35e1" integrity sha512-zTU3DaZaF3Rt9rhN3uBMGQD3dD2/vFQqnvZCDv4dl5iOzq2IZQqTxu90r4E5J+nP70J3ilqVCrbho2eWaeW8Ow== +lz-string@^1.4.4: + version "1.5.0" + resolved "https://registry.yarnpkg.com/lz-string/-/lz-string-1.5.0.tgz#c1ab50f77887b712621201ba9fd4e3a6ed099941" + integrity sha512-h5bgJWpxJNswbU7qCrV0tIKQCaS3blPDrqKWx+QxzuzL1zGUzij9XCWLrSLsJPu5t+eWA/ycetzYAO5IOMcWAQ== + magic-string@^0.25.7: version "0.25.9" resolved "https://registry.yarnpkg.com/magic-string/-/magic-string-0.25.9.tgz#de7f9faf91ef8a1c91d02c2e5314c8277dbcdd1c" @@ -7131,6 +7220,16 @@ pretty-format@29.4.3: ansi-styles "^5.0.0" react-is "^18.0.0" +pretty-format@^26.4.2: + version "26.6.2" + resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-26.6.2.tgz#e35c2705f14cb7fe2fe94fa078345b444120fc93" + integrity sha512-7AeGuCYNGmycyQbCqd/3PWH4eOoX/OiCa0uphp57NVTeAGdJGaAliecxwBDHYQCIvrW7aDBZCYeNTP/WX69mkg== + dependencies: + "@jest/types" "^26.6.2" + ansi-regex "^5.0.0" + ansi-styles "^4.0.0" + react-is "^17.0.1" + pretty-format@^29.0.0, pretty-format@^29.6.2: version "29.6.2" resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-29.6.2.tgz#3d5829261a8a4d89d8b9769064b29c50ed486a47" @@ -7278,6 +7377,11 @@ randombytes@^2.1.0: dependencies: safe-buffer "^5.1.0" +react-is@^17.0.1: + version "17.0.2" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0" + integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== + react-is@^18.0.0: version "18.2.0" resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.2.0.tgz#199431eeaaa2e09f86427efbb4f1473edb47609b" @@ -7440,6 +7544,11 @@ regenerator-runtime@^0.13.11: resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.13.11.tgz#f6dca3e7ceec20590d07ada785636a90cdca17f9" integrity sha512-kY1AZVr2Ra+t+piVaJ4gxaFaReZVH40AKNo7UCX6W+dEwBo/2oZJzqfuN1qLq1oL45o56cPaTXELwrTh8Fpggg== +regenerator-runtime@^0.14.0: + version "0.14.1" + resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.14.1.tgz#356ade10263f685dda125100cd862c1db895327f" + integrity sha512-dYnhHh0nJoMfnkZs6GmmhFknAGRrLznOu5nc9ML+EJxGvrx6H7teuevqVqCuPcPK//3eDrrjQhehXVx9cnkGdw== + regenerator-transform@^0.15.1: version "0.15.1" resolved "https://registry.yarnpkg.com/regenerator-transform/-/regenerator-transform-0.15.1.tgz#f6c4e99fc1b4591f780db2586328e4d9a9d8dc56" From cf86ff2af54778f0f15c634a3f2d034c5c974df2 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 23 Apr 2025 16:26:45 -0700 Subject: [PATCH 07/22] fix: pull variant update into func --- .../src/experimentClient.ts | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index f33991b2..bfde4ed0 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -261,7 +261,7 @@ export class ExperimentClient implements Client { * Stop the local flag configuration poller. */ public stop() { - this.variantUpdater.stop(); + this.variantUpdater.stop(); // Stop the variant updater, no waiting needed. if (!this.isRunning) { return; } @@ -305,11 +305,8 @@ export class ExperimentClient implements Client { user = this.cleanUserPropsForFetch(user); await this.variantUpdater.start( async (results: Record) => { - const variants: Variants = {}; - for (const key of Object.keys(results)) { - variants[key] = convertEvaluationVariantToVariant(results[key]); - } - await this.storeVariants(variants, options); + // On receiving variants update. + await this.processVariants(results, options); }, (err) => { console.error(err); @@ -319,6 +316,17 @@ export class ExperimentClient implements Client { return this; } + private async processVariants( + flagKeyToVariant: Record, + options?: FetchOptions, + ): Promise { + const variants: Variants = {}; + Object.entries(flagKeyToVariant).forEach(([key, variant]) => { + variants[key] = convertEvaluationVariantToVariant(variant); + }); + await this.storeVariants(variants, options); + } + /** * Returns the variant for the provided key. * From e4767108a0f1842c4908eabb439184d8bf498efa Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Wed, 23 Apr 2025 16:28:23 -0700 Subject: [PATCH 08/22] fix: CI test --- .github/workflows/test.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dc7e6701..8d763a85 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -47,3 +47,5 @@ jobs: - name: Test run: yarn test + env: + MANAGEMENT_API_KEY: ${{ secrets.MANAGEMENT_API_KEY }} From 9c6668ee14a396ad128596cc9a098762066bd07e Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 25 Apr 2025 04:12:12 -0700 Subject: [PATCH 09/22] feat: more logs, add test retry --- packages/experiment-browser/src/experimentClient.ts | 1 + packages/experiment-browser/src/util/updaters.ts | 5 +++++ packages/experiment-browser/test/stream.test.ts | 1 + 3 files changed, 7 insertions(+) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index bfde4ed0..9544754c 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -306,6 +306,7 @@ export class ExperimentClient implements Client { await this.variantUpdater.start( async (results: Record) => { // On receiving variants update. + this.debug('[Experiment] Received variants update'); await this.processVariants(results, options); }, (err) => { diff --git a/packages/experiment-browser/src/util/updaters.ts b/packages/experiment-browser/src/util/updaters.ts index 6a5ba605..609bb29f 100644 --- a/packages/experiment-browser/src/util/updaters.ts +++ b/packages/experiment-browser/src/util/updaters.ts @@ -82,6 +82,11 @@ export class VariantsStreamUpdater implements VariantUpdater { }, ); } catch (error) { + console.error( + '[Experiment] Stream updater failed to start: ' + + error + + error.statusCode, + ); await this.handleError(error); throw error; } diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts index e3606d0e..4f8bfab8 100644 --- a/packages/experiment-browser/test/stream.test.ts +++ b/packages/experiment-browser/test/stream.test.ts @@ -49,6 +49,7 @@ const OPTIONS: GetVariantsOptions = {}; // This test may be flaky if multiple edits to the flag happens simultaneously, // i.e. multiple invocation of this test is run at the same time. // If two edits are made in a very very very short period (few seconds), the first edit may not be streamed. +jest.retryTimes(2); test('SDK stream is compatible with stream server (flaky possible, see comments)', async () => { const api: StreamEvaluationApi = new SdkStreamEvaluationApi( DEPLOYMENT_KEY, From ca2cef57e077bfaf71964fe6dad6d31bce783b36 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 25 Apr 2025 14:06:12 -0700 Subject: [PATCH 10/22] beta release --- .github/workflows/release.yml | 2 ++ lerna.json | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bf03c02e..8d27150a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -48,6 +48,8 @@ jobs: - name: Test run: yarn test + env: + MANAGEMENT_API_KEY: ${{ secrets.MANAGEMENT_API_KEY }} - name: Configure Git User run: | diff --git a/lerna.json b/lerna.json index fbc25b49..852a9709 100644 --- a/lerna.json +++ b/lerna.json @@ -7,7 +7,7 @@ "useWorkspaces": true, "command": { "version": { - "allowBranch": "main", + "allowBranch": "stream-vardata", "conventionalCommits": true, "createRelease": "github", "message": "chore(release): publish", From a99270fc497c2e7610cf29dc4c19357facc5020c Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 25 Apr 2025 14:11:11 -0700 Subject: [PATCH 11/22] release workflow --- .github/workflows/release.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8d27150a..407b63c8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -60,7 +60,8 @@ jobs: if: ${{ github.event.inputs.dryRun == 'true'}} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: yarn lerna version --no-push --no-git-tag-version --loglevel silly --yes + run: | + yarn lerna version --conventional-prerelease --no-private --no-push --no-git-tag-version --loglevel silly --yes - name: Setup NPM Token if: ${{ github.event.inputs.dryRun == 'false'}} @@ -74,5 +75,5 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - yarn lerna version --yes - yarn lerna publish from-git --yes --loglevel silly + yarn lerna version --conventional-prerelease --no-private + yarn lerna publish from-git --loglevel silly From 5a8f7e2dc9a687d14f2f8fbf73be05acec48b5ce Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 25 Apr 2025 14:58:24 -0700 Subject: [PATCH 12/22] beta tag --- .github/workflows/release.yml | 4 ++-- lerna.json | 13 ++++++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 407b63c8..19bb4702 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -61,7 +61,7 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - yarn lerna version --conventional-prerelease --no-private --no-push --no-git-tag-version --loglevel silly --yes + yarn lerna version --no-push --no-git-tag-version --loglevel silly --yes - name: Setup NPM Token if: ${{ github.event.inputs.dryRun == 'false'}} @@ -75,5 +75,5 @@ jobs: env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | - yarn lerna version --conventional-prerelease --no-private + yarn lerna version yarn lerna publish from-git --loglevel silly diff --git a/lerna.json b/lerna.json index 852a9709..f6dc39a8 100644 --- a/lerna.json +++ b/lerna.json @@ -11,7 +11,18 @@ "conventionalCommits": true, "createRelease": "github", "message": "chore(release): publish", - "preid": "beta" + "preid": "beta", + "conventionalPrerelease": true, + "noPrivate": true, + "noGitTagVersion": true, + "noPush": true, + "noChangeLog": true, + "distTag": "beta", + "preDistTag": "beta" + }, + "publishConfig": { + "distTag": "beta", + "preDistTag": "beta" } } } From 49980c08ddd94c5af1286d18829753085d54c102 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Fri, 25 Apr 2025 23:46:07 -0700 Subject: [PATCH 13/22] fix: rename utils --- packages/experiment-tag/src/experiment.ts | 2 +- packages/experiment-tag/src/{util.ts => utils.ts} | 0 packages/experiment-tag/test/experiment.test.ts | 6 +++--- .../experiment-tag/test/{util.test.ts => utils.test.ts} | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) rename packages/experiment-tag/src/{util.ts => utils.ts} (100%) rename packages/experiment-tag/test/{util.test.ts => utils.test.ts} (99%) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 88501892..32048b4b 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -31,7 +31,7 @@ import { urlWithoutParamsAndAnchor, UUID, concatenateQueryParamsOf, -} from './util'; +} from './utils'; import { WebExperimentClient } from './web-experiment'; export const PAGE_NOT_TARGETED_SEGMENT_NAME = 'Page not targeted'; diff --git a/packages/experiment-tag/src/util.ts b/packages/experiment-tag/src/utils.ts similarity index 100% rename from packages/experiment-tag/src/util.ts rename to packages/experiment-tag/src/utils.ts diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 495cd40b..14152fa9 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -7,7 +7,7 @@ import { PAGE_NOT_TARGETED_SEGMENT_NAME, DefaultWebExperimentClient, } from 'src/experiment'; -import * as util from 'src/util'; +import * as utils from 'src/utils'; import { stringify } from 'ts-jest'; import { @@ -65,7 +65,7 @@ describe('initializeExperiment', () => { jest.spyOn(ExperimentClient.prototype, 'setUser'); jest.spyOn(ExperimentClient.prototype, 'all'); const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); - jest.spyOn(util, 'UUID').mockReturnValue('mock'); + jest.spyOn(utils, 'UUID').mockReturnValue('mock'); let mockGlobal; let antiFlickerSpy; @@ -769,7 +769,7 @@ describe('helper methods', () => { // @ts-ignore mockGetGlobalScope.mockReturnValue(mockGlobal); apiKey++; - jest.spyOn(util, 'UUID').mockReturnValue('mock'); + jest.spyOn(utils, 'UUID').mockReturnValue('mock'); jest.clearAllMocks(); }); diff --git a/packages/experiment-tag/test/util.test.ts b/packages/experiment-tag/test/utils.test.ts similarity index 99% rename from packages/experiment-tag/test/util.test.ts rename to packages/experiment-tag/test/utils.test.ts index d1271d98..8882926c 100644 --- a/packages/experiment-tag/test/util.test.ts +++ b/packages/experiment-tag/test/utils.test.ts @@ -4,7 +4,7 @@ import { getUrlParams, matchesUrl, urlWithoutParamsAndAnchor, -} from 'src/util'; +} from 'src/utils'; // Mock the getGlobalScope function const spyGetGlobalScope = jest.spyOn(coreUtil, 'getGlobalScope'); From 3e3805839ae91bde7c2b4738bff1781f0b180828 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Sat, 26 Apr 2025 15:23:20 -0700 Subject: [PATCH 14/22] beta release cmd --- .github/workflows/release.yml | 5 +++-- lerna.json | 12 +----------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 19bb4702..01e5aaef 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -74,6 +74,7 @@ jobs: if: ${{ github.event.inputs.dryRun == 'false'}} env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + # Adds all params to both version and publish just to be safe. run: | - yarn lerna version - yarn lerna publish from-git --loglevel silly + yarn lerna version --conventionalPrerelease --preid=beta --no-private + yarn lerna publish from-git --loglevel silly --dist-tag=beta --pre-dist-tag=beta --conventionalPrerelease --preid=beta --no-private diff --git a/lerna.json b/lerna.json index f6dc39a8..0a517788 100644 --- a/lerna.json +++ b/lerna.json @@ -12,17 +12,7 @@ "createRelease": "github", "message": "chore(release): publish", "preid": "beta", - "conventionalPrerelease": true, - "noPrivate": true, - "noGitTagVersion": true, - "noPush": true, - "noChangeLog": true, - "distTag": "beta", - "preDistTag": "beta" - }, - "publishConfig": { - "distTag": "beta", - "preDistTag": "beta" + "conventionalPrerelease": true } } } From 41dea41ed78f773e8db5bd8429c11690d0482885 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 8 May 2025 09:48:04 -0700 Subject: [PATCH 15/22] test: updated test name --- packages/experiment-core/test/transport/stream.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/experiment-core/test/transport/stream.test.ts b/packages/experiment-core/test/transport/stream.test.ts index 7b269e6d..4a074289 100644 --- a/packages/experiment-core/test/transport/stream.test.ts +++ b/packages/experiment-core/test/transport/stream.test.ts @@ -16,7 +16,7 @@ describe('SSEStream', () => { })); }); - test('should connect and receive data', async () => { + test('should connect and receive data, keeps alive, reconnects', async () => { const stream = new SSEStream( mockStreamProvider, 'http://localhost:7999', From 46f00a2b3a662c5c74ecb713bcf0d864bcb24246 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 12 Aug 2025 14:56:53 -0700 Subject: [PATCH 16/22] fix: open handles --- .../experiment-browser/test/stream.test.ts | 188 +++++++++--------- 1 file changed, 99 insertions(+), 89 deletions(-) diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts index 4f8bfab8..b2633469 100644 --- a/packages/experiment-browser/test/stream.test.ts +++ b/packages/experiment-browser/test/stream.test.ts @@ -50,96 +50,106 @@ const OPTIONS: GetVariantsOptions = {}; // i.e. multiple invocation of this test is run at the same time. // If two edits are made in a very very very short period (few seconds), the first edit may not be streamed. jest.retryTimes(2); -test('SDK stream is compatible with stream server (flaky possible, see comments)', async () => { - const api: StreamEvaluationApi = new SdkStreamEvaluationApi( - DEPLOYMENT_KEY, - STREAM_SERVER_URL, - (url, params) => { - return new EventSource(url, params); - }, - 5000, // A bit more generous timeout than the default. - ); - const streamVariants: Record[] = []; - let streamError = undefined; - const connectedPromise = new Promise((resolve, reject) => { - api - .streamVariants( - USER, - OPTIONS, - (variants: Record) => { - streamVariants.push(variants); - resolve(); - }, - (err) => { - streamError = err; - reject(err); - }, - ) - .catch((err) => { - reject(err); - }); +describe('SDK stream', () => { + + let api: StreamEvaluationApi; + beforeEach(() => { + jest.clearAllMocks(); + api = new SdkStreamEvaluationApi( + DEPLOYMENT_KEY, + STREAM_SERVER_URL, + (url, params) => { + return new EventSource(url, params); + }, + 5000, // A bit more generous timeout than the default. + ); }); - await connectedPromise; - - // Get variant from the fetch api to compare. - const httpClient = FetchHttpClient; - const fetchApi = new SdkEvaluationApi( - DEPLOYMENT_KEY, - SERVER_URL, - new WrapperClient(httpClient), - ); - const fetchVariants = await fetchApi.getVariants(USER, OPTIONS); - - // At least one vardata streamed should be the same as the one fetched. - // There can be other updates after stream establishment and before fetch. - await sleep(5000); // Assume there's an update right before fetch but after stream, wait for stream to receive that data. - expect( - // Find the one that match using payload of our test flag. - streamVariants.filter( - (f) => f[FLAG_KEY]['payload'] === fetchVariants[FLAG_KEY]['payload'], - )[0], - ).toStrictEqual(fetchVariants); - - // Test that stream is kept alive. - await sleep(40000); - expect(streamError).toBeUndefined(); - - // Get flag id using management-api. - const getFlagIdRequest = await httpClient.request( - `${MANAGEMENT_API_SERVER_URL}/api/1/flags?key=${FLAG_KEY}`, - 'GET', - { - Authorization: 'Bearer ' + MANAGEMENT_API_KEY, - 'Content-Type': 'application/json', - Accept: '*/*', - }, - '', - ); - expect(getFlagIdRequest.status).toBe(200); - const flagId = JSON.parse(getFlagIdRequest.body)['flags'][0]['id']; - - // Call management api to edit deployment. Then wait for stream to update. - const randNumber = Math.random(); - const modifyFlagReq = await httpClient.request( - `${MANAGEMENT_API_SERVER_URL}/api/1/flags/${flagId}/variants/on`, - 'PATCH', - { - Authorization: 'Bearer ' + MANAGEMENT_API_KEY, - 'Content-Type': 'application/json', - Accept: '*/*', - }, - `{"payload": ${randNumber}}`, - 10000, - ); - expect(modifyFlagReq.status).toBe(200); - await sleep(5000); // 5s is generous enough for update to stream. - // Check that at least one of the updates happened during this time have the random number we generated. - // This means that the stream is working and we are getting updates. - expect( - streamVariants.filter((f) => f[FLAG_KEY]['payload'] === randNumber).length, - ).toBeGreaterThanOrEqual(1); + afterEach(() => { + api.close(); + }); - api.close(); -}, 60000); + test('SDK stream is compatible with stream server (flaky possible, see comments)', async () => { + const streamVariants: Record[] = []; + let streamError = undefined; + const connectedPromise = new Promise((resolve, reject) => { + api + .streamVariants( + USER, + OPTIONS, + (variants: Record) => { + streamVariants.push(variants); + resolve(); + }, + (err) => { + streamError = err; + reject(err); + }, + ) + .catch((err) => { + reject(err); + }); + }); + await connectedPromise; + + // Get variant from the fetch api to compare. + const httpClient = FetchHttpClient; + const fetchApi = new SdkEvaluationApi( + DEPLOYMENT_KEY, + SERVER_URL, + new WrapperClient(httpClient), + ); + const fetchVariants = await fetchApi.getVariants(USER, OPTIONS); + + // At least one vardata streamed should be the same as the one fetched. + // There can be other updates after stream establishment and before fetch. + await sleep(5000); // Assume there's an update right before fetch but after stream, wait for stream to receive that data. + expect( + // Find the one that match using payload of our test flag. + streamVariants.filter( + (f) => f[FLAG_KEY]['payload'] === fetchVariants[FLAG_KEY]['payload'], + )[0], + ).toStrictEqual(fetchVariants); + + // Test that stream is kept alive. + await sleep(40000); + expect(streamError).toBeUndefined(); + + // Get flag id using management-api. + const getFlagIdRequest = await httpClient.request( + `${MANAGEMENT_API_SERVER_URL}/api/1/flags?key=${FLAG_KEY}`, + 'GET', + { + Authorization: 'Bearer ' + MANAGEMENT_API_KEY, + 'Content-Type': 'application/json', + Accept: '*/*', + }, + '', + ); + expect(getFlagIdRequest.status).toBe(200); + const flagId = JSON.parse(getFlagIdRequest.body)['flags'][0]['id']; + + // Call management api to edit deployment. Then wait for stream to update. + const randNumber = Math.random(); + const modifyFlagReq = await httpClient.request( + `${MANAGEMENT_API_SERVER_URL}/api/1/flags/${flagId}/variants/on`, + 'PATCH', + { + Authorization: 'Bearer ' + MANAGEMENT_API_KEY, + 'Content-Type': 'application/json', + Accept: '*/*', + }, + `{"payload": ${randNumber}}`, + 10000, + ); + expect(modifyFlagReq.status).toBe(200); + await sleep(5000); // 5s is generous enough for update to stream. + + // Check that at least one of the updates happened during this time have the random number we generated. + // This means that the stream is working and we are getting updates. + expect( + streamVariants.filter((f) => f[FLAG_KEY]['payload'] === randNumber).length, + ).toBeGreaterThanOrEqual(1); + }, 60000); +}); \ No newline at end of file From 76443bee55f3c8c1098d546a34e0c3da363ac865 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 12 Aug 2025 16:20:08 -0700 Subject: [PATCH 17/22] fix: test cors --- packages/experiment-browser/jest.config.js | 8 ++++++++ packages/experiment-browser/test/stream.test.ts | 3 +++ 2 files changed, 11 insertions(+) diff --git a/packages/experiment-browser/jest.config.js b/packages/experiment-browser/jest.config.js index b853122b..56ea72cc 100644 --- a/packages/experiment-browser/jest.config.js +++ b/packages/experiment-browser/jest.config.js @@ -1,9 +1,13 @@ /* eslint-disable @typescript-eslint/no-var-requires */ const { pathsToModuleNameMapper } = require('ts-jest'); +const dotenv = require('dotenv'); const package = require('./package'); const { compilerOptions } = require('./tsconfig.test.json'); +dotenv.config({path: process.env['ENVIRONMENT'] ? '.env.' + process.env['ENVIRONMENT'] : '.env'}); +const MANAGEMENT_API_SERVER_URL = process.env['MANAGEMENT_API_SERVER_URL'] || 'https://experiment.amplitude.com'; + module.exports = { preset: 'ts-jest', testEnvironment: 'jsdom', @@ -16,4 +20,8 @@ module.exports = { '^.+\\.tsx?$': ['ts-jest', { tsconfig: './tsconfig.test.json' }], }, testTimeout: 10 * 1000, + // Remove this once management-api has CORS properly configured. + testEnvironmentOptions: { + url: MANAGEMENT_API_SERVER_URL, + }, }; diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts index b2633469..c4400102 100644 --- a/packages/experiment-browser/test/stream.test.ts +++ b/packages/experiment-browser/test/stream.test.ts @@ -16,6 +16,8 @@ import { FetchHttpClient, WrapperClient } from '../src/transport/http'; import { sleep } from './util/misc'; +console.log('ENVIRONMENT', process.env['ENVIRONMENT']); + dotenv.config({ path: path.join( __dirname, @@ -128,6 +130,7 @@ describe('SDK stream', () => { '', ); expect(getFlagIdRequest.status).toBe(200); + console.log('managed'); const flagId = JSON.parse(getFlagIdRequest.body)['flags'][0]['id']; // Call management api to edit deployment. Then wait for stream to update. From 0406400e961bc4e14bf5727b66eee41d3cfe599d Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 12 Aug 2025 16:35:26 -0700 Subject: [PATCH 18/22] fix: print stream server url instead of env --- packages/experiment-browser/test/stream.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts index c4400102..f5765f68 100644 --- a/packages/experiment-browser/test/stream.test.ts +++ b/packages/experiment-browser/test/stream.test.ts @@ -16,8 +16,6 @@ import { FetchHttpClient, WrapperClient } from '../src/transport/http'; import { sleep } from './util/misc'; -console.log('ENVIRONMENT', process.env['ENVIRONMENT']); - dotenv.config({ path: path.join( __dirname, @@ -43,6 +41,8 @@ const DEPLOYMENT_KEY = const MANAGEMENT_API_KEY = process.env['MANAGEMENT_API_KEY']; const FLAG_KEY = 'sdk-ci-stream-vardata-test'; +console.log('Testing stream server:', STREAM_SERVER_URL); + const USER = {}; const OPTIONS: GetVariantsOptions = {}; From 82c61fae904811149480bf4a362e45fedb4ad6c2 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 12 Aug 2025 16:36:38 -0700 Subject: [PATCH 19/22] fix: delete extra print --- packages/experiment-browser/test/stream.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts index f5765f68..1410ea34 100644 --- a/packages/experiment-browser/test/stream.test.ts +++ b/packages/experiment-browser/test/stream.test.ts @@ -130,7 +130,6 @@ describe('SDK stream', () => { '', ); expect(getFlagIdRequest.status).toBe(200); - console.log('managed'); const flagId = JSON.parse(getFlagIdRequest.body)['flags'][0]['id']; // Call management api to edit deployment. Then wait for stream to update. From 4e9ae5532d065401c21f206f18ab0e54bf34ebf7 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Thu, 25 Sep 2025 01:08:28 -0700 Subject: [PATCH 20/22] fix: dont reconnect if no params has changed --- .../experiment-browser/src/util/updaters.ts | 30 ++++++++-- .../experiment-browser/test/stream.test.ts | 1 + .../test/util/updaters.test.ts | 60 +++++++++++++++++++ 3 files changed, 87 insertions(+), 4 deletions(-) diff --git a/packages/experiment-browser/src/util/updaters.ts b/packages/experiment-browser/src/util/updaters.ts index 609bb29f..5bfff953 100644 --- a/packages/experiment-browser/src/util/updaters.ts +++ b/packages/experiment-browser/src/util/updaters.ts @@ -57,6 +57,10 @@ function isErrorRetriable(e: Error | ErrorEvent): boolean { export class VariantsStreamUpdater implements VariantUpdater { private evaluationApi: StreamEvaluationApi; private hasNonretriableError = false; + private streamOk = false; + private lastParams: {user: ExperimentUser, options: GetVariantsOptions} | undefined; + private onUpdate: VariantUpdateCallback | undefined; + private onError: VariantErrorCallback | undefined; constructor(evaluationApi: StreamEvaluationApi) { this.evaluationApi = evaluationApi; @@ -70,17 +74,32 @@ export class VariantsStreamUpdater implements VariantUpdater { if (this.hasNonretriableError) { throw new Error('Stream updater has non-retriable error, not starting'); } + const {user, options} = params; + + if (this.streamOk && JSON.stringify(this.lastParams?.user) === JSON.stringify(user) && JSON.stringify(this.lastParams?.options) === JSON.stringify(options)) { + this.onUpdate = onUpdate; + this.onError = onError; + return; + } await this.stop(); + + this.onUpdate = onUpdate; + this.onError = onError; try { await this.evaluationApi.streamVariants( - params.user, - params.options, - onUpdate, + user, + options, + (data) => { + this.onUpdate?.(data); + }, async (error) => { + const onError = this.onError; await this.handleError(error); - onError(error); + onError?.(error); }, ); + this.streamOk = true; + this.lastParams = {user, options}; } catch (error) { console.error( '[Experiment] Stream updater failed to start: ' + @@ -103,7 +122,10 @@ export class VariantsStreamUpdater implements VariantUpdater { } async stop(): Promise { + this.streamOk = false; await this.evaluationApi.close(); + this.onUpdate = undefined; + this.onError = undefined; } } diff --git a/packages/experiment-browser/test/stream.test.ts b/packages/experiment-browser/test/stream.test.ts index 1410ea34..c7f9b10d 100644 --- a/packages/experiment-browser/test/stream.test.ts +++ b/packages/experiment-browser/test/stream.test.ts @@ -31,6 +31,7 @@ if (!process.env['MANAGEMENT_API_KEY']) { } const SERVER_URL = process.env['SERVER_URL'] || Defaults.serverUrl; +// const FLAGS_SERVER_URL = process.env['FLAGS_SERVER_URL'] || Defaults.flagsServerUrl; const STREAM_SERVER_URL = process.env['STREAM_SERVER_URL'] || Defaults.streamVariantsServerUrl; const MANAGEMENT_API_SERVER_URL = diff --git a/packages/experiment-browser/test/util/updaters.test.ts b/packages/experiment-browser/test/util/updaters.test.ts index c53f5aca..da172586 100644 --- a/packages/experiment-browser/test/util/updaters.test.ts +++ b/packages/experiment-browser/test/util/updaters.test.ts @@ -94,6 +94,66 @@ describe('VariantsStreamUpdater tests', () => { expect(mockStreamApi.close).toHaveBeenCalledTimes(2); // Close called again on error. expect(updater['hasNonretriableError']).toBe(false); }); + + test('connect success then re-start with same params, should not reconnect', async () => { + const mockStreamApi = { + streamVariants: jest.fn(), + close: jest.fn(), + }; + const updater = new VariantsStreamUpdater(mockStreamApi); + const onUpdate = jest.fn(); + await updater.start(onUpdate, jest.fn(), { + user: {}, + config: {}, + options: {}, + }); + expect(mockStreamApi.streamVariants).toHaveBeenCalledTimes(1); + mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {} }); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(mockStreamApi.close).toHaveBeenCalledTimes(1); + + const onUpdate2 = jest.fn(); + await updater.start(onUpdate2, jest.fn(), { + user: {}, + config: {}, + options: {}, + }); + expect(mockStreamApi.streamVariants).toHaveBeenCalledTimes(1); + mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {} }); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate2).toHaveBeenCalledTimes(1); + expect(mockStreamApi.close).toHaveBeenCalledTimes(1); + }); + + test('connect success then params change, should reconnect', async () => { + const mockStreamApi = { + streamVariants: jest.fn(), + close: jest.fn(), + }; + const updater = new VariantsStreamUpdater(mockStreamApi); + const onUpdate = jest.fn(); + await updater.start(onUpdate, jest.fn(), { + user: {}, + config: {}, + options: {}, + }); + expect(mockStreamApi.streamVariants).toHaveBeenCalledTimes(1); + mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {} }); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(mockStreamApi.close).toHaveBeenCalledTimes(1); + + const onUpdate2 = jest.fn(); + await updater.start(onUpdate2, jest.fn(), { + user: {user_id: '123'}, + config: {}, + options: {}, + }); + expect(mockStreamApi.streamVariants).toHaveBeenCalledTimes(2); + mockStreamApi.streamVariants.mock.lastCall[2]({ 'test-flag': {} }); + expect(onUpdate).toHaveBeenCalledTimes(1); + expect(onUpdate2).toHaveBeenCalledTimes(1); + expect(mockStreamApi.close).toHaveBeenCalledTimes(2); + }); }); describe('VariantsFetchUpdater tests', () => { From 653c069041b3b5daf6f79403dbc235486d3dd46b Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Mon, 29 Sep 2025 13:00:54 -0700 Subject: [PATCH 21/22] fix: equals for stream dedupe --- .../experiment-browser/src/util/equals.ts | 54 +++++++++++++++++++ .../experiment-browser/src/util/updaters.ts | 3 +- .../test/util/updaters.test.ts | 4 +- 3 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 packages/experiment-browser/src/util/equals.ts diff --git a/packages/experiment-browser/src/util/equals.ts b/packages/experiment-browser/src/util/equals.ts new file mode 100644 index 00000000..c8069f4a --- /dev/null +++ b/packages/experiment-browser/src/util/equals.ts @@ -0,0 +1,54 @@ +// eslint-disable-next-line @typescript-eslint/no-explicit-any +export const isEqual = (obj1: any, obj2: any): boolean => { + const primitive = ['string', 'number', 'boolean', 'undefined']; + const typeA = typeof obj1; + const typeB = typeof obj2; + if (typeA !== typeB) { + return false; + } + for (const p of primitive) { + if (p === typeA) { + return obj1 === obj2; + } + } + // check null + if (obj1 == null && obj2 == null) { + return true; + } else if (obj1 == null || obj2 == null) { + return false; + } + // if got here - objects + if (obj1.length !== obj2.length) { + return false; + } + //check if arrays + const isArrayA = Array.isArray(obj1); + const isArrayB = Array.isArray(obj2); + if (isArrayA !== isArrayB) { + return false; + } + if (isArrayA && isArrayB) { + //arrays + for (let i = 0; i < obj1.length; i++) { + if (!isEqual(obj1[i], obj2[i])) { + return false; + } + } + } else { + //objects + const sorted1 = Object.keys(obj1).sort(); + const sorted2 = Object.keys(obj2).sort(); + if (!isEqual(sorted1, sorted2)) { + return false; + } + //compare object values + let result = true; + Object.keys(obj1).forEach((key) => { + if (!isEqual(obj1[key], obj2[key])) { + result = false; + } + }); + return result; + } + return true; +}; diff --git a/packages/experiment-browser/src/util/updaters.ts b/packages/experiment-browser/src/util/updaters.ts index 5bfff953..087ff717 100644 --- a/packages/experiment-browser/src/util/updaters.ts +++ b/packages/experiment-browser/src/util/updaters.ts @@ -12,6 +12,7 @@ import { FetchOptions } from '../types/client'; import { ExperimentUser } from '../types/user'; import { Backoff } from './backoff'; +import { isEqual } from './equals'; export interface Updater { start( @@ -76,7 +77,7 @@ export class VariantsStreamUpdater implements VariantUpdater { } const {user, options} = params; - if (this.streamOk && JSON.stringify(this.lastParams?.user) === JSON.stringify(user) && JSON.stringify(this.lastParams?.options) === JSON.stringify(options)) { + if (this.streamOk && isEqual(this.lastParams?.user, user) && isEqual(this.lastParams?.options, options)) { this.onUpdate = onUpdate; this.onError = onError; return; diff --git a/packages/experiment-browser/test/util/updaters.test.ts b/packages/experiment-browser/test/util/updaters.test.ts index da172586..c8655896 100644 --- a/packages/experiment-browser/test/util/updaters.test.ts +++ b/packages/experiment-browser/test/util/updaters.test.ts @@ -103,7 +103,7 @@ describe('VariantsStreamUpdater tests', () => { const updater = new VariantsStreamUpdater(mockStreamApi); const onUpdate = jest.fn(); await updater.start(onUpdate, jest.fn(), { - user: {}, + user: {user_id: "b", device_id: "c"}, config: {}, options: {}, }); @@ -114,7 +114,7 @@ describe('VariantsStreamUpdater tests', () => { const onUpdate2 = jest.fn(); await updater.start(onUpdate2, jest.fn(), { - user: {}, + user: {device_id: "c", user_id: "b"}, config: {}, options: {}, }); From bbd160694339fa7418deac32a6c81d289102ced7 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Tue, 28 Oct 2025 12:57:17 -0700 Subject: [PATCH 22/22] fix: new equals method --- .../analytics-connector/src/util/equals.ts | 93 +++++++++++-------- 1 file changed, 53 insertions(+), 40 deletions(-) diff --git a/packages/analytics-connector/src/util/equals.ts b/packages/analytics-connector/src/util/equals.ts index c8069f4a..699be35f 100644 --- a/packages/analytics-connector/src/util/equals.ts +++ b/packages/analytics-connector/src/util/equals.ts @@ -1,54 +1,67 @@ // eslint-disable-next-line @typescript-eslint/no-explicit-any -export const isEqual = (obj1: any, obj2: any): boolean => { - const primitive = ['string', 'number', 'boolean', 'undefined']; - const typeA = typeof obj1; - const typeB = typeof obj2; - if (typeA !== typeB) { - return false; - } - for (const p of primitive) { - if (p === typeA) { - return obj1 === obj2; - } - } - // check null - if (obj1 == null && obj2 == null) { +export const isEqual = (value1: any, value2: any, seen = new WeakSet()) => { + // 1. Strict equality check for primitives and same object references + if (value1 === value2) { return true; - } else if (obj1 == null || obj2 == null) { - return false; } - // if got here - objects - if (obj1.length !== obj2.length) { + + // 2. Handle null and undefined (already covered by === if both are null/undefined) + // If one is null/undefined and the other is not, they are not equal. + if (value1 == null || value2 == null) { return false; } - //check if arrays - const isArrayA = Array.isArray(obj1); - const isArrayB = Array.isArray(obj2); - if (isArrayA !== isArrayB) { + + // 3. Handle different types + if (typeof value1 !== typeof value2) { return false; } - if (isArrayA && isArrayB) { - //arrays - for (let i = 0; i < obj1.length; i++) { - if (!isEqual(obj1[i], obj2[i])) { + + // 4. Handle specific object types (Date, RegExp) + if (value1 instanceof Date && value2 instanceof Date) { + return value1.getTime() === value2.getTime(); + } + if (value1 instanceof RegExp && value2 instanceof RegExp) { + return value1.source === value2.source && value1.flags === value2.flags; + } + + // 5. Handle objects and arrays (deep comparison) + if (typeof value1 === 'object' && typeof value2 === 'object') { + // Prevent infinite recursion with circular references + if (seen.has(value1) && seen.has(value2)) { + return true; // Already processed and found to be equal + } + seen.add(value1); + seen.add(value2); + + // Compare arrays + if (Array.isArray(value1) && Array.isArray(value2)) { + if (value1.length !== value2.length) { return false; } + for (let i = 0; i < value1.length; i++) { + if (!isEqual(value1[i], value2[i], seen)) { + return false; + } + } + return true; } - } else { - //objects - const sorted1 = Object.keys(obj1).sort(); - const sorted2 = Object.keys(obj2).sort(); - if (!isEqual(sorted1, sorted2)) { + + // Compare plain objects + const keys1 = Object.keys(value1); + const keys2 = Object.keys(value2); + + if (keys1.length !== keys2.length) { return false; } - //compare object values - let result = true; - Object.keys(obj1).forEach((key) => { - if (!isEqual(obj1[key], obj2[key])) { - result = false; + + for (const key of keys1) { + if (!keys2.includes(key) || !isEqual(value1[key], value2[key], seen)) { + return false; } - }); - return result; + } + return true; } - return true; -}; + + // 6. Default case: values are not equal + return false; +} \ No newline at end of file