From 18cef678b91e0fd93b7154ff59360325df0957c5 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 8 Oct 2026 04:50:29 -0700 Subject: [PATCH 01/25] =?UTF-8?q?=E2=9C=A8=20add=20remote=20configuration?= =?UTF-8?q?=20for=20the=20session=20sampling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The main process asks GET /api/v2/rum/config at init and at every new session (remoteConfigurationEnabled, off by default), keeps the last accepted answer on disk for the next launch's first draw, and sends its ETag back as If-None-Match. The ETag of another SDK version is not sent, so a 304 cannot keep a knob that version did not read. Sessions read sessionSampleRate and sessionOnError at each draw, and record the configuration version they were drawn under, reported as _dd.configuration.rc_version. A configuration published with activation: immediate ends the running session only where the new sampling is decisive: a collected session at rate 0, unless it is an on-error session and the switch stays on; a session drawn at rate 0 when the rate leaves 0 or the switch turns on. Renderer events take the main process's version. The application's custom values are exposed as getRemoteConfig(). --- src/assembly/Assembly.spec.ts | 33 ++ src/assembly/Assembly.ts | 13 +- src/config.spec.ts | 27 + src/config.ts | 25 +- src/domain/RemoteConfiguration.spec.ts | 584 ++++++++++++++++++++++ src/domain/RemoteConfiguration.ts | 404 +++++++++++++++ src/domain/session/SessionContext.spec.ts | 41 ++ src/domain/session/SessionContext.ts | 24 +- src/domain/session/SessionManager.spec.ts | 296 +++++++++-- src/domain/session/SessionManager.ts | 76 ++- src/domain/session/index.ts | 8 +- src/index.ts | 23 +- src/mocks.specUtil.ts | 1 + 13 files changed, 1502 insertions(+), 53 deletions(-) create mode 100644 src/domain/RemoteConfiguration.spec.ts create mode 100644 src/domain/RemoteConfiguration.ts diff --git a/src/assembly/Assembly.spec.ts b/src/assembly/Assembly.spec.ts index 7a5019ed..3cf080c5 100644 --- a/src/assembly/Assembly.spec.ts +++ b/src/assembly/Assembly.spec.ts @@ -182,6 +182,39 @@ describe('Assembly — renderer events', () => { expect(data._dd.configuration).toEqual({ session_sample_rate: 0, session_replay_sample_rate: 20 }); }); + it.each([ + { title: "the main process's", main: 4, expected: 4 }, + { title: 'none, when the main process drew without one', main: undefined, expected: undefined }, + ])( + "should report $title remote configuration version on renderer events, never the renderer's", + ({ main, expected }) => { + const { eventManager, hooks } = setup(); + hooks.registerRum(() => ({ _dd: { configuration: { session_sample_rate: 100, rc_version: main } } })); + const collected: ServerEvent[] = []; + eventManager.registerHandler({ + canHandle: (event): event is ServerEvent => event.kind === EventKind.SERVER, + handle: (event) => collected.push(event), + }); + + eventManager.notify({ + kind: EventKind.RAW, + source: EventSource.RENDERER, + format: EventFormat.RUM, + data: { + type: 'view', + source: 'browser', + view: { id: 'renderer-view-id' }, + session: { id: 'renderer-session-id', type: 'user' }, + application: { id: 'renderer-app-id' }, + _dd: { configuration: { session_sample_rate: 100, rc_version: 9 } }, + }, + } as unknown as RawRumEvent); + + const configuration = (collected[0].data as RumEvent)._dd.configuration as Record; + expect(configuration.rc_version).toBe(expected); + } + ); + it("should replace a renderer's own sampling marker with the main process's, absent included", () => { const { eventManager } = setup(); const collected: ServerEvent[] = []; diff --git a/src/assembly/Assembly.ts b/src/assembly/Assembly.ts index 9b429489..aa2fde77 100644 --- a/src/assembly/Assembly.ts +++ b/src/assembly/Assembly.ts @@ -52,9 +52,9 @@ export class Assembly { /** * Renderer RUM events arrive already assembled by `@flashcatcloud/browser-rum`. * Only `session.id`, `application.id` and the session's sampling attributes - * (`session.sampled_for_error`, `_dd.configuration.session_sample_rate`) are - * overridden from the main process hooks, preserving the renderer's own view, - * source, and other attributes. + * (`session.sampled_for_error`, `_dd.configuration.session_sample_rate` and + * `_dd.configuration.rc_version`) are overridden from the main process hooks, preserving the + * renderer's own view, source, and other attributes. */ private assembleRendererRumEvent(event: RawRumEvent): ServerEvent | DISCARDED { const hookResult = this.hooks.triggerRum({ @@ -82,6 +82,13 @@ export class Assembly { // Assigned rather than merged: `combine` keeps the renderer's value where the main process has // none, and the marker is the main process's to set or clear. data.session.sampled_for_error = session?.sampled_for_error; + // Same for the remote configuration version: the main process draws the session, so a version + // the renderer's own SDK reported — it reads no configuration under the bridge, but it could — + // must not survive next to the main process's draw. + const configuration = data._dd.configuration as { rc_version?: unknown } | undefined; + if (configuration) { + configuration.rc_version = _dd?.configuration?.rc_version; + } return { kind: EventKind.SERVER, diff --git a/src/config.spec.ts b/src/config.spec.ts index 6c5d0857..6c638095 100644 --- a/src/config.spec.ts +++ b/src/config.spec.ts @@ -505,5 +505,32 @@ describe('buildConfiguration', () => { expect(display.displayWarn).toHaveBeenCalledWith(expect.stringContaining('does not affect new sessions')); }); + + it('does not warn with remote configuration, which may lower the rate', () => { + buildConfiguration({ ...DEFAULT_CONFIG, sessionOnError: true, remoteConfigurationEnabled: true }); + + expect(display.displayWarn).not.toHaveBeenCalled(); + }); + }); + + describe('remoteConfigurationEnabled validation', () => { + it('defaults to false', () => { + expect(buildConfiguration({ ...DEFAULT_CONFIG })?.remoteConfigurationEnabled).toBe(false); + }); + + it('accepts true', () => { + expect( + buildConfiguration({ ...DEFAULT_CONFIG, remoteConfigurationEnabled: true })?.remoteConfigurationEnabled + ).toBe(true); + }); + + it('logs an error and keeps it off when not a boolean', () => { + const config = { ...DEFAULT_CONFIG, remoteConfigurationEnabled: 'yes' } as unknown as InitConfiguration; + + expect(buildConfiguration(config)?.remoteConfigurationEnabled).toBe(false); + expect(display.displayError).toHaveBeenCalledWith( + "Configuration error: 'remoteConfigurationEnabled' must be a boolean" + ); + }); }); }); diff --git a/src/config.ts b/src/config.ts index 206aa07b..6b79ffd4 100644 --- a/src/config.ts +++ b/src/config.ts @@ -58,6 +58,21 @@ export interface InitConfiguration { * of 100 there is nothing left for it to apply to. */ sessionOnError?: boolean; + /** + * Let the console change `sessionSampleRate` and `sessionOnError` without a new release of the + * application, and deliver the application's own `custom` values (see `getRemoteConfig()`). + * Defaults to `false`: nothing is requested and the init values apply. + * + * The main process asks for the configuration at init and whenever a new session starts, and + * keeps the last good answer on disk, so the next launch draws its first session with it before + * the network answers. A delivered value takes precedence over the init value; a value the + * console did not set leaves the init value in place. A change applies to the next session, + * unless the console asks for it to apply at once — see the README, Remote configuration. + * + * Set it here, in the main process, and not in the renderers' browser SDK: the main process owns + * the sessions and their sampling, and a renderer under the bridge ignores its own. + */ + remoteConfigurationEnabled?: boolean; telemetrySampleRate?: number; batchSize?: BatchSize; uploadFrequency?: UploadFrequency; @@ -114,6 +129,7 @@ export interface Configuration { proxy?: string; sessionSampleRate: number; sessionOnError: boolean; + remoteConfigurationEnabled: boolean; telemetrySampleRate: number; batchSize?: BatchSize; uploadFrequency?: UploadFrequency; @@ -245,7 +261,13 @@ export function buildConfiguration(initConfig: InitConfiguration): Configuration const proxy = validateOptionalString(initConfig.proxy); const sessionSampleRate = initConfig.sessionSampleRate ?? 100; const sessionOnError = validateOptionalBoolean(initConfig.sessionOnError, 'sessionOnError', false); - if (sessionOnError && sessionSampleRate === 100) { + const remoteConfigurationEnabled = validateOptionalBoolean( + initConfig.remoteConfigurationEnabled, + 'remoteConfigurationEnabled', + false + ); + // Not with remote configuration: the console may lower the rate the switch then applies to. + if (sessionOnError && sessionSampleRate === 100 && !remoteConfigurationEnabled) { displayWarn( 'sessionOnError does not affect new sessions at sessionSampleRate 100. Resumed sessions retain their previous sampling decision.' ); @@ -261,6 +283,7 @@ export function buildConfiguration(initConfig: InitConfiguration): Configuration proxy, sessionSampleRate, sessionOnError, + remoteConfigurationEnabled, telemetrySampleRate: validateTelemetrySampleRate(initConfig.telemetrySampleRate), defaultPrivacyLevel: validateDefaultPrivacyLevel(initConfig.defaultPrivacyLevel), allowedWebViewHosts: validateAllowedWebViewHosts(initConfig.allowedWebViewHosts), diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts new file mode 100644 index 00000000..928bb47a --- /dev/null +++ b/src/domain/RemoteConfiguration.spec.ts @@ -0,0 +1,584 @@ +import { mockFs, createTestConfiguration } from '../mocks.specUtil'; +vi.mock('node:fs/promises'); +const { writeFileSync, renameSync, unlinkSync, readdirSync } = vi.hoisted(() => ({ + writeFileSync: vi.fn(), + renameSync: vi.fn(), + unlinkSync: vi.fn(), + readdirSync: vi.fn(() => []), +})); +vi.mock('node:fs', () => ({ writeFileSync, renameSync, unlinkSync, readdirSync })); +vi.mock('electron', () => ({ + app: { getPath: vi.fn(() => '/mock/user/data') }, +})); +vi.mock('../tools/display', () => ({ displayError: vi.fn(), displayWarn: vi.fn() })); + +import { describe, it, expect, vi, beforeEach, afterEach, type Mock } from 'vitest'; +import type { Configuration } from '../config'; +import { EventKind, EventManager, LifecycleKind } from '../event'; +import { displayWarn } from '../tools/display'; +import { + REMOTE_CONFIGURATION_FILE_NAME, + REQUEST_TIMEOUT, + RETRY_DELAYS, + RemoteConfiguration, +} from './RemoteConfiguration'; + +const mfs = mockFs(); +const FILE_PATH = `/mock/user/data/${REMOTE_CONFIGURATION_FILE_NAME}`; + +/** A response the way the server writes it: every envelope field present. */ +function configurationBody(overrides: Record = {}) { + return { + schema_version: 1, + version: 3, + ttl: 600, + enabled: true, + activation: 'next_session', + refresh_on_foreground: false, + rum: { sessionSampleRate: 25, sessionOnError: true }, + ...overrides, + }; +} + +function ok(body: unknown, etag = '"tag-3"') { + return new Response(typeof body === 'string' ? body : JSON.stringify(body), { + status: 200, + headers: { ETag: etag, 'Content-Type': 'application/json' }, + }); +} + +/** What the SDK would have written for this configuration and identity. */ +function storedFile(overrides: Record = {}) { + return JSON.stringify({ + format: 1, + sdkVersion: 'test', + identity: JSON.stringify(['http://proxy.test', 'test-app-id', 'prod', '1.2.3']), + version: 2, + values: { sessionSampleRate: 0, sessionOnError: true }, + custom: { flag: 'cached' }, + etag: '"tag-2"', + ...overrides, + }); +} + +describe('RemoteConfiguration', () => { + let fetchMock: Mock; + let eventManager: EventManager; + let config: Configuration; + let remote: RemoteConfiguration | undefined; + let onImmediateChange: Mock<() => void>; + + beforeEach(() => { + vi.useFakeTimers(); + fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + mfs.readFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + mfs.writeFile.mockResolvedValue(undefined); + eventManager = new EventManager(); + onImmediateChange = vi.fn(); + config = createTestConfiguration({ + proxy: 'http://proxy.test', + clientToken: 'pub-token', + applicationId: 'test-app-id', + env: 'prod', + version: '1.2.3', + sessionSampleRate: 100, + sessionOnError: false, + remoteConfigurationEnabled: true, + }); + }); + + afterEach(() => { + remote?.stop(); + remote = undefined; + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.clearAllMocks(); + vi.restoreAllMocks(); + mfs.reset(); + }); + + async function start(): Promise { + remote = await RemoteConfiguration.init(config); + remote.start(eventManager, onImmediateChange); + await vi.advanceTimersByTimeAsync(0); + return remote; + } + + function writtenFiles(): Record[] { + return mfs.writeFile.mock.calls + .filter(([filePath]) => (filePath as string).startsWith(`${FILE_PATH}.`)) + .map(([, content]) => JSON.parse(content as string) as Record); + } + + function requestedUrl(call = 0): URL { + const outer = new URL(fetchMock.mock.calls[call][0] as string); + return new URL(outer.searchParams.get('ddforward')!, 'http://inner.test'); + } + + function requestHeaders(call = 0): Record { + return (fetchMock.mock.calls[call][1]?.headers ?? {}) as Record; + } + + describe('when disabled', () => { + it('requests nothing, reads nothing, and answers with the init values', async () => { + config = { ...config, remoteConfigurationEnabled: false }; + mfs.readFile.mockResolvedValue(storedFile()); + + const configuration = await start(); + + expect(fetchMock).not.toHaveBeenCalled(); + expect(mfs.readFile).not.toHaveBeenCalled(); + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(configuration.getCustom()).toBeUndefined(); + }); + }); + + describe('request', () => { + it('asks the config endpoint through the proxy with the client identity', async () => { + fetchMock.mockResolvedValue(ok(configurationBody())); + + await start(); + + const outer = new URL(fetchMock.mock.calls[0][0] as string); + expect(outer.origin).toBe('http://proxy.test'); + const inner = requestedUrl(); + expect(inner.pathname).toBe('/api/v2/rum/config'); + expect(Object.fromEntries(inner.searchParams)).toEqual({ + client_token: 'pub-token', + sdk: 'electron', + sdk_version: 'test', + env: 'prod', + app_version: '1.2.3', + }); + }); + + it('asks the site directly without a proxy, and leaves out what is not configured', async () => { + config = { ...config, proxy: undefined, site: 'intake.example.test', env: undefined, version: undefined }; + fetchMock.mockResolvedValue(ok(configurationBody())); + + await start(); + + const url = new URL(fetchMock.mock.calls[0][0] as string); + expect(`${url.origin}${url.pathname}`).toBe('https://intake.example.test/api/v2/rum/config'); + expect([...url.searchParams.keys()]).toEqual(['client_token', 'sdk', 'sdk_version']); + }); + + it('reports the version it holds, and sends its ETag', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockResolvedValue(new Response(null, { status: 304 })); + + await start(); + + expect(requestedUrl().searchParams.get('applied_version')).toBe('2'); + expect(requestHeaders()).toEqual({ 'If-None-Match': '"tag-2"' }); + }); + + it('does not block: start returns while the request is still pending', async () => { + fetchMock.mockReturnValue(new Promise(() => undefined)); + remote = await RemoteConfiguration.init(config); + + remote.start(eventManager, onImmediateChange); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(remote.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + }); + + it('asks again whenever a new session starts', async () => { + fetchMock.mockResolvedValue(ok(configurationBody())); + await start(); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(requestedUrl(1).searchParams.get('applied_version')).toBe('3'); + }); + + it('does not start a second request while one is in flight', async () => { + fetchMock.mockReturnValue(new Promise(() => undefined)); + await start(); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + }); + + describe('parsing and validation', () => { + it('applies the delivered rate and switch over the init values, with their version', async () => { + fetchMock.mockResolvedValue(ok(configurationBody())); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 25, sessionOnError: true, rcVersion: 3 }); + }); + + it('keeps the init value of a knob the console did not set', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ rum: { sessionOnError: true } }))); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: true, rcVersion: 3 }); + }); + + it.each([ + ['absent', undefined], + ['null', null], + ])('reads a %s rum bag as empty', async (_, rum) => { + fetchMock.mockResolvedValue(ok(configurationBody({ rum }))); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false, rcVersion: 3 }); + }); + + it('drops the values and custom while the kill switch is off, and keeps the version', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ enabled: false, custom: { flag: true } }))); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false, rcVersion: 3 }); + expect(configuration.getCustom()).toBeUndefined(); + }); + + it.each([ + ['a rate above 100', { sessionSampleRate: 150 }], + ['a negative rate', { sessionSampleRate: -1 }], + ['a rate as a string', { sessionSampleRate: '50' }], + ['a switch as a string', { sessionOnError: 'true' }], + ['a switch as a number', { sessionOnError: 1 }], + ])('drops %s and keeps the init value', async (_, rum) => { + fetchMock.mockResolvedValue(ok(configurationBody({ rum }))); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false, rcVersion: 3 }); + }); + + it('hands the custom bag over as a copy', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ custom: { allowList: ['a', 'b'], level: 2 } }))); + const configuration = await start(); + + const custom = configuration.getCustom()!; + expect(custom).toEqual({ allowList: ['a', 'b'], level: 2 }); + (custom.allowList as string[]).push('c'); + + expect(configuration.getCustom()).toEqual({ allowList: ['a', 'b'], level: 2 }); + }); + + it('drops a custom bag that is not an object', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ custom: ['a'] }))); + + const configuration = await start(); + + expect(configuration.getCustom()).toBeUndefined(); + expect(configuration.getSampling().rcVersion).toBe(3); + }); + + it.each([ + ['a body that is not JSON', 'captive portal'], + ['a JSON array', '[]'], + ['an unstamped body', JSON.stringify({ version: 4, enabled: true })], + ['a negative version', JSON.stringify(configurationBody({ version: -1 }))], + ['a fractional version', JSON.stringify(configurationBody({ version: 1.5 }))], + ['a version as a string', JSON.stringify(configurationBody({ version: '4' }))], + ['no kill switch', JSON.stringify(configurationBody({ enabled: 'yes' }))], + ['a rum bag that is not an object', JSON.stringify(configurationBody({ rum: [] }))], + ])('refuses %s, keeps what it holds, and retries', async (_, body) => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockResolvedValue(ok(body)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(writtenFiles()).toEqual([]); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('refuses a schema it does not know without retrying, and says so', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ schema_version: 2 }))); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(displayWarn).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); + expect(fetchMock).toHaveBeenCalledTimes(1); + + // Asked again at the next session, answered the same way: not said again. + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(displayWarn).toHaveBeenCalledTimes(1); + }); + + it('ignores an older version arriving late', async () => { + mfs.readFile.mockResolvedValue(storedFile({ version: 5 })); + fetchMock.mockResolvedValue(ok(configurationBody({ version: 4, rum: { sessionSampleRate: 100 } }))); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 5 }); + expect(writtenFiles()).toEqual([]); + }); + }); + + describe('cache on disk', () => { + it('writes the configuration with its ETag, the SDK version and who it was for', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ custom: { flag: 1 } }))); + + await start(); + + expect(writtenFiles()).toEqual([ + { + format: 1, + sdkVersion: 'test', + identity: JSON.stringify(['http://proxy.test', 'test-app-id', 'prod', '1.2.3']), + version: 3, + values: { sessionSampleRate: 25, sessionOnError: true }, + custom: { flag: 1 }, + etag: '"tag-3"', + }, + ]); + expect(renameSync).toHaveBeenCalledWith(expect.stringMatching(/\.tmp$/), FILE_PATH); + }); + + it('applies what the previous launch kept before the network answers', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(configuration.getCustom()).toEqual({ flag: 'cached' }); + }); + + it.each([ + ['another env', { identity: JSON.stringify(['http://proxy.test', 'test-app-id', 'staging', '1.2.3']) }], + ['another app version', { identity: JSON.stringify(['http://proxy.test', 'test-app-id', 'prod', '1.2.2']) }], + ['another application', { identity: JSON.stringify(['http://proxy.test', 'other-app', 'prod', '1.2.3']) }], + ['another file format', { format: 2 }], + ['a rate out of range', { values: { sessionSampleRate: 200 } }], + ['a switch that is not a boolean', { values: { sessionOnError: 'true' } }], + ['no version', { version: undefined }], + ])('ignores a file written for %s, and asks without a version or an ETag', async (_, overrides) => { + mfs.readFile.mockResolvedValue(storedFile(overrides)); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(configuration.getCustom()).toBeUndefined(); + expect(requestedUrl().searchParams.has('applied_version')).toBe(false); + expect(requestHeaders()).toEqual({}); + }); + + it('ignores a file that is not JSON', async () => { + mfs.readFile.mockResolvedValue('{not json'); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + }); + + describe('304', () => { + it('keeps what it holds, and writes nothing', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockResolvedValue(new Response(null, { status: 304 })); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(writtenFiles()).toEqual([]); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('retries when it holds nothing a 304 could refer to', async () => { + fetchMock.mockResolvedValue(new Response(null, { status: 304 })); + + await start(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + }); + + describe('after an SDK upgrade', () => { + it('keeps the values another SDK version wrote, but does not send its ETag', async () => { + mfs.readFile.mockResolvedValue(storedFile({ sdkVersion: 'older' })); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(requestedUrl().searchParams.get('applied_version')).toBe('2'); + expect(requestHeaders()).toEqual({}); + }); + + it('replaces them with the full answer, read by this version, and rewrites the file as its own', async () => { + // The older SDK did not read the switch, so its file has none. + mfs.readFile.mockResolvedValue(storedFile({ sdkVersion: 'older', values: { sessionSampleRate: 0 } })); + fetchMock.mockResolvedValue( + ok(configurationBody({ version: 2, rum: { sessionSampleRate: 0, sessionOnError: true } })) + ); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(writtenFiles()).toEqual([expect.objectContaining({ sdkVersion: 'test', etag: '"tag-3"' })]); + // Its own ETag from now on. + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + expect(requestHeaders(1)).toEqual({ 'If-None-Match': '"tag-3"' }); + }); + }); + }); + + describe('offline and failures', () => { + it('keeps the init values when the network is down, retries twice, then waits for the next session', async () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + + const configuration = await start(); + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] - 1); + expect(fetchMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1]); + expect(fetchMock).toHaveBeenCalledTimes(3); + await vi.advanceTimersByTimeAsync(10 * RETRY_DELAYS[1]); + expect(fetchMock).toHaveBeenCalledTimes(3); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + + it('spreads the retries by ±20%', async () => { + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + // Drawn as each retry is scheduled: the lowest spread for the first, the highest for the second. + const random = vi.spyOn(Math, 'random').mockReturnValue(0); + await start(); + random.mockReturnValue(0.999); + + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 0.8); + expect(fetchMock).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 1.19); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 0.01); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it('keeps what it holds offline', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(writtenFiles()).toEqual([]); + }); + + it('gives up on a request that does not answer in time, and retries', async () => { + fetchMock.mockImplementation( + (_url, init) => + new Promise((_resolve, reject) => { + init!.signal!.addEventListener('abort', () => reject(new DOMException('aborted', 'AbortError'))); + }) + ); + await start(); + + await vi.advanceTimersByTimeAsync(REQUEST_TIMEOUT); + expect((fetchMock.mock.calls[0][1]!.signal as AbortSignal).aborted).toBe(true); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it.each([500, 503, 429])('retries on %s', async (status) => { + fetchMock.mockResolvedValue(new Response('', { status })); + + await start(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it.each([401, 403, 404])('does not retry on %s', async (status) => { + fetchMock.mockResolvedValue(new Response('', { status })); + + await start(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('stops: cancels the request in flight and any retry, and applies nothing', async () => { + let answer!: (response: Response) => void; + fetchMock.mockReturnValue(new Promise((resolve) => (answer = resolve))); + const configuration = await start(); + + configuration.stop(); + answer(ok(configurationBody())); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); + + expect((fetchMock.mock.calls[0][1]!.signal as AbortSignal).aborted).toBe(true); + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + }); + + describe('activation', () => { + it('tells the session owner about a new version that applies at once, once it is in force', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'immediate' }))); + let samplingWhenTold: unknown; + onImmediateChange.mockImplementation(() => (samplingWhenTold = remote!.getSampling())); + + await start(); + + expect(onImmediateChange).toHaveBeenCalledTimes(1); + expect(samplingWhenTold).toEqual({ sessionSampleRate: 25, sessionOnError: true, rcVersion: 3 }); + }); + + it('does not tell it about a change for the next session', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'next_session' }))); + + const configuration = await start(); + + expect(configuration.getSampling().sessionSampleRate).toBe(25); + expect(onImmediateChange).not.toHaveBeenCalled(); + }); + + it('reads an absent or unknown activation as the next session', async () => { + fetchMock.mockResolvedValue(ok(configurationBody({ activation: undefined }))); + + await start(); + + expect(onImmediateChange).not.toHaveBeenCalled(); + }); + + it('does not tell it again about the version it already holds', async () => { + mfs.readFile.mockResolvedValue(storedFile({ version: 3, sdkVersion: 'older' })); + fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'immediate' }))); + + await start(); + + expect(onImmediateChange).not.toHaveBeenCalled(); + }); + + it('tells it about a higher version', async () => { + mfs.readFile.mockResolvedValue(storedFile({ version: 2 })); + fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'immediate' }))); + + await start(); + + expect(onImmediateChange).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts new file mode 100644 index 00000000..1203ea28 --- /dev/null +++ b/src/domain/RemoteConfiguration.ts @@ -0,0 +1,404 @@ +import { app } from 'electron'; +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { deepClone, ONE_SECOND, type Subscription } from '@flashcatcloud/browser-core'; +import type { Configuration } from '../config'; +import { EventKind, type EventManager, LifecycleKind, type SessionRenewEvent } from '../event'; +import { displayWarn } from '../tools/display'; +import { StateFile } from '../tools/StateFile'; +import { isConfigurationVersion, type SamplingConfiguration } from './session'; +import { addError, setTimeout } from './telemetry'; + +export const REMOTE_CONFIGURATION_FILE_NAME = '_fc_remote_config'; +const CONFIG_PATH = '/api/v2/rum/config'; +/** + * The response shape this SDK can read. The server stamps every response with it, and a value this + * build does not recognise means the payload changed in a way it could misread — so the whole + * response is discarded and the settings already in force are kept. + */ +const SUPPORTED_SCHEMA_VERSION = 1; +/** The shape of the file written here, not the SDK version: only a change of shape orphans it. */ +const FILE_FORMAT = 1; +export const REQUEST_TIMEOUT = 10 * ONE_SECOND; +/** + * A failed request is retried quickly, then patiently, then not until the next natural trigger (a + * new session, or the next launch). Two extra requests per outage, so a fleet can never turn an + * endpoint incident into a storm. + */ +export const RETRY_DELAYS = [5 * ONE_SECOND, 60 * ONE_SECOND]; + +/** What the console asks a running client to do about the session it already has. */ +export const Activation = { + /** Leave it as it is: the new values apply from the next session. The server's default. */ + NEXT_SESSION: 'next_session', + /** End it where the new values are decisive for it, see `SessionManager.applySamplingChange`. */ + IMMEDIATE: 'immediate', +} as const; + +/** The knobs this SDK reads. One the console did not set is absent, and the init value applies. */ +interface RemoteValues { + sessionSampleRate?: number; + sessionOnError?: boolean; +} + +/** The last configuration the server delivered and this SDK accepted. */ +interface Delivered { + /** A publish counter that only goes up — a rollback is republished under a new number. */ + version: number; + values: RemoteValues; + /** The application's own pass-through bag, handed to it verbatim by `getRemoteConfig()`. */ + custom?: Record; +} + +interface StoredConfiguration extends Delivered { + format: typeof FILE_FORMAT; + /** + * The SDK that wrote the file. Another version may have read the same response differently — a + * knob it did not know is missing from its values — and its ETag would keep that knob missing + * through every 304. So the ETag of another version is not sent; its values still apply until + * the full response replaces them, which keeps the first session after an upgrade on the + * console's values rather than back on the init ones. + */ + sdkVersion: string; + /** Who the answer was for: it only applies to the same endpoint, application, env and version. */ + identity: string; + etag?: string; +} + +type RequestOutcome = 'done' | 'retry'; + +/** + * The console's settings for this application, kept fresh and kept on disk. + * + * Sessions read the sampling from {@link getSampling} at each draw and never in between, so a + * configuration arriving mid-session changes the next session, not this one. When the console asks + * for a change to apply at once (`activation: immediate`), the owner of the sessions is told and + * decides whether the running session has to end — see `SessionManager.applySamplingChange`. + * + * Fetching follows the sessions' rhythm, as in the other FlashCat SDKs: once at init and once + * whenever a new session starts — a change can only matter at a draw, and every draw is a new + * session. There is no timer between sessions; the server's `ttl` and `refresh_on_foreground` are + * read by no one here. Nothing waits on the network: the last good answer is read from disk at init, + * so the first session of a launch draws with it, and a request that fails, times out, or answers + * something unreadable leaves the settings in force exactly as they were. + * + * The server pairs `Cache-Control: no-cache` with an `ETag`. Node's `fetch` has no HTTP cache to + * revalidate with, so the ETag is kept here with the values it describes and sent back as + * `If-None-Match`; a 304 then keeps what is held. + * + * Off unless `remoteConfigurationEnabled`: no request, no file, and the init values apply. + */ +export class RemoteConfiguration { + private readonly stateFile = new StateFile(getFilePath(), 'remote configuration'); + private readonly identity: string; + private delivered: Delivered | undefined; + private etag: string | undefined; + private inFlight: AbortController | undefined; + private retryTimeoutId: ReturnType | undefined; + private failedAttempts = 0; + private renewSubscription: Subscription | undefined; + private onImmediateChange: () => void = () => undefined; + /** Said once per launch: the server keeps answering the same way at every new session. */ + private warnedUnsupportedSchema = false; + private stopped = false; + + private constructor(private readonly config: Configuration) { + this.identity = JSON.stringify([config.proxy ?? config.site, config.applicationId, config.env, config.version]); + } + + /** Reads the configuration the previous launch kept, if remote configuration is on. */ + static async init(config: Configuration): Promise { + const remoteConfiguration = new RemoteConfiguration(config); + if (config.remoteConfigurationEnabled) { + await remoteConfiguration.restore(); + } + return remoteConfiguration; + } + + /** + * The sampling a draw made now would use: the console's value where it set one, the init value + * where it did not, and the version they came from once a configuration has been delivered. + */ + getSampling(): SamplingConfiguration { + const values = this.delivered?.values ?? {}; + return { + sessionSampleRate: values.sessionSampleRate ?? this.config.sessionSampleRate, + sessionOnError: values.sessionOnError ?? this.config.sessionOnError, + ...(this.delivered ? { rcVersion: this.delivered.version } : {}), + }; + } + + /** A copy of the application's `custom` values, or `undefined` when none were delivered. */ + getCustom(): Record | undefined { + return this.delivered?.custom && deepClone(this.delivered.custom); + } + + /** + * Fetches now and at every new session. `onImmediateChange` is called when a newly published + * configuration asks to apply at once, after it is in force. + */ + start(eventManager: EventManager, onImmediateChange: () => void): void { + if (!this.config.remoteConfigurationEnabled) { + return; + } + this.onImmediateChange = onImmediateChange; + this.renewSubscription = eventManager.registerHandler({ + canHandle: (event): event is SessionRenewEvent => + event.kind === EventKind.LIFECYCLE && event.lifecycle === LifecycleKind.SESSION_RENEW, + handle: () => this.trigger(), + }); + this.trigger(); + } + + stop(): void { + this.stopped = true; + this.inFlight?.abort(); + this.inFlight = undefined; + clearTimeout(this.retryTimeoutId); + this.renewSubscription?.unsubscribe(); + } + + private async restore(): Promise { + this.stateFile.sweep(); + let stored: StoredConfiguration | undefined; + try { + stored = parseStoredConfiguration(JSON.parse(await fs.readFile(getFilePath(), 'utf-8'))); + } catch { + // No file yet, or one that is not ours to read: the init values apply until the server answers. + return; + } + if (stored === undefined || stored.identity !== this.identity) { + return; + } + const { version, values, custom } = stored; + this.delivered = { version, values, ...(custom ? { custom } : {}) }; + this.etag = stored.sdkVersion === __SDK_VERSION__ ? stored.etag : undefined; + } + + /** A natural trigger re-arms the whole backoff: a session starting mid-outage asks again at once. */ + private trigger(): void { + clearTimeout(this.retryTimeoutId); + this.retryTimeoutId = undefined; + this.failedAttempts = 0; + this.fetchNow(); + } + + private fetchNow(): void { + if (this.stopped || this.inFlight) { + return; + } + const controller = new AbortController(); + this.inFlight = controller; + const timeoutId = setTimeout(() => controller.abort(), REQUEST_TIMEOUT); + void this.request(controller.signal) + .catch((error: unknown): RequestOutcome => { + // Not a network failure — those are outcomes — but a bug: reported, and not retried. + addError(error); + return 'done'; + }) + .then((outcome) => { + clearTimeout(timeoutId); + if (this.inFlight !== controller) { + return; + } + this.inFlight = undefined; + if (outcome === 'retry') { + this.scheduleRetry(); + } + }); + } + + private scheduleRetry(): void { + if (this.failedAttempts >= RETRY_DELAYS.length) { + return; + } + // ±20%: an endpoint incident aligns every failed client's retry clock, and recovery would + // otherwise be greeted by the whole fleet at once. + const delay = RETRY_DELAYS[this.failedAttempts] * (0.8 + 0.4 * Math.random()); + this.failedAttempts += 1; + this.retryTimeoutId = setTimeout(() => { + this.retryTimeoutId = undefined; + this.fetchNow(); + }, delay); + } + + private async request(signal: AbortSignal): Promise { + let response: Response; + let body: string; + try { + response = await fetch(this.buildUrl(), { + headers: this.etag ? { 'If-None-Match': this.etag } : {}, + signal, + }); + body = await response.text(); + } catch { + // Offline, refused, timed out, or stopped. + return 'retry'; + } + if (signal.aborted) { + return 'done'; + } + if (response.status === 304) { + // Only ever asked with an ETag, which only ever sits next to the values it describes. + return this.delivered ? 'done' : 'retry'; + } + if (response.status !== 200) { + return response.status === 429 || response.status >= 500 ? 'retry' : 'done'; + } + const parsed = parseResponse(body); + if (parsed === 'unsupported') { + if (!this.warnedUnsupportedSchema) { + this.warnedUnsupportedSchema = true; + displayWarn('Remote configuration ignored: the server answered with a schema this SDK cannot read.'); + } + return 'done'; + } + if (parsed === undefined) { + // A 200 is not proof the body came from the configuration endpoint: a captive portal or a + // misrouted proxy answers 200 too. Storing that would blank the console's values. + return 'retry'; + } + this.apply(parsed, response.headers.get('etag') ?? undefined); + return 'done'; + } + + private apply({ activation, ...delivered }: ParsedResponse, etag: string | undefined): void { + const heldVersion = this.delivered?.version; + // Settings only ever change under a higher number, so a lower one is an older answer arriving + // late; applying it would put this client back on settings the console has already replaced. + if (heldVersion !== undefined && delivered.version < heldVersion) { + return; + } + this.delivered = delivered; + this.etag = etag; + const stored: StoredConfiguration = { + format: FILE_FORMAT, + sdkVersion: __SDK_VERSION__, + identity: this.identity, + ...delivered, + ...(etag === undefined ? {} : { etag }), + }; + void this.stateFile.write(() => JSON.stringify(stored)); + + // A repeat of the version already held is the ordinary answer, and it brings nothing new: the + // running session must hear about published changes only, or it would be re-judged at every + // renewal for nothing. + const isNew = heldVersion === undefined || delivered.version > heldVersion; + if (isNew && activation === Activation.IMMEDIATE) { + this.onImmediateChange(); + } + } + + private buildUrl(): string { + const { clientToken, env, version, proxy, site } = this.config; + const parameters = [ + `client_token=${encodeURIComponent(clientToken)}`, + 'sdk=electron', + `sdk_version=${encodeURIComponent(__SDK_VERSION__)}`, + ]; + if (env) { + parameters.push(`env=${encodeURIComponent(env)}`); + } + if (version) { + parameters.push(`app_version=${encodeURIComponent(version)}`); + } + // What lets the console tell how far a change has reached. 0 is a version like any other. + if (this.delivered) { + parameters.push(`applied_version=${this.delivered.version}`); + } + // Behind a proxy, everything the intake has to see travels inside `ddforward`. + const pathAndQuery = `${CONFIG_PATH}?${parameters.join('&')}`; + return proxy ? `${proxy}?ddforward=${encodeURIComponent(pathAndQuery)}` : `https://${site}${pathAndQuery}`; + } +} + +interface ParsedResponse extends Delivered { + activation: string; +} + +/** + * The configuration `text` holds, `'unsupported'` for a schema this SDK does not know, or + * `undefined` for anything that is not recognisably a configuration response. + * + * Only the envelope decides that. A knob holding something that is not a rate or a switch is + * dropped — it reads as "not delivered", and the init value stays — rather than refusing the + * response, so one bad value cannot switch every other knob back off. The values and `custom` are + * withheld while the kill switch (`enabled`) is off; the version is kept either way. + */ +function parseResponse(text: string): ParsedResponse | 'unsupported' | undefined { + let body: unknown; + try { + body = JSON.parse(text); + } catch { + return undefined; + } + if (!isBag(body)) { + return undefined; + } + if (body.schema_version !== SUPPORTED_SCHEMA_VERSION) { + return typeof body.schema_version === 'number' ? 'unsupported' : undefined; + } + const { version, enabled, rum, custom, activation } = body; + if (!isConfigurationVersion(version) || typeof enabled !== 'boolean') { + return undefined; + } + // Absent or null is an empty bag: a response with nothing set has no knobs to carry. + if (rum !== undefined && rum !== null && !isBag(rum)) { + return undefined; + } + const values: RemoteValues = {}; + if (enabled && isBag(rum)) { + if (isRate(rum.sessionSampleRate)) { + values.sessionSampleRate = rum.sessionSampleRate; + } + // A switch is a boolean or nothing: a "true" string or a 1 is dropped, not read as either. + if (typeof rum.sessionOnError === 'boolean') { + values.sessionOnError = rum.sessionOnError; + } + } + return { + version, + values, + ...(enabled && isBag(custom) ? { custom } : {}), + activation: typeof activation === 'string' ? activation : Activation.NEXT_SESSION, + }; +} + +/** + * The file is checked on the way out as strictly as a response on the way in: anything on disk can + * be edited by hand or left by another version. A file that does not hold a well-formed record is + * no configuration at all, and the init values apply until the server answers. + */ +function parseStoredConfiguration(value: unknown): StoredConfiguration | undefined { + if (!isBag(value) || value.format !== FILE_FORMAT) { + return undefined; + } + const { sdkVersion, identity, version, values, custom, etag } = value; + if (typeof sdkVersion !== 'string' || typeof identity !== 'string' || !isConfigurationVersion(version)) { + return undefined; + } + if (!isBag(values) || (custom !== undefined && !isBag(custom)) || (etag !== undefined && typeof etag !== 'string')) { + return undefined; + } + const { sessionSampleRate, sessionOnError } = values; + if (sessionSampleRate !== undefined && !isRate(sessionSampleRate)) { + return undefined; + } + if (sessionOnError !== undefined && typeof sessionOnError !== 'boolean') { + return undefined; + } + return value as unknown as StoredConfiguration; +} + +function isRate(value: unknown): value is number { + return typeof value === 'number' && value >= 0 && value <= 100; +} + +/** A keyed object, as opposed to an array or a primitive. */ +function isBag(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +function getFilePath(): string { + return path.join(app.getPath('userData'), REMOTE_CONFIGURATION_FILE_NAME); +} diff --git a/src/domain/session/SessionContext.spec.ts b/src/domain/session/SessionContext.spec.ts index 5152f795..69c20615 100644 --- a/src/domain/session/SessionContext.spec.ts +++ b/src/domain/session/SessionContext.spec.ts @@ -107,6 +107,47 @@ describe('SessionContext', () => { }); }); + describe('remote configuration version', () => { + it('reports the version a session was drawn under, and keeps reporting it after the session ended', async () => { + const hooks = createFormatHooks(); + const context = await SessionContext.init(hooks, EXPIRE_DELAY); + + context.add({ id: 'session-abc', trackingType: TrackingType.TRACKED, sampleRate: 20, rcVersion: 6 }); + vi.advanceTimersByTime(10); + context.close(); + + expect(hooks.triggerRum({ eventType: 'error', startTime: T0 })).toMatchObject({ + _dd: { configuration: { session_sample_rate: 20, rc_version: 6 } }, + }); + }); + + it.each([ + { title: 'a whole version', rcVersion: 2, kept: true }, + { title: 'a negative version', rcVersion: -1, kept: false }, + { title: 'a fractional version', rcVersion: 2.5, kept: false }, + { title: 'a version as a string', rcVersion: '2', kept: false }, + ])('restores a history entry with $title only if it is well formed', async ({ rcVersion, kept }) => { + mfs.readFile.mockResolvedValue( + JSON.stringify([ + { + startTime: 0, + endTime: 10, + value: { id: 'session-abc', trackingType: TrackingType.TRACKED, sampleRate: 100, rcVersion }, + }, + ]) + ); + const hooks = createFormatHooks(); + await SessionContext.init(hooks, EXPIRE_DELAY); + + const result = hooks.triggerRum({ eventType: 'error', startTime: T0 }); + if (kept) { + expect(result).toMatchObject({ session: { id: 'session-abc' }, _dd: { configuration: { rc_version: 2 } } }); + } else { + expect(result).toBe(DISCARDED); + } + }); + }); + describe('after close()', () => { it('RUM hook still attributes events during the session period (crash attribution)', async () => { const hooks = createFormatHooks(); diff --git a/src/domain/session/SessionContext.ts b/src/domain/session/SessionContext.ts index 1d527587..794bc46b 100644 --- a/src/domain/session/SessionContext.ts +++ b/src/domain/session/SessionContext.ts @@ -37,6 +37,13 @@ export interface SessionRecord { * the rate still stands for `100 / rate` sessions of the rate that drew it. */ sampleRate: number; + /** + * The version of the remote configuration the session was drawn under, when its draw read one. + * Reported on its events as `_dd.configuration.rc_version`, so the console can trace a session + * back to the settings that decided whether to keep it — a resumed session and a crash reported + * a launch later included. + */ + rcVersion?: number; /** * Set once a `TRACKED_ON_ERROR` session reports its first error, which releases what it withheld. * Never set on any other session. @@ -82,7 +89,9 @@ export class SessionContext { // A session kept only because it errored stands for itself, not for `100 / rate` sessions // like a drawn one: 0 is what the backend reads as "do not extrapolate". Derived from the // record, so a resumed session and a crash reported a launch later get it too. - _dd: { configuration: { session_sample_rate: sampledForError ? 0 : record.sampleRate } }, + _dd: { + configuration: { session_sample_rate: sampledForError ? 0 : record.sampleRate, rc_version: record.rcVersion }, + }, }; }); @@ -169,13 +178,22 @@ function toSessionRecord(value: unknown): SessionRecord | undefined { const TRACKING_TYPES = new Set(Object.values(TrackingType)); -/** The record `value` is, or `undefined` when it is not one: a non-empty id, a known type, a finite rate, a boolean mark. */ +/** + * The record `value` is, or `undefined` when it is not one: a non-empty id, a known type, a finite + * rate, a whole non-negative configuration version, a boolean mark. + */ export function parseSessionRecord(value: unknown): SessionRecord | undefined { if (typeof value !== 'object' || value === null) return undefined; - const { id, trackingType, sampleRate, hasError } = value as Record; + const { id, trackingType, sampleRate, rcVersion, hasError } = value as Record; if (typeof id !== 'string' || id === '') return undefined; if (!TRACKING_TYPES.has(trackingType)) return undefined; if (typeof sampleRate !== 'number' || !Number.isFinite(sampleRate)) return undefined; + if (rcVersion !== undefined && !isConfigurationVersion(rcVersion)) return undefined; if (hasError !== undefined && typeof hasError !== 'boolean') return undefined; return value as SessionRecord; } + +/** A remote configuration version: a publish counter, so a whole number from 0 up. */ +export function isConfigurationVersion(value: unknown): value is number { + return typeof value === 'number' && Number.isSafeInteger(value) && value >= 0; +} diff --git a/src/domain/session/SessionManager.spec.ts b/src/domain/session/SessionManager.spec.ts index c5b6d2e8..82b8bf2e 100644 --- a/src/domain/session/SessionManager.spec.ts +++ b/src/domain/session/SessionManager.spec.ts @@ -19,7 +19,12 @@ vi.mock('../../tools/display', () => ({ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { type TimeStamp } from '@flashcatcloud/browser-core'; -import { SessionManager, SESSION_EXPIRATION_DELAY, SESSION_FILE_NAME } from './SessionManager'; +import { + SessionManager, + SESSION_EXPIRATION_DELAY, + SESSION_FILE_NAME, + type SamplingConfiguration, +} from './SessionManager'; import { SESSION_TIME_OUT_DELAY } from './session.constants'; import { TrackingType } from './SessionContext'; @@ -64,7 +69,7 @@ describe('sessionManager', () => { it('creates new session when no file exists', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().id).toMatch(/^[0-9a-f-]+$/); expect(sessionManager.getSession().status).toBe('active'); @@ -88,7 +93,7 @@ describe('sessionManager', () => { mfs.readFile.mockResolvedValue(JSON.stringify(existingState)); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().id).toBe('existing-session-id'); expect(sessionManager.getSession().status).toBe('active'); @@ -104,7 +109,7 @@ describe('sessionManager', () => { mfs.readFile.mockResolvedValue(JSON.stringify(existingState)); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().id).not.toBe('expired-session-id'); expect(sessionManager.getSession().status).toBe('active'); @@ -120,7 +125,7 @@ describe('sessionManager', () => { mfs.readFile.mockResolvedValue(JSON.stringify(existingState)); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().id).not.toBe('timed-out-session-id'); expect(sessionManager.getSession().status).toBe('active'); @@ -138,7 +143,7 @@ describe('sessionManager', () => { .mockResolvedValueOnce(JSON.stringify(expiredState)) // _dd_s .mockResolvedValueOnce(JSON.stringify([{ startTime: 0, endTime: null, value: 'expired-session-id' }])); // _dd_session_history - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const newSessionId = sessionManager.getSession().id; expect(newSessionId).not.toBe('expired-session-id'); @@ -159,7 +164,7 @@ describe('sessionManager', () => { it('expires session after inactivity delay', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().status).toBe('active'); @@ -174,7 +179,7 @@ describe('sessionManager', () => { const now = Date.now(); mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const sessionId = sessionManager.getSession().id; @@ -208,7 +213,7 @@ describe('sessionManager', () => { const startTime = Date.now(); mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const sessionId = sessionManager.getSession().id; expect(sessionId).toBeDefined(); @@ -253,7 +258,7 @@ describe('sessionManager', () => { it('creates new session on activity when expired', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const originalSessionId = sessionManager.getSession().id; expect(sessionManager.getSession().status).toBe('active'); @@ -283,7 +288,7 @@ describe('sessionManager', () => { mfs.access.mockResolvedValue(undefined); mfs.readFile.mockRejectedValue(new Error('Read error')); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); // Should create a new session despite read error @@ -294,7 +299,7 @@ describe('sessionManager', () => { mockNoSessionFile(); mfs.writeFile.mockRejectedValue(new Error('Write error')); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); // Session should still be created in memory @@ -306,7 +311,7 @@ describe('sessionManager', () => { mfs.access.mockResolvedValue(undefined); mfs.readFile.mockResolvedValue('invalid json'); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); // Should create a new session despite parse error @@ -318,7 +323,7 @@ describe('sessionManager', () => { it('sets session status to expired and clears timers', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().status).toBe('active'); @@ -336,7 +341,7 @@ describe('sessionManager', () => { it('RUM hook returns session id immediately after start()', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const result = hooks.triggerRum({ eventType: 'view', startTime: T0 }); expect(result).toMatchObject({ session: { id: sessionManager.getSession().id } }); @@ -345,7 +350,7 @@ describe('sessionManager', () => { it('telemetry hook returns session id immediately after start()', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const result = hooks.triggerTelemetry({ startTime: T0 }); expect(result).toMatchObject({ session: { id: sessionManager.getSession().id } }); @@ -356,7 +361,7 @@ describe('sessionManager', () => { it('should not allow to mutate the current session', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const session = sessionManager.getSession(); session.id = 'new-id'; @@ -390,7 +395,7 @@ describe('sessionManager', () => { mockNoSessionFile(); vi.spyOn(Math, 'random').mockReturnValue(random); - sessionManager = await SessionManager.start(eventManager, hooks, { sessionSampleRate, sessionOnError }); + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ sessionSampleRate, sessionOnError })); expect(sessionManager.getSession()).toMatchObject({ trackingType: expected, sampleRate: sessionSampleRate }); expect(savedSessionStates()[0]).toMatchObject({ trackingType: expected, sampleRate: sessionSampleRate }); @@ -399,7 +404,7 @@ describe('sessionManager', () => { it('does not write back a state read before the session ended', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); const { id } = sessionManager.getSession(); let finishRead!: (content: string) => void; mfs.access.mockResolvedValue(undefined); @@ -437,7 +442,7 @@ describe('sessionManager', () => { ) ); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); await vi.advanceTimersByTimeAsync(0); const histories = mfs.writeFile.mock.calls @@ -466,7 +471,7 @@ describe('sessionManager', () => { }) ); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(hooks.triggerRum({ eventType: 'view', startTime: now as TimeStamp })).toMatchObject({ _dd: { configuration: { session_sample_rate: 10 } }, @@ -479,10 +484,10 @@ describe('sessionManager', () => { JSON.stringify({ id: 'existing', created: now, lastActivity: now, trackingType: TrackingType.TRACKED_ON_ERROR }) ); - sessionManager = await SessionManager.start(eventManager, hooks, { + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ sessionSampleRate: 100, sessionOnError: false, - }); + })); expect(sessionManager.getSession()).toMatchObject({ id: 'existing', @@ -507,7 +512,7 @@ describe('sessionManager', () => { }) ); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); expect(sessionManager.getSession().hasError).toBe(true); }); @@ -526,7 +531,10 @@ describe('sessionManager', () => { JSON.stringify({ id: 'existing', created: now, lastActivity: now, sampleRate: 100, ...malformed }) ); - sessionManager = await SessionManager.start(eventManager, hooks, { sessionSampleRate: 0, sessionOnError: true }); + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ + sessionSampleRate: 0, + sessionOnError: true, + })); expect(sessionManager.getSession().id).not.toBe('existing'); expect(sessionManager.getSession().trackingType).toBe(TrackingType.TRACKED_ON_ERROR); @@ -536,10 +544,10 @@ describe('sessionManager', () => { const now = Date.now(); mfs.readFile.mockResolvedValue(JSON.stringify({ id: 'existing', created: now, lastActivity: now })); - sessionManager = await SessionManager.start(eventManager, hooks, { + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ sessionSampleRate: 20, sessionOnError: false, - }); + })); expect(sessionManager.getSession()).toMatchObject({ id: 'existing', trackingType: TrackingType.TRACKED }); // Every session was collected then: it stands for itself, not for five. @@ -551,10 +559,10 @@ describe('sessionManager', () => { it('draws again when the session is renewed', async () => { mockNoSessionFile(); const random = vi.spyOn(Math, 'random').mockReturnValue(0.8); - sessionManager = await SessionManager.start(eventManager, hooks, { + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ sessionSampleRate: 50, sessionOnError: false, - }); + })); expect(sessionManager.getSession().trackingType).toBe(TrackingType.NOT_TRACKED); sessionManager.expire(); @@ -565,13 +573,237 @@ describe('sessionManager', () => { expect(sessionManager.getSession().trackingType).toBe(TrackingType.TRACKED); }); + describe('sampling read from a provider (remote configuration)', () => { + it('reads the sampling at each draw, so a change applies to the next session and not to this one', async () => { + mockNoSessionFile(); + let sampling: SamplingConfiguration = { sessionSampleRate: 100, sessionOnError: false }; + sessionManager = await SessionManager.start(eventManager, hooks, () => sampling); + const first = sessionManager.getSession(); + + sampling = { sessionSampleRate: 0, sessionOnError: false, rcVersion: 7 }; + expect(sessionManager.getSession()).toEqual(first); + + sessionManager.expire(); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.END_USER_ACTIVITY }); + await vi.advanceTimersByTimeAsync(0); + + expect(sessionManager.getSession()).toMatchObject({ + trackingType: TrackingType.NOT_TRACKED, + sampleRate: 0, + rcVersion: 7, + }); + }); + + it('stores the configuration version a session was drawn under, and reports it on its events', async () => { + mockNoSessionFile(); + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ + sessionSampleRate: 100, + sessionOnError: false, + rcVersion: 3, + })); + + expect(savedSessionStates()[0]).toMatchObject({ rcVersion: 3, sampleRate: 100 }); + expect(hooks.triggerRum({ eventType: 'view', startTime: Date.now() as TimeStamp })).toMatchObject({ + _dd: { configuration: { session_sample_rate: 100, rc_version: 3 } }, + }); + }); + + it('reports no version for a session drawn without a delivered configuration', async () => { + mockNoSessionFile(); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); + + expect(savedSessionStates()[0]).not.toHaveProperty('rcVersion'); + const result = hooks.triggerRum({ eventType: 'view', startTime: Date.now() as TimeStamp }) as { + _dd: { configuration: Record }; + }; + expect(result._dd.configuration.rc_version).toBeUndefined(); + }); + + it('keeps the version a resumed session was drawn under, whatever is delivered now', async () => { + const now = Date.now(); + mfs.readFile.mockResolvedValue( + JSON.stringify({ + id: 'existing', + created: now, + lastActivity: now, + trackingType: TrackingType.TRACKED_ON_ERROR, + sampleRate: 0, + rcVersion: 4, + }) + ); + + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ + sessionSampleRate: 100, + sessionOnError: false, + rcVersion: 9, + })); + + expect(sessionManager.getSession()).toMatchObject({ id: 'existing', rcVersion: 4 }); + // An on-error session stands for itself whatever it was drawn at. + expect(hooks.triggerRum({ eventType: 'view', startTime: now as TimeStamp })).toMatchObject({ + _dd: { configuration: { session_sample_rate: 0, rc_version: 4 } }, + }); + }); + + it.each([-1, 1.5, '3'])( + 'starts a fresh session rather than resuming one saved with version %s', + async (rcVersion) => { + const now = Date.now(); + mfs.readFile.mockResolvedValue( + JSON.stringify({ + id: 'existing', + created: now, + lastActivity: now, + trackingType: TrackingType.TRACKED, + sampleRate: 100, + rcVersion, + }) + ); + + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); + + expect(sessionManager.getSession().id).not.toBe('existing'); + } + ); + }); + + describe('applySamplingChange (a configuration that applies at once)', () => { + let sampling: SamplingConfiguration; + + async function startSession(drawnUnder: SamplingConfiguration, random = 0.5) { + mockNoSessionFile(); + vi.spyOn(Math, 'random').mockReturnValue(random); + sampling = drawnUnder; + sessionManager = await SessionManager.start(eventManager, hooks, () => sampling); + lifecycleEvents.length = 0; + } + + function applyChange(next: SamplingConfiguration) { + sampling = next; + sessionManager.applySamplingChange(); + } + + it.each([ + { + title: 'a session drawn at rate 0 draws again when the rate leaves 0', + drawnUnder: { sessionSampleRate: 0, sessionOnError: false }, + next: { sessionSampleRate: 30, sessionOnError: false }, + drawn: TrackingType.NOT_TRACKED, + ends: true, + }, + { + title: 'a session drawn at rate 0 draws again when the on-error switch turns on', + drawnUnder: { sessionSampleRate: 0, sessionOnError: false }, + next: { sessionSampleRate: 0, sessionOnError: true }, + drawn: TrackingType.NOT_TRACKED, + ends: true, + }, + { + title: 'a session drawn at rate 0 stays when nothing would keep it now either', + drawnUnder: { sessionSampleRate: 0, sessionOnError: false }, + next: { sessionSampleRate: 0, sessionOnError: false, rcVersion: 2 }, + drawn: TrackingType.NOT_TRACKED, + ends: false, + }, + { + title: 'a session that lost a draw at a real rate keeps its outcome', + drawnUnder: { sessionSampleRate: 50, sessionOnError: false }, + next: { sessionSampleRate: 100, sessionOnError: true }, + drawn: TrackingType.NOT_TRACKED, + random: 0.8, + ends: false, + }, + { + title: 'a drawn session meets the emergency stop', + drawnUnder: { sessionSampleRate: 100, sessionOnError: false }, + next: { sessionSampleRate: 0, sessionOnError: false }, + drawn: TrackingType.TRACKED, + ends: true, + }, + { + title: 'a drawn session meets the emergency stop even with the switch on: the switch shapes the next draw', + drawnUnder: { sessionSampleRate: 100, sessionOnError: false }, + next: { sessionSampleRate: 0, sessionOnError: true }, + drawn: TrackingType.TRACKED, + ends: true, + }, + { + title: 'an on-error session survives rate 0 while the switch stays on', + drawnUnder: { sessionSampleRate: 0, sessionOnError: true }, + next: { sessionSampleRate: 0, sessionOnError: true, rcVersion: 5 }, + drawn: TrackingType.TRACKED_ON_ERROR, + ends: false, + }, + { + title: 'an on-error session ends when the switch turns off at rate 0', + drawnUnder: { sessionSampleRate: 0, sessionOnError: true }, + next: { sessionSampleRate: 0, sessionOnError: false }, + drawn: TrackingType.TRACKED_ON_ERROR, + ends: true, + }, + { + title: 'a drawn session keeps its draw when the rate changes to another real rate', + drawnUnder: { sessionSampleRate: 100, sessionOnError: false }, + next: { sessionSampleRate: 10, sessionOnError: false }, + drawn: TrackingType.TRACKED, + ends: false, + }, + { + title: 'an on-error session keeps its draw when the rate rises above 0', + drawnUnder: { sessionSampleRate: 0, sessionOnError: true }, + next: { sessionSampleRate: 50, sessionOnError: false }, + drawn: TrackingType.TRACKED_ON_ERROR, + ends: false, + }, + ])('$title', async ({ drawnUnder, next, drawn, random, ends }) => { + await startSession(drawnUnder, random); + expect(sessionManager.getSession().trackingType).toBe(drawn); + + applyChange(next); + + expect(sessionManager.getSession().status).toBe(ends ? 'expired' : 'active'); + expect(lifecycleEvents.includes(LifecycleKind.SESSION_EXPIRED)).toBe(ends); + }); + + it('draws the next session under the new sampling, and is idempotent for it', async () => { + await startSession({ sessionSampleRate: 0, sessionOnError: false }); + + applyChange({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 6 }); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.END_USER_ACTIVITY }); + await vi.advanceTimersByTimeAsync(0); + + const renewed = [LifecycleKind.SESSION_EXPIRED, LifecycleKind.END_USER_ACTIVITY, LifecycleKind.SESSION_RENEW]; + expect(lifecycleEvents).toEqual(renewed); + expect(sessionManager.getSession()).toMatchObject({ + status: 'active', + trackingType: TrackingType.TRACKED_ON_ERROR, + rcVersion: 6, + }); + + // The same configuration again finds nothing decisive left. + sessionManager.applySamplingChange(); + expect(sessionManager.getSession().status).toBe('active'); + expect(lifecycleEvents).toEqual(renewed); + }); + + it('does nothing to a session that has already ended', async () => { + await startSession({ sessionSampleRate: 100, sessionOnError: false }); + sessionManager.expire(); + lifecycleEvents.length = 0; + + applyChange({ sessionSampleRate: 0, sessionOnError: false }); + + expect(lifecycleEvents).toEqual([]); + }); + }); + describe('setSessionHasError', () => { async function startWithheldSession() { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, { + sessionManager = await SessionManager.start(eventManager, hooks, () => ({ sessionSampleRate: 0, sessionOnError: true, - }); + })); mfs.writeFile.mockClear(); } @@ -721,7 +953,7 @@ describe('sessionManager', () => { it('leaves a drawn session alone, and writes nothing', async () => { mockNoSessionFile(); - sessionManager = await SessionManager.start(eventManager, hooks, SAMPLING); + sessionManager = await SessionManager.start(eventManager, hooks, () => SAMPLING); mfs.writeFile.mockClear(); sessionManager.setSessionHasError(sessionManager.getSession().id, T0); diff --git a/src/domain/session/SessionManager.ts b/src/domain/session/SessionManager.ts index ee6af615..6029650a 100644 --- a/src/domain/session/SessionManager.ts +++ b/src/domain/session/SessionManager.ts @@ -40,7 +40,10 @@ interface SessionState extends SessionRecord { lastActivity: number; } -export type SamplingConfiguration = Pick; +/** What a draw reads: the rate and the on-error switch in force, and the remote version they came from. */ +export interface SamplingConfiguration extends Pick { + rcVersion?: number; +} /** * Track session lifecycle @@ -49,8 +52,10 @@ export type SamplingConfiguration = Pick SamplingConfiguration ) {} static async start( eventManager: EventManager, hooks: FormatHooks, - sampling: SamplingConfiguration + getSampling: () => SamplingConfiguration ): Promise { - const manager = new SessionManager(eventManager, hooks, sampling); + const manager = new SessionManager(eventManager, hooks, getSampling); await manager.init(); return manager; } getSession(): Session { - const { id, trackingType, sampleRate, hasError } = this.currentState; - return deepClone({ id, trackingType, sampleRate, hasError, status: this.status }); + return deepClone({ ...toSessionRecord(this.currentState), status: this.status }); } expire(): void { this.expireSession(); } + /** + * Ends the running session where the sampling now in force is decisive for it, so that the next + * activity draws a new one under it. Called when a remote configuration that asks to apply at + * once (`activation: immediate`) has changed the sampling; anything else waits for the next + * session, because a session's draw is locked for its whole life. + * + * Only two cases are decisive: + * - A session that was not collected and was drawn at rate 0 lost no lottery: nothing was ever + * drawn for it. A rate leaving 0, or the on-error switch turning on, would now keep some such + * sessions, so it draws again. One that lost a draw at a real rate keeps its outcome, or the + * fleet would be re-rolled and a rate `p` would turn into `p²`. + * - A collected session meets the emergency stop, rate 0 — unless it is an on-error session and + * the switch stays on: rate 0 next to the switch is that switch's ordinary setting, and ending + * the session would throw away exactly the history the switch exists to keep. + * + * Idempotent: a session drawn under the sampling now in force is never decisive, so the same + * configuration arriving again ends nothing. + */ + applySamplingChange(): void { + if (this.status !== 'active') { + return; + } + const next = this.getSampling(); + const { trackingType, sampleRate } = this.currentState; + if (trackingType === TrackingType.NOT_TRACKED) { + if (sampleRate === 0 && (next.sessionSampleRate > 0 || next.sessionOnError)) { + this.expireSession(); + } + return; + } + if (next.sessionSampleRate === 0 && !(next.sessionOnError && trackingType === TrackingType.TRACKED_ON_ERROR)) { + this.expireSession(); + } + } + /** The session in force at `startTime`, current or past, or `undefined` when there was none. */ findSession(startTime: TimeStamp): SessionRecord | undefined { return this.sessionContext.find(startTime); @@ -163,10 +203,12 @@ export class SessionManager { private async createNewSession(): Promise { const now = Date.now(); + const sampling = this.getSampling(); const state: SessionState = { id: generateUUID(), - trackingType: drawTrackingType(this.sampling), - sampleRate: this.sampling.sessionSampleRate, + trackingType: drawTrackingType(sampling), + sampleRate: sampling.sessionSampleRate, + ...(sampling.rcVersion === undefined ? {} : { rcVersion: sampling.rcVersion }), created: now, lastActivity: now, }; @@ -181,8 +223,7 @@ export class SessionManager { private setCurrent(state: SessionState): void { this.currentState = state; this.status = 'active'; - const { id, trackingType, sampleRate, hasError } = state; - this.sessionContext.add({ id, trackingType, sampleRate, hasError }); + this.sessionContext.add(toSessionRecord(state)); } /** @@ -273,6 +314,17 @@ function drawTrackingType({ sessionSampleRate, sessionOnError }: SamplingConfigu return sessionOnError ? TrackingType.TRACKED_ON_ERROR : TrackingType.NOT_TRACKED; } +/** The record part of a state, without its timestamps. Absent fields stay absent. */ +function toSessionRecord({ id, trackingType, sampleRate, rcVersion, hasError }: SessionState): SessionRecord { + return { + id, + trackingType, + sampleRate, + ...(rcVersion === undefined ? {} : { rcVersion }), + ...(hasError === undefined ? {} : { hasError }), + }; +} + function getSessionFilePath(): string { return path.join(app.getPath('userData'), SESSION_FILE_NAME); } diff --git a/src/domain/session/index.ts b/src/domain/session/index.ts index 885549d9..b6280354 100644 --- a/src/domain/session/index.ts +++ b/src/domain/session/index.ts @@ -1,3 +1,9 @@ export * from './SessionManager'; export { SESSION_TIME_OUT_DELAY } from './session.constants'; -export { TrackingType, withholdsEvents, parseSessionRecord, type SessionRecord } from './SessionContext'; +export { + TrackingType, + withholdsEvents, + parseSessionRecord, + isConfigurationVersion, + type SessionRecord, +} from './SessionContext'; diff --git a/src/index.ts b/src/index.ts index 04a8aaec..5c38edfe 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,6 +5,7 @@ import { buildConfiguration } from './config'; import { RumCollection } from './domain/rum'; import { SessionManager, TrackingType } from './domain/session'; import { initAnonymousId } from './domain/AnonymousId'; +import { RemoteConfiguration } from './domain/RemoteConfiguration'; import { UserContext, type User } from './domain/UserContext'; import { UserActivityTracker } from './domain/UserActivityTracker'; import { RendererRegistry } from './domain/RendererRegistry'; @@ -25,6 +26,7 @@ let transport: Transport | undefined; let rumApi: ReturnType | undefined; let tracing: Tracing | undefined; let userContext: UserContext | undefined; +let remoteConfiguration: RemoteConfiguration | undefined; /** * Initialize the Electron SDK @@ -53,7 +55,10 @@ export async function init(configuration: InitConfiguration): Promise { registerCommonContext(config, hooks, anonymousId, getUserAt); startTelemetry(eventManager, config); - const manager = await SessionManager.start(eventManager, hooks, config); + // Before the first session is drawn: it draws with what the previous launch kept, if anything. + const remote = await RemoteConfiguration.init(config); + remoteConfiguration = remote; + const manager = await SessionManager.start(eventManager, hooks, () => remote.getSampling()); sessionManager = manager; const rendererRegistry = new RendererRegistry(); @@ -91,6 +96,9 @@ export async function init(configuration: InitConfiguration): Promise { const rum = await RumCollection.start(eventManager, hooks, rendererRegistry, stackPathNormalizer, manager); rumApi = rum.getApi(); + // Last, and never awaited: nothing in init waits on the network. + remote.start(eventManager, () => manager.applySamplingChange()); + return true; } @@ -162,6 +170,19 @@ export function clearUser(): void { callMonitored(() => userContext?.clear()); } +/** + * The application's own `custom` values, as delivered by the console's remote configuration — + * a copy, or `undefined` when `remoteConfigurationEnabled` is off, nothing has been delivered yet, + * or the console set none. Answers from the last configuration kept on disk until the server + * answers, so it can be read right after `init` resolves. + * + * The name matches `flashcatRum.getRemoteConfig()` in `@flashcatcloud/browser-rum`. Anyone holding + * the client token can read these values: put nothing secret in them. + */ +export function getRemoteConfig(): Record | undefined { + return callMonitored(() => remoteConfiguration?.getCustom()); +} + /** * Report a manually handled error */ diff --git a/src/mocks.specUtil.ts b/src/mocks.specUtil.ts index 7f4bdf3a..e5e11e9e 100644 --- a/src/mocks.specUtil.ts +++ b/src/mocks.specUtil.ts @@ -38,6 +38,7 @@ export function createTestConfiguration(overrides: Partial = {}): applicationId: 'test-app-id', sessionSampleRate: 100, sessionOnError: false, + remoteConfigurationEnabled: false, telemetrySampleRate: 100, defaultPrivacyLevel: 'mask', allowedWebViewHosts: [], From e97d08b9ca71c6f73c6551ee1fa40650802473f6 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 8 Oct 2026 04:50:29 -0700 Subject: [PATCH 02/25] =?UTF-8?q?=E2=9C=85=20cover=20remote=20configuratio?= =?UTF-8?q?n=20end=20to=20end=20against=20the=20fake=20intake?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fake intake serves /api/v2/rum/config with a body and ETag, a status, or a dropped connection, and records what the SDK asked. Each scenario has a negative control differing only in what it claims. --- e2e/lib/helpers.ts | 9 +- e2e/lib/intake.ts | 119 ++++++- e2e/scenarios/remote-config.scenario.ts | 438 ++++++++++++++++++++++++ 3 files changed, 554 insertions(+), 12 deletions(-) create mode 100644 e2e/scenarios/remote-config.scenario.ts diff --git a/e2e/lib/helpers.ts b/e2e/lib/helpers.ts index 46b3cf4c..725b09cf 100644 --- a/e2e/lib/helpers.ts +++ b/e2e/lib/helpers.ts @@ -2,7 +2,7 @@ import { test as base, _electron as electron, type ElectronApplication, type Pag import { basename, join } from 'node:path'; import { mkdtemp, readdir, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; -import { Intake } from './intake'; +import { Intake, type RemoteConfigBehaviour } from './intake'; import { TestServer } from './testServer'; import { MainPage } from './mainPage'; import type { InitConfiguration } from '@flashcatcloud/electron-sdk'; @@ -51,6 +51,8 @@ export interface TestFixtures { rumBrowserSdk: Record | null; /** Merged over the SDK configuration every scenario starts from. */ sdkConfig: Partial; + /** What the fake config endpoint answers from the first request on. Unset: 404. */ + remoteConfig: RemoteConfigBehaviour | undefined; } /** @@ -59,9 +61,9 @@ export interface TestFixtures { */ export const test = base.extend({ intake: [ - // eslint-disable-next-line no-empty-pattern - async ({}, use) => { + async ({ remoteConfig }, use) => { const intake = new Intake(); + intake.setRemoteConfig(remoteConfig); await intake.start(); await use(intake); await intake.stop(); @@ -102,6 +104,7 @@ export const test = base.extend({ rumBrowserSdk: [null, { option: true }], sdkConfig: [{}, { option: true }], + remoteConfig: [undefined, { option: true }], }); /** How the test app sequences `init()` against window creation. See `getInitMode` in its main.ts. */ diff --git a/e2e/lib/intake.ts b/e2e/lib/intake.ts index 651b594b..118c0c52 100644 --- a/e2e/lib/intake.ts +++ b/e2e/lib/intake.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto'; import * as http from 'node:http'; /** @@ -16,6 +17,12 @@ import * as http from 'node:http'; * - `Content-Type: text/plain;charset=UTF-8` (`application/json` is rejected) * - body is newline-delimited JSON, one event per line (a JSON array is rejected) * - every number is a whole number, except the few fields the intake really types as floats + * + * It also stands in for the remote configuration endpoint, `GET /api/v2/rum/config`, so remote + * configuration is exercised without ever pointing the SDK at a real backend — a fake configuration + * version reported to a real one would show up in the console's version statistics. What it answers + * is set per test with {@link Intake.setRemoteConfig}; with nothing set it answers 404, as for an + * application that has none. */ export interface ReceivedEvent { timestamp: number; @@ -31,6 +38,27 @@ export interface ProtocolViolation { } const RUM_TRACK_PATH = '/api/v2/rum'; +const CONFIG_PATH = '/api/v2/rum/config'; + +/** How the fake config endpoint answers. */ +export type RemoteConfigBehaviour = + /** + * 200 with this body, and an `ETag` digested from it the way the real endpoint does — a request + * that sends the tag back gets a 304. + */ + | { body: Record } + /** This status, with an empty body. */ + | { status: number } + /** The connection is dropped without an answer: what an offline client sees. */ + | { reset: true }; + +/** A request the SDK made to the config endpoint, as the real endpoint would have read it. */ +export interface ConfigRequest { + timestamp: number; + /** The query parameters inside `ddforward`, which is where the SDK puts them behind a proxy. */ + params: Record; + headers: Record; +} const EXPECTED_CONTENT_TYPE = 'text/plain'; /** @@ -81,6 +109,8 @@ export class Intake { private server: http.Server | null = null; private rumEvents: ReceivedEvent[] = []; private violations: ProtocolViolation[] = []; + private configRequests: ConfigRequest[] = []; + private remoteConfig: RemoteConfigBehaviour | undefined; private port = 0; private addViolation(reason: string, detail: string) { @@ -140,6 +170,10 @@ export class Intake { async start(port = 0): Promise { return new Promise((resolve, reject) => { this.server = http.createServer((req, res) => { + if (req.method === 'GET' && this.isConfigRequest(req)) { + this.answerConfigRequest(req, res); + return; + } if (req.method !== 'POST') { this.addViolation('unexpected method', `${req.method ?? '?'} ${req.url ?? '/'}`); res.writeHead(404, { 'Content-Type': 'application/json' }); @@ -154,15 +188,7 @@ export class Intake { }); req.on('end', () => { - const headers: Record = {}; - - for (const [key, value] of Object.entries(req.headers)) { - if (typeof value === 'string') { - headers[key.toLowerCase()] = value; - } else if (Array.isArray(value)) { - headers[key.toLowerCase()] = value.join(', '); - } - } + const headers = lowerCaseHeaders(req); this.checkRequest(req, headers); this.storeBody(body, headers); @@ -251,6 +277,61 @@ export class Intake { } } + /** Sets what the config endpoint answers from now on; `undefined` for a 404. */ + setRemoteConfig(behaviour: RemoteConfigBehaviour | undefined): void { + this.remoteConfig = behaviour; + } + + /** Every request made to the config endpoint so far, in arrival order. */ + getConfigRequests(): ConfigRequest[] { + return [...this.configRequests]; + } + + async waitForConfigRequests(count: number, timeout = 10000): Promise { + const startTime = Date.now(); + while (Date.now() - startTime < timeout) { + if (this.configRequests.length >= count) { + return this.getConfigRequests(); + } + await new Promise((resolve) => setTimeout(resolve, 50)); + } + throw new Error(`Timed out waiting for ${count} config request(s). Received ${this.configRequests.length}.`); + } + + private isConfigRequest(req: http.IncomingMessage): boolean { + return forwardedUrl(req).pathname === CONFIG_PATH; + } + + private answerConfigRequest(req: http.IncomingMessage, res: http.ServerResponse) { + const headers = lowerCaseHeaders(req); + this.configRequests.push({ + timestamp: Date.now(), + params: Object.fromEntries(forwardedUrl(req).searchParams), + headers, + }); + + const behaviour = this.remoteConfig; + if (behaviour === undefined) { + res.writeHead(404); + res.end(); + } else if ('reset' in behaviour) { + req.socket.destroy(); + } else if ('status' in behaviour) { + res.writeHead(behaviour.status); + res.end(); + } else { + const body = JSON.stringify(behaviour.body); + const etag = `"${createHash('sha256').update(body).digest('hex').slice(0, 16)}"`; + if (headers['if-none-match'] === etag) { + res.writeHead(304, { ETag: etag }); + res.end(); + return; + } + res.writeHead(200, { 'Content-Type': 'application/json; charset=utf-8', ETag: etag }); + res.end(body); + } + } + /** Every request that breached the intake wire contract, in arrival order. */ getProtocolViolations(): ProtocolViolation[] { return [...this.violations]; @@ -264,6 +345,7 @@ export class Intake { clear(): void { this.rumEvents = []; this.violations = []; + this.configRequests = []; } getPort(): number { @@ -271,6 +353,25 @@ export class Intake { } } +function lowerCaseHeaders(req: http.IncomingMessage): Record { + const headers: Record = {}; + for (const [key, value] of Object.entries(req.headers)) { + if (typeof value === 'string') { + headers[key.toLowerCase()] = value; + } else if (Array.isArray(value)) { + headers[key.toLowerCase()] = value.join(', '); + } + } + return headers; +} + +/** The URL the real intake would have seen: the one inside `ddforward`, or the request's own. */ +function forwardedUrl(req: http.IncomingMessage): URL { + const requestUrl = new URL(req.url ?? '/', 'http://localhost'); + const forward = requestUrl.searchParams.get('ddforward'); + return forward === null ? requestUrl : new URL(forward, 'http://localhost'); +} + function truncate(value: string, max = 200): string { return value.length > max ? `${value.slice(0, max)}…` : value; } diff --git a/e2e/scenarios/remote-config.scenario.ts b/e2e/scenarios/remote-config.scenario.ts new file mode 100644 index 00000000..6d8a9168 --- /dev/null +++ b/e2e/scenarios/remote-config.scenario.ts @@ -0,0 +1,438 @@ +import { mkdir, rm, stat, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import type { InitConfiguration } from '@flashcatcloud/electron-sdk'; +import { test, expect, launchAppManually, createUserDataDir, cleanupUserDataDir } from '../lib/helpers'; +import type { Intake } from '../lib/intake'; +import type { MainPage } from '../lib/mainPage'; + +/** + * Remote configuration, end to end, against the fake intake's config endpoint — never a real one: + * a fake configuration version reported to a real backend would show up in the console's version + * statistics. + * + * Every scenario comes with a negative control: the same steps under a configuration that differs + * only in what the scenario claims makes the difference. + */ + +/** Comfortably past the release jitter of an on-error session, which is at most 3 s. */ +const RELEASE_WAIT = 3_500; +/** Long enough for a configuration answer to be applied and the bridge to hear of a session change. */ +const APPLY_WAIT = 1_000; + +interface SessionEvent { + type: string; + date: number; + session: { id: string; sampled_for_error?: boolean }; + view: { id: string; is_active?: boolean }; + _dd?: { configuration?: { session_sample_rate?: number; rc_version?: number } }; +} + +/** A response the way the real endpoint writes it. */ +function configuration( + version: number, + activation: 'immediate' | 'next_session', + rum: { sessionSampleRate?: number; sessionOnError?: boolean } +) { + return { + body: { + schema_version: 1, + version, + ttl: 600, + enabled: true, + activation, + refresh_on_foreground: false, + rum, + custom: { scenario: 'e2e' }, + }, + }; +} + +/** Every RUM event received — of one session only, when given: an ended session's final view update may still arrive. */ +function rumEvents(intake: Intake, sessionId?: string): SessionEvent[] { + return intake + .getAllEvents() + .map((event) => event.body as SessionEvent) + .filter((body) => body.type !== 'telemetry' && (sessionId === undefined || body.session.id === sessionId)); +} + +async function settle(mainPage: MainPage, waitMs = RELEASE_WAIT) { + await new Promise((resolve) => setTimeout(resolve, waitMs)); + await mainPage.flushTransport(); +} + +/** Waits for the first answer to have been applied, and for the bridge to have heard of it. */ +async function waitForConfiguration(intake: Intake) { + await intake.waitForConfigRequests(1); + await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); +} + +test.afterEach(async ({ intake }, testInfo) => { + const captureDir = process.env.FC_E2E_CAPTURE_DIR; + if (!captureDir) { + return; + } + await mkdir(captureDir, { recursive: true }); + const fileName = `remote-config - ${testInfo.titlePath + .slice(1) + .join(' - ') + .replace(/[^\w.-]+/g, '_')}.json`; + await writeFile( + join(captureDir, fileName), + JSON.stringify({ configRequests: intake.getConfigRequests(), events: intake.getAllEvents() }, null, 2) + ); +}); + +test.describe('remote configuration', () => { + test.describe.configure({ timeout: 60_000 }); + + test.describe('the request', () => { + test.use({ + sdkConfig: { remoteConfigurationEnabled: true }, + remoteConfig: configuration(1, 'next_session', {}), + }); + + test('asks once at init and again at each new session, saying who it is and what it runs', async ({ + intake, + mainPage, + }) => { + const [first] = await intake.waitForConfigRequests(1); + expect(first.params).toEqual({ + client_token: 'test-client-token', + sdk: 'electron', + sdk_version: expect.any(String), + env: 'test', + app_version: '1.0.0', + }); + + await mainPage.renewSession(); + const [, second] = await intake.waitForConfigRequests(2); + expect(second.params.applied_version).toBe('1'); + // Its ETag, sent back: an unchanged configuration costs a 304. + expect(second.headers['if-none-match']).toMatch(/^".+"$/); + }); + }); + + test.describe('without remoteConfigurationEnabled (control)', () => { + test.use({ remoteConfig: configuration(1, 'immediate', { sessionSampleRate: 0 }) }); + + test('asks nothing, and a rate of 0 published for it changes nothing', async ({ intake, mainPage }) => { + await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); + + expect(intake.getConfigRequests()).toEqual([]); + expect(await mainPage.getBridgeSessionId()).not.toBe(''); + }); + }); + + test.describe('rate 0 with sessionOnError, immediate (§7.1)', () => { + test.use({ + // "Only error sessions" at init too: the session the configuration finds is an on-error one. + sdkConfig: { sessionSampleRate: 0, sessionOnError: true, remoteConfigurationEnabled: true }, + remoteConfig: configuration(1, 'immediate', { sessionSampleRate: 0, sessionOnError: true }), + }); + + test('leaves the on-error session running, and its error releases what it held', async ({ intake, mainPage }) => { + await waitForConfiguration(intake); + const configuredAt = intake.getConfigRequests()[0].timestamp; + const sessionId = await mainPage.getBridgeSessionId(); + expect(sessionId).not.toBe(''); + await settle(mainPage, 0); + expect(rumEvents(intake)).toEqual([]); + + await mainPage.generateManualError(); + await settle(mainPage); + + const events = rumEvents(intake); + expect(events.some((event) => event.type === 'error')).toBe(true); + expect(new Set(events.map((event) => event.session.id))).toEqual(new Set([sessionId])); + // The view the session opened with, at init — before the configuration was even asked for. + const views = events.filter((event) => event.type === 'view'); + expect(Math.min(...views.map((view) => view.date))).toBeLessThan(configuredAt); + for (const view of views) { + expect(view.session.sampled_for_error).toBe(true); + } + for (const event of events) { + expect(event._dd?.configuration?.session_sample_rate).toBe(0); + // Drawn before any configuration was delivered: there is no version it was drawn under. + expect(event._dd?.configuration?.rc_version).toBeUndefined(); + } + expect(intake.getProtocolViolations()).toEqual([]); + }); + }); + + test.describe('rate 0 with sessionOnError turned off, immediate (control for §7.1)', () => { + test.use({ + sdkConfig: { sessionSampleRate: 0, sessionOnError: true, remoteConfigurationEnabled: true }, + remoteConfig: configuration(1, 'immediate', { sessionSampleRate: 0, sessionOnError: false }), + }); + + test('ends the on-error session, and an error then releases nothing', async ({ intake, mainPage }) => { + await waitForConfiguration(intake); + + expect(await mainPage.getBridgeSessionId()).toBe(''); + await mainPage.generateManualError(); + await settle(mainPage); + expect(rumEvents(intake)).toEqual([]); + }); + }); + + test.describe('rate 0, immediate: the emergency stop', () => { + test.use({ + sdkConfig: { remoteConfigurationEnabled: true }, + remoteConfig: configuration(2, 'immediate', { sessionSampleRate: 0 }), + }); + + test('ends the drawn session, and the next one is not collected', async ({ intake, mainPage, testServer }) => { + await waitForConfiguration(intake); + expect(await mainPage.getBridgeSessionId()).toBe(''); + await settle(mainPage, 0); + const ended = rumEvents(intake).filter((event) => event.type === 'view'); + expect(ended.length).toBeGreaterThan(0); + // The view of the session that ended reports it as over. + expect(ended.some((view) => view.view.is_active === false)).toBe(true); + const endedSessionId = ended[0].session.id; + + intake.clear(); + await mainPage.generateActivity(); + await mainPage.mainFetch(testServer.urlFor(200)); + await settle(mainPage, 0); + + expect(await mainPage.getBridgeSessionId()).toBe(''); + expect(rumEvents(intake).filter((event) => event.session.id !== endedSessionId)).toEqual([]); + }); + }); + + test.describe('rate 0, next session (control for the emergency stop)', () => { + test.use({ + sdkConfig: { remoteConfigurationEnabled: true }, + remoteConfig: configuration(2, 'next_session', { sessionSampleRate: 0 }), + }); + + test('leaves the drawn session collecting', async ({ intake, mainPage, testServer }) => { + await waitForConfiguration(intake); + const sessionId = await mainPage.getBridgeSessionId(); + expect(sessionId).not.toBe(''); + + await mainPage.mainFetch(testServer.urlFor(200)); + await settle(mainPage, 0); + + const resources = rumEvents(intake).filter((event) => event.type === 'resource'); + expect(resources.length).toBeGreaterThan(0); + for (const resource of resources) { + expect(resource.session.id).toBe(sessionId); + expect(resource._dd?.configuration?.session_sample_rate).toBe(100); + } + }); + }); + + test.describe('next session (deferred)', () => { + test.use({ + sdkConfig: { remoteConfigurationEnabled: true }, + remoteConfig: configuration(3, 'next_session', { sessionSampleRate: 0, sessionOnError: true }), + }); + + test('keeps the current session as drawn, and draws the next one under the new values', async ({ + intake, + mainPage, + testServer, + }) => { + await waitForConfiguration(intake); + const current = await mainPage.getBridgeSessionId(); + await mainPage.mainFetch(testServer.urlFor(200)); + await settle(mainPage, 0); + const before = rumEvents(intake); + expect(before.some((event) => event.type === 'resource')).toBe(true); + for (const event of before) { + expect(event.session.id).toBe(current); + expect(event._dd?.configuration?.session_sample_rate).toBe(100); + expect(event._dd?.configuration?.rc_version).toBeUndefined(); + } + + intake.clear(); + await mainPage.renewSession(); + const next = await mainPage.getBridgeSessionId(); + expect(next).not.toBe(''); + expect(next).not.toBe(current); + await mainPage.mainFetch(testServer.urlFor(200)); + await settle(mainPage); + // An on-error session: nothing until it errors. + expect(rumEvents(intake, next)).toEqual([]); + + await mainPage.generateManualError(); + await settle(mainPage); + const after = rumEvents(intake, next); + expect(after.some((event) => event.type === 'error')).toBe(true); + for (const event of after) { + expect(event.session.id).toBe(next); + expect(event._dd?.configuration?.session_sample_rate).toBe(0); + expect(event._dd?.configuration?.rc_version).toBe(3); + } + expect(intake.getProtocolViolations()).toEqual([]); + }); + }); + + test.describe('next session without a configuration (control for deferred)', () => { + test.use({ sdkConfig: { remoteConfigurationEnabled: true } }); + + test('draws the next session under the init values', async ({ intake, mainPage, testServer }) => { + await waitForConfiguration(intake); + await mainPage.renewSession(); + await mainPage.mainFetch(testServer.urlFor(200)); + await settle(mainPage, 0); + + const resources = rumEvents(intake).filter((event) => event.type === 'resource'); + expect(resources.length).toBeGreaterThan(0); + for (const resource of resources) { + expect(resource._dd?.configuration?.session_sample_rate).toBe(100); + expect(resource._dd?.configuration?.rc_version).toBeUndefined(); + } + }); + }); + + test.describe('sessionOnError turned on for a session drawn at rate 0, immediate', () => { + test.use({ + sdkConfig: { sessionSampleRate: 0, remoteConfigurationEnabled: true }, + remoteConfig: configuration(4, 'immediate', { sessionOnError: true }), + }); + + test('ends the session so the next activity draws again, as an on-error session', async ({ intake, mainPage }) => { + await waitForConfiguration(intake); + expect(await mainPage.getBridgeSessionId()).toBe(''); + + await mainPage.generateActivity(); + const redrawn = await mainPage.getBridgeSessionId(); + expect(redrawn).not.toBe(''); + + await mainPage.generateManualError(); + await settle(mainPage); + const events = rumEvents(intake); + expect(events.some((event) => event.type === 'error')).toBe(true); + for (const event of events) { + expect(event.session.id).toBe(redrawn); + expect(event._dd?.configuration?.session_sample_rate).toBe(0); + expect(event._dd?.configuration?.rc_version).toBe(4); + } + expect(events.filter((event) => event.type === 'view').every((view) => view.session.sampled_for_error)).toBe( + true + ); + }); + }); + + test.describe('sessionOnError turned on for a session drawn at rate 0, next session (control)', () => { + test.use({ + sdkConfig: { sessionSampleRate: 0, remoteConfigurationEnabled: true }, + remoteConfig: configuration(4, 'next_session', { sessionOnError: true }), + }); + + test('keeps the session it has, which collects nothing', async ({ intake, mainPage }) => { + await waitForConfiguration(intake); + + await mainPage.generateActivity(); + expect(await mainPage.getBridgeSessionId()).toBe(''); + await mainPage.generateManualError(); + await settle(mainPage); + expect(rumEvents(intake)).toEqual([]); + }); + }); +}); + +test.describe('remote configuration kept for the next launch', () => { + // The app every test launches anyway collects nothing, so all events come from the launches below. + test.use({ sdkConfig: { sessionSampleRate: 0 } }); + test.describe.configure({ timeout: 90_000 }); + + const ENABLED: Partial = { remoteConfigurationEnabled: true }; + + /** Launch one: receives the configuration, keeps it, ends its session so the next launch draws anew. */ + async function receiveAndKeep(intake: Intake, userDataDir: string) { + intake.setRemoteConfig(configuration(5, 'next_session', { sessionSampleRate: 0, sessionOnError: true })); + const first = await launchAppManually(intake, userDataDir, 'await', ENABLED); + try { + await intake.waitForConfigRequests(1); + await waitForFile(join(userDataDir, '_fc_remote_config')); + await first.mainPage.stopSession(); + } finally { + await first.electronApp.close(); + } + intake.clear(); + // Offline from now on, as far as the configuration is concerned. + intake.setRemoteConfig({ reset: true }); + } + + test('draws the first session of a cold start, while offline, with the configuration kept on disk', async ({ + intake, + testServer, + }) => { + const userDataDir = await createUserDataDir(); + try { + await receiveAndKeep(intake, userDataDir); + + const second = await launchAppManually(intake, userDataDir, 'await', ENABLED); + try { + await intake.waitForConfigRequests(1); + expect(intake.getConfigRequests()[0].params.applied_version).toBe('5'); + // On-error, as kept: collected, so the bridge answers it, but nothing leaves without an error. + const sessionId = await second.mainPage.getBridgeSessionId(); + expect(sessionId).not.toBe(''); + await second.mainPage.mainFetch(testServer.urlFor(200)); + await settle(second.mainPage); + expect(rumEvents(intake, sessionId)).toEqual([]); + + await second.mainPage.generateManualError(); + await settle(second.mainPage); + const events = rumEvents(intake, sessionId); + expect(events.some((event) => event.type === 'error')).toBe(true); + for (const event of events) { + expect(event._dd?.configuration?.session_sample_rate).toBe(0); + expect(event._dd?.configuration?.rc_version).toBe(5); + } + } finally { + await second.electronApp.close(); + } + } finally { + await cleanupUserDataDir(userDataDir); + } + }); + + test('draws it under the init values when nothing was kept (control)', async ({ intake, testServer }) => { + const userDataDir = await createUserDataDir(); + try { + await receiveAndKeep(intake, userDataDir); + await rm(join(userDataDir, '_fc_remote_config')); + + const second = await launchAppManually(intake, userDataDir, 'await', ENABLED); + try { + await intake.waitForConfigRequests(1); + expect(intake.getConfigRequests()[0].params.applied_version).toBeUndefined(); + const sessionId = await second.mainPage.getBridgeSessionId(); + await second.mainPage.mainFetch(testServer.urlFor(200)); + await settle(second.mainPage, 0); + + const resources = rumEvents(intake, sessionId).filter((event) => event.type === 'resource'); + expect(resources.length).toBeGreaterThan(0); + for (const resource of resources) { + expect(resource._dd?.configuration?.session_sample_rate).toBe(100); + expect(resource._dd?.configuration?.rc_version).toBeUndefined(); + } + } finally { + await second.electronApp.close(); + } + } finally { + await cleanupUserDataDir(userDataDir); + } + }); +}); + +async function waitForFile(filePath: string, timeout = 10_000) { + const deadline = Date.now() + timeout; + for (;;) { + try { + await stat(filePath); + return; + } catch { + if (Date.now() >= deadline) { + throw new Error(`${filePath} did not appear within ${timeout}ms`); + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + } +} From 4677b6a6f1eb66a6e39774479597ba5237d734c6 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 8 Oct 2026 04:50:29 -0700 Subject: [PATCH 03/25] =?UTF-8?q?=F0=9F=93=9D=20document=20remote=20config?= =?UTF-8?q?uration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 58 ++++++++++++++++++++++++++++++++++++++++++++ docs/ARCHITECTURE.md | 8 ++++++ docs/TESTING.md | 6 +++++ 3 files changed, 72 insertions(+) diff --git a/README.md b/README.md index c459bdc3..b75f6ecb 100644 --- a/README.md +++ b/README.md @@ -371,6 +371,14 @@ device. Events already reported keep the identity they were reported with, and events describing a moment before the logout still resolve to the user who was logged in then. +### `getRemoteConfig(): Record | undefined` + +The application's own `custom` values, as delivered by the console's remote configuration — a copy, +or `undefined` when `remoteConfigurationEnabled` is off, nothing has been delivered yet, or the +console set none. It answers from the configuration kept on disk until the server answers, so it can +be read right after `init` resolves. Anyone holding the client token can read these values: put +nothing secret in them. See [Remote configuration](#remote-configuration). + ### Configuration Options | Option | Type | Required | Default | Description | @@ -383,6 +391,7 @@ before the logout still resolve to the user who was logged in then. | `version` | `string` | No | — | Application version | | `sessionSampleRate` | `number` | No | `100` | Percentage of sessions collected (0–100). An out-of-range value fails `init`. See [Sampling](#sampling) | | `sessionOnError` | `boolean` | No | `false` | Keeps the sessions `sessionSampleRate` did not draw in memory, uploading them only if they report an error. See [Sampling](#sampling) | +| `remoteConfigurationEnabled` | `boolean` | No | `false` | Let the console change the sampling and deliver `custom` values. See [Remote configuration](#remote-configuration) | | `telemetrySampleRate` | `number` | No | `20` | Telemetry sample rate (0–100) | | `batchSize` | `'SMALL' \| 'MEDIUM' \| 'LARGE'` | No | — | Batch size for event uploads | | `uploadFrequency` | `'RARE' \| 'NORMAL' \| 'FREQUENT'` | No | — | Upload frequency for event batches | @@ -440,6 +449,55 @@ await init({ > is uploaded whether or not the session ever reports an error. Do not combine the two until the > Browser SDK can withhold replay for a bridged session. +### Remote configuration + +With `remoteConfigurationEnabled: true`, the console can change `sessionSampleRate` and +`sessionOnError` without a new release of the application, and hand it `custom` values (read with +`getRemoteConfig()`). Off by default: nothing is requested and the init values apply. + +```ts +await init({ + // ... + sessionSampleRate: 100, // used until the console says otherwise, and for any knob it does not set + remoteConfigurationEnabled: true, +}); +``` + +- **Precedence.** A value the console sets takes precedence over the init value; a value it does not + set leaves the init value in place. While the console's configuration is switched off, the init + values apply. +- **When it is asked for.** By the main process, at `init` and whenever a new session starts — a + change can only matter at a draw, and every draw is a new session. A failed request (offline, + timeout, an error status, a body that is not a configuration) changes nothing and is retried after + about 5 s and 60 s, then not until the next session. Nothing in `init` waits for it. +- **Kept on disk.** The last configuration accepted is kept in the application's `userData` + directory, so the next launch draws its first session with it before the network answers — or + without the network at all. It applies only to the same intake, application, `env` and `version`. + An SDK upgrade keeps the values but asks for the full configuration again rather than revalidating + the one the previous version read. +- **When a change applies.** A session's draw is locked for its whole life, so by default + (`next_session`) a change applies from the next session on. When the console asks for a change to + apply immediately, the running session is ended — and the next user activity draws a new one — + only where the new values decide it: + - a collected session ends when the rate becomes `0` (the emergency stop), unless it is a session + kept by `sessionOnError` and the switch stays on — rate `0` next to the switch is that switch's + ordinary setting; + - a session that was not collected, and was drawn at rate `0`, ends when the rate rises above `0` + or `sessionOnError` turns on, so that it is drawn again. One that lost a draw at a real rate keeps + its outcome. + + Any other change waits for the next session. + +- **What events report.** A session reports the rate it was drawn at (`0` for a session kept by + `sessionOnError`) and, when its draw read a delivered configuration, that configuration's version + as `_dd.configuration.rc_version` — on main process and renderer events alike, resumed sessions and + next-launch crash reports included. + +**Renderers.** The main process owns the sessions and their sampling, so enable remote configuration +in the main process only. A renderer's Browser SDK under the bridge does not fetch a configuration +of its own, and the main process overwrites the sampling attributes and `rc_version` of every event +a renderer sends, so nothing is applied twice. + ### Pre-warmed windows Applications commonly pre-create a renderer — `new BrowserWindow({ show: false })` — and navigate diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index accba090..3103a5b3 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -106,6 +106,14 @@ judged, and reported, by the draw they were made under. not drawn, and of a withheld session that ended without an error (its late events), and stamps `session.sampled_for_error` and `_dd.configuration.session_sample_rate`. +With `remoteConfigurationEnabled`, the sampling a draw reads comes from `RemoteConfiguration`: the +console's values over the init ones, fetched at init and at every new session, and kept on disk +(`_fc_remote_config`, replaced through `StateFile`) for the next launch's first draw. The session +records the configuration version it was drawn under next to its rate, and `SessionContext` reports it +as `_dd.configuration.rc_version`. A configuration published with `activation: immediate` asks +`SessionManager.applySamplingChange` to end the running session where the new sampling is decisive +for it; anything else applies from the next session. + `WithheldEventBuffer` sits between assembly and the RUM batch, so it sees main-process and renderer events alike, already final. It holds a withheld session's events and releases them at the session's first error; telemetry bypasses it. `CrashCollection` releases the session of a crash reported on the diff --git a/docs/TESTING.md b/docs/TESTING.md index 92705eea..dbdc5243 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -68,6 +68,12 @@ fractional crash `date` (`fs.Stats.birthtimeMs`) and a fractional resource `date start in nanoseconds) all the way through the `v0.1.0` release. When adding a scenario for a new event type, assert `intake.getProtocolViolations()` is empty. +The fake intake also serves the remote configuration endpoint (`GET /api/v2/rum/config`): a test +sets what it answers with the `remoteConfig` fixture option or `intake.setRemoteConfig()` — a body +(with an `ETag`, and a 304 when it is sent back), a status, or a dropped connection — and reads what +the SDK asked with `intake.getConfigRequests()`. Remote configuration is never tested against a real +backend: a fake configuration version reported there shows up in the console's version statistics. + ### Not covered - **APM spans.** FlashCat exposes no `/api/v2/spans` ingest, so the SPANS track is not uploaded and From 597ade7c85f444d9e881ce171174524dc6612626 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 01:39:56 -0700 Subject: [PATCH 04/25] =?UTF-8?q?=F0=9F=90=9B=20keep=20an=20immediate=20co?= =?UTF-8?q?nfiguration=20enforced=20across=20restarts,=20exits=20and=20upg?= =?UTF-8?q?rades,=20and=20revalidate=20it=20every=20ttl?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Keep the activation with the configuration on disk. At startup, before anything is collected, a session resumed from before an immediate configuration the previous launch kept (an older version, or none) is judged by it, so a stop that launch obeyed in memory is not undone by a session file it did not get to delete. - On APP_MAY_EXIT, write an accepted configuration whose asynchronous write has not landed, before returning; stop fetching on quit. - Judge the running session on a same-version answer whose values this SDK reads differently (after an upgrade), not only on a higher version. - Revalidate every ttl the server gives (60 s floor, 600 s default), conditionally, with one timer restarted after every request, so a stop reaches a long-lived desktop session without waiting for it to turn over. - Treat version 0, what an application with nothing published answers, as no version: no rc_version, no applied_version. - Name the schema version received in the unsupported-schema warning. --- src/config.ts | 6 +- src/domain/RemoteConfiguration.spec.ts | 371 ++++++++++++++++++++++--- src/domain/RemoteConfiguration.ts | 213 ++++++++++---- src/index.ts | 7 +- 4 files changed, 507 insertions(+), 90 deletions(-) diff --git a/src/config.ts b/src/config.ts index 6b79ffd4..3bcf6519 100644 --- a/src/config.ts +++ b/src/config.ts @@ -63,9 +63,9 @@ export interface InitConfiguration { * application, and deliver the application's own `custom` values (see `getRemoteConfig()`). * Defaults to `false`: nothing is requested and the init values apply. * - * The main process asks for the configuration at init and whenever a new session starts, and - * keeps the last good answer on disk, so the next launch draws its first session with it before - * the network answers. A delivered value takes precedence over the init value; a value the + * The main process asks for the configuration at init, whenever a new session starts, and every + * `ttl` the server gives while the application runs, and keeps the last good answer on disk, so new + * sessions draw with it before the network answers. A delivered value takes precedence over the init value; a value the * console did not set leaves the init value in place. A change applies to the next session, * unless the console asks for it to apply at once — see the README, Remote configuration. * diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts index 928bb47a..a9ee7c1d 100644 --- a/src/domain/RemoteConfiguration.spec.ts +++ b/src/domain/RemoteConfiguration.spec.ts @@ -7,8 +7,12 @@ const { writeFileSync, renameSync, unlinkSync, readdirSync } = vi.hoisted(() => readdirSync: vi.fn(() => []), })); vi.mock('node:fs', () => ({ writeFileSync, renameSync, unlinkSync, readdirSync })); +const { appListeners } = vi.hoisted(() => ({ appListeners: new Map void>() })); vi.mock('electron', () => ({ - app: { getPath: vi.fn(() => '/mock/user/data') }, + app: { + getPath: vi.fn(() => '/mock/user/data'), + on: vi.fn((event: string, listener: () => void) => appListeners.set(event, listener)), + }, })); vi.mock('../tools/display', () => ({ displayError: vi.fn(), displayWarn: vi.fn() })); @@ -17,11 +21,15 @@ import type { Configuration } from '../config'; import { EventKind, EventManager, LifecycleKind } from '../event'; import { displayWarn } from '../tools/display'; import { + DEFAULT_TTL, + MIN_TTL, REMOTE_CONFIGURATION_FILE_NAME, REQUEST_TIMEOUT, RETRY_DELAYS, RemoteConfiguration, } from './RemoteConfiguration'; +import { createFormatHooks } from '../assembly'; +import { SessionManager, TrackingType, type Session } from './session'; const mfs = mockFs(); const FILE_PATH = `/mock/user/data/${REMOTE_CONFIGURATION_FILE_NAME}`; @@ -91,6 +99,8 @@ describe('RemoteConfiguration', () => { afterEach(() => { remote?.stop(); remote = undefined; + session = undefined as unknown as Partial; + appListeners.clear(); vi.useRealTimers(); vi.unstubAllGlobals(); vi.clearAllMocks(); @@ -98,9 +108,13 @@ describe('RemoteConfiguration', () => { mfs.reset(); }); + /** The session owner: a session drawn under the configuration in force, unless a test says otherwise. */ + let session: Partial; + async function start(): Promise { remote = await RemoteConfiguration.init(config); - remote.start(eventManager, onImmediateChange); + session ??= { rcVersion: remote.getSampling().rcVersion }; + remote.start(eventManager, { getSession: () => session as Session, applySamplingChange: onImmediateChange }); await vi.advanceTimersByTimeAsync(0); return remote; } @@ -136,7 +150,7 @@ describe('RemoteConfiguration', () => { describe('request', () => { it('asks the config endpoint through the proxy with the client identity', async () => { - fetchMock.mockResolvedValue(ok(configurationBody())); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody()))); await start(); @@ -155,7 +169,7 @@ describe('RemoteConfiguration', () => { it('asks the site directly without a proxy, and leaves out what is not configured', async () => { config = { ...config, proxy: undefined, site: 'intake.example.test', env: undefined, version: undefined }; - fetchMock.mockResolvedValue(ok(configurationBody())); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody()))); await start(); @@ -166,7 +180,7 @@ describe('RemoteConfiguration', () => { it('reports the version it holds, and sends its ETag', async () => { mfs.readFile.mockResolvedValue(storedFile()); - fetchMock.mockResolvedValue(new Response(null, { status: 304 })); + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status: 304 }))); await start(); @@ -178,14 +192,14 @@ describe('RemoteConfiguration', () => { fetchMock.mockReturnValue(new Promise(() => undefined)); remote = await RemoteConfiguration.init(config); - remote.start(eventManager, onImmediateChange); + remote.start(eventManager, { getSession: () => ({}) as Session, applySamplingChange: onImmediateChange }); expect(fetchMock).toHaveBeenCalledTimes(1); expect(remote.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); }); it('asks again whenever a new session starts', async () => { - fetchMock.mockResolvedValue(ok(configurationBody())); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody()))); await start(); eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); @@ -207,7 +221,7 @@ describe('RemoteConfiguration', () => { describe('parsing and validation', () => { it('applies the delivered rate and switch over the init values, with their version', async () => { - fetchMock.mockResolvedValue(ok(configurationBody())); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody()))); const configuration = await start(); @@ -215,7 +229,7 @@ describe('RemoteConfiguration', () => { }); it('keeps the init value of a knob the console did not set', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ rum: { sessionOnError: true } }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ rum: { sessionOnError: true } })))); const configuration = await start(); @@ -226,7 +240,7 @@ describe('RemoteConfiguration', () => { ['absent', undefined], ['null', null], ])('reads a %s rum bag as empty', async (_, rum) => { - fetchMock.mockResolvedValue(ok(configurationBody({ rum }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ rum })))); const configuration = await start(); @@ -234,7 +248,9 @@ describe('RemoteConfiguration', () => { }); it('drops the values and custom while the kill switch is off, and keeps the version', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ enabled: false, custom: { flag: true } }))); + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ enabled: false, custom: { flag: true } }))) + ); const configuration = await start(); @@ -249,7 +265,7 @@ describe('RemoteConfiguration', () => { ['a switch as a string', { sessionOnError: 'true' }], ['a switch as a number', { sessionOnError: 1 }], ])('drops %s and keeps the init value', async (_, rum) => { - fetchMock.mockResolvedValue(ok(configurationBody({ rum }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ rum })))); const configuration = await start(); @@ -257,7 +273,9 @@ describe('RemoteConfiguration', () => { }); it('hands the custom bag over as a copy', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ custom: { allowList: ['a', 'b'], level: 2 } }))); + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ custom: { allowList: ['a', 'b'], level: 2 } }))) + ); const configuration = await start(); const custom = configuration.getCustom()!; @@ -268,7 +286,7 @@ describe('RemoteConfiguration', () => { }); it('drops a custom bag that is not an object', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ custom: ['a'] }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ custom: ['a'] })))); const configuration = await start(); @@ -287,7 +305,7 @@ describe('RemoteConfiguration', () => { ['a rum bag that is not an object', JSON.stringify(configurationBody({ rum: [] }))], ])('refuses %s, keeps what it holds, and retries', async (_, body) => { mfs.readFile.mockResolvedValue(storedFile()); - fetchMock.mockResolvedValue(ok(body)); + fetchMock.mockImplementation(() => Promise.resolve(ok(body))); const configuration = await start(); @@ -298,12 +316,13 @@ describe('RemoteConfiguration', () => { }); it('refuses a schema it does not know without retrying, and says so', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ schema_version: 2 }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ schema_version: 2 })))); const configuration = await start(); expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); expect(displayWarn).toHaveBeenCalledTimes(1); + expect(displayWarn).toHaveBeenCalledWith(expect.stringContaining('schema_version 2, which this SDK (test)')); await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); expect(fetchMock).toHaveBeenCalledTimes(1); @@ -316,7 +335,9 @@ describe('RemoteConfiguration', () => { it('ignores an older version arriving late', async () => { mfs.readFile.mockResolvedValue(storedFile({ version: 5 })); - fetchMock.mockResolvedValue(ok(configurationBody({ version: 4, rum: { sessionSampleRate: 100 } }))); + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ version: 4, rum: { sessionSampleRate: 100 } }))) + ); const configuration = await start(); @@ -327,7 +348,7 @@ describe('RemoteConfiguration', () => { describe('cache on disk', () => { it('writes the configuration with its ETag, the SDK version and who it was for', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ custom: { flag: 1 } }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ custom: { flag: 1 } })))); await start(); @@ -339,6 +360,7 @@ describe('RemoteConfiguration', () => { version: 3, values: { sessionSampleRate: 25, sessionOnError: true }, custom: { flag: 1 }, + activation: 'next_session', etag: '"tag-3"', }, ]); @@ -387,7 +409,7 @@ describe('RemoteConfiguration', () => { describe('304', () => { it('keeps what it holds, and writes nothing', async () => { mfs.readFile.mockResolvedValue(storedFile()); - fetchMock.mockResolvedValue(new Response(null, { status: 304 })); + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status: 304 }))); const configuration = await start(); @@ -398,7 +420,7 @@ describe('RemoteConfiguration', () => { }); it('retries when it holds nothing a 304 could refer to', async () => { - fetchMock.mockResolvedValue(new Response(null, { status: 304 })); + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status: 304 }))); await start(); await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); @@ -422,8 +444,8 @@ describe('RemoteConfiguration', () => { it('replaces them with the full answer, read by this version, and rewrites the file as its own', async () => { // The older SDK did not read the switch, so its file has none. mfs.readFile.mockResolvedValue(storedFile({ sdkVersion: 'older', values: { sessionSampleRate: 0 } })); - fetchMock.mockResolvedValue( - ok(configurationBody({ version: 2, rum: { sessionSampleRate: 0, sessionOnError: true } })) + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ version: 2, rum: { sessionSampleRate: 0, sessionOnError: true } }))) ); const configuration = await start(); @@ -452,7 +474,7 @@ describe('RemoteConfiguration', () => { expect(fetchMock).toHaveBeenCalledTimes(2); await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1]); expect(fetchMock).toHaveBeenCalledTimes(3); - await vi.advanceTimersByTimeAsync(10 * RETRY_DELAYS[1]); + await vi.advanceTimersByTimeAsync(DEFAULT_TTL - 1000); expect(fetchMock).toHaveBeenCalledTimes(3); eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); @@ -502,7 +524,7 @@ describe('RemoteConfiguration', () => { }); it.each([500, 503, 429])('retries on %s', async (status) => { - fetchMock.mockResolvedValue(new Response('', { status })); + fetchMock.mockImplementation(() => Promise.resolve(new Response('', { status }))); await start(); await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); @@ -511,7 +533,7 @@ describe('RemoteConfiguration', () => { }); it.each([401, 403, 404])('does not retry on %s', async (status) => { - fetchMock.mockResolvedValue(new Response('', { status })); + fetchMock.mockImplementation(() => Promise.resolve(new Response('', { status }))); await start(); await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); @@ -536,7 +558,7 @@ describe('RemoteConfiguration', () => { describe('activation', () => { it('tells the session owner about a new version that applies at once, once it is in force', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'immediate' }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: 'immediate' })))); let samplingWhenTold: unknown; onImmediateChange.mockImplementation(() => (samplingWhenTold = remote!.getSampling())); @@ -547,7 +569,7 @@ describe('RemoteConfiguration', () => { }); it('does not tell it about a change for the next session', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'next_session' }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: 'next_session' })))); const configuration = await start(); @@ -556,16 +578,44 @@ describe('RemoteConfiguration', () => { }); it('reads an absent or unknown activation as the next session', async () => { - fetchMock.mockResolvedValue(ok(configurationBody({ activation: undefined }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: undefined })))); await start(); expect(onImmediateChange).not.toHaveBeenCalled(); }); - it('does not tell it again about the version it already holds', async () => { - mfs.readFile.mockResolvedValue(storedFile({ version: 3, sdkVersion: 'older' })); - fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'immediate' }))); + it('does not tell it again about the version it already holds, unchanged', async () => { + mfs.readFile.mockResolvedValue( + storedFile({ version: 3, sdkVersion: 'older', values: { sessionSampleRate: 25, sessionOnError: true } }) + ); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: 'immediate' })))); + + await start(); + + expect(onImmediateChange).not.toHaveBeenCalled(); + }); + + it('tells it about the version it already holds when this SDK reads different values in it', async () => { + // An older SDK did not read the switch: same version, values this SDK reads differently. + mfs.readFile.mockResolvedValue( + storedFile({ version: 3, sdkVersion: 'older', values: { sessionSampleRate: 25 } }) + ); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: 'immediate' })))); + let samplingWhenTold: unknown; + onImmediateChange.mockImplementation(() => (samplingWhenTold = remote!.getSampling())); + + await start(); + + expect(onImmediateChange).toHaveBeenCalledTimes(1); + expect(samplingWhenTold).toEqual({ sessionSampleRate: 25, sessionOnError: true, rcVersion: 3 }); + }); + + it('does not tell it about the version it already holds with different values for the next session', async () => { + mfs.readFile.mockResolvedValue( + storedFile({ version: 3, sdkVersion: 'older', values: { sessionSampleRate: 25 } }) + ); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: 'next_session' })))); await start(); @@ -574,11 +624,268 @@ describe('RemoteConfiguration', () => { it('tells it about a higher version', async () => { mfs.readFile.mockResolvedValue(storedFile({ version: 2 })); - fetchMock.mockResolvedValue(ok(configurationBody({ activation: 'immediate' }))); + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ activation: 'immediate' })))); await start(); expect(onImmediateChange).toHaveBeenCalledTimes(1); }); }); + + describe('version 0: an application with nothing published', () => { + it('reports no version for what it draws, and asks again without one', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ version: 0, enabled: false, rum: {} }))) + ); + + const configuration = await start(); + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + expect(requestedUrl(1).searchParams.has('applied_version')).toBe(false); + // The ETag still saves the body. + expect(requestHeaders(1)).toEqual({ 'If-None-Match': '"tag-3"' }); + }); + }); + + describe('revalidation while the application runs (ttl)', () => { + it('asks again, conditionally, once the ttl the server gave has passed', async () => { + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl: 120 })))); + await start(); + + await vi.advanceTimersByTimeAsync(120_000 - 1); + expect(fetchMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(requestHeaders(1)).toEqual({ 'If-None-Match': '"tag-3"' }); + + // And again, ttl after that. + await vi.advanceTimersByTimeAsync(120_000); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it.each([ + ['below the floor', 5, MIN_TTL], + ['missing', undefined, DEFAULT_TTL], + ['not a number', '120', DEFAULT_TTL], + ['zero', 0, DEFAULT_TTL], + ])('uses a sane interval for a ttl %s', async (_, ttl, expected) => { + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl })))); + await start(); + + await vi.advanceTimersByTimeAsync(expected - 1); + expect(fetchMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('revalidates on the default interval when nothing was ever answered', async () => { + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + vi.spyOn(Math, 'random').mockReturnValue(0.5); + await start(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] + RETRY_DELAYS[1]); + expect(fetchMock).toHaveBeenCalledTimes(3); + + await vi.advanceTimersByTimeAsync(DEFAULT_TTL); + + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + + it('restarts the interval after a fetch a new session made', async () => { + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl: 120 })))); + await start(); + await vi.advanceTimersByTimeAsync(100_000); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(2); + + await vi.advanceTimersByTimeAsync(120_000 - 1); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); + expect(fetchMock).toHaveBeenCalledTimes(3); + }); + + it('never has two requests in flight', async () => { + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60 }))); + fetchMock.mockReturnValue(new Promise(() => undefined)); + await start(); + + await vi.advanceTimersByTimeAsync(60_000); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(REQUEST_TIMEOUT - 1); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('delivers an immediate stop to the running session at revalidation', async () => { + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, version: 3 }))); + await start(); + expect(onImmediateChange).not.toHaveBeenCalled(); + + fetchMock.mockResolvedValueOnce( + ok(configurationBody({ version: 4, activation: 'immediate', rum: { sessionSampleRate: 0 } }), '"tag-4"') + ); + await vi.advanceTimersByTimeAsync(60_000); + + expect(onImmediateChange).toHaveBeenCalledTimes(1); + expect(remote!.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: false, rcVersion: 4 }); + }); + + it('stops revalidating once stopped', async () => { + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl: 60 })))); + const configuration = await start(); + + configuration.stop(); + await vi.advanceTimersByTimeAsync(DEFAULT_TTL * 2); + + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + }); + + describe('when the application exits', () => { + function mayExit() { + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.APP_MAY_EXIT }); + } + + function syncWrites(): Record[] { + return writeFileSync.mock.calls + .filter(([filePath]) => (filePath as string).startsWith(`${FILE_PATH}.`)) + .map(([, content]) => JSON.parse(content as string) as Record); + } + + it('writes an accepted configuration whose write has not landed before returning, and the next launch draws with it offline', async () => { + mfs.readFile.mockResolvedValue(storedFile({ version: 8, values: { sessionSampleRate: 100 } })); + mfs.writeFile.mockReturnValue(new Promise(() => undefined)); + fetchMock.mockImplementation(() => + Promise.resolve( + ok(configurationBody({ version: 9, activation: 'immediate', rum: { sessionSampleRate: 0 } }), '"tag-9"') + ) + ); + await start(); + + mayExit(); + + expect(syncWrites()).toEqual([expect.objectContaining({ version: 9, activation: 'immediate', etag: '"tag-9"' })]); + expect(renameSync).toHaveBeenLastCalledWith(expect.stringMatching(/\.tmp$/), FILE_PATH); + + // Next launch, offline: it reads what the exit wrote. + remote!.stop(); + mfs.readFile.mockResolvedValue(writeFileSync.mock.calls[writeFileSync.mock.calls.length - 1][1] as string); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + remote = await RemoteConfiguration.init(config); + expect(remote.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: false, rcVersion: 9 }); + }); + + it('writes nothing when what it holds has landed already', async () => { + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody()))); + await start(); + + mayExit(); + + expect(syncWrites()).toEqual([]); + }); + + it('writes nothing when it holds nothing new since the launch', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status: 304 }))); + await start(); + + mayExit(); + + expect(syncWrites()).toEqual([]); + }); + + it('stops when the application quits: the request in flight is aborted and nothing is asked again', async () => { + fetchMock.mockReturnValue(new Promise(() => undefined)); + await start(); + + appListeners.get('quit')!(); + await vi.advanceTimersByTimeAsync(DEFAULT_TTL * 2); + + expect((fetchMock.mock.calls[0][1]!.signal as AbortSignal).aborted).toBe(true); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + }); + + describe('a session resumed from before an immediate configuration the previous launch kept', () => { + const SESSION_PATH = '/mock/user/data/_dd_s'; + let manager: SessionManager | undefined; + + afterEach(() => { + manager?.stop(); + manager = undefined; + }); + + /** What a launch that ended before its session file was deleted leaves behind. */ + async function restart(resumed: Record, kept: Record): Promise { + const now = Date.now(); + mfs.access.mockResolvedValue(undefined); + mfs.readFile.mockImplementation((filePath: string) => { + if (filePath === SESSION_PATH) { + return Promise.resolve(JSON.stringify({ id: 'resumed', created: now, lastActivity: now, ...resumed })); + } + if (filePath === FILE_PATH) { + return Promise.resolve(storedFile(kept)); + } + return Promise.reject(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); + }); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + + remote = await RemoteConfiguration.init(config); + const configuration = remote; + manager = await SessionManager.start(eventManager, createFormatHooks(), () => configuration.getSampling()); + expect(manager.getSession()).toMatchObject({ id: 'resumed', status: 'active' }); + remote.applyKept(manager); + return manager; + } + + const STOP = { version: 9, values: { sessionSampleRate: 0 }, activation: 'immediate' }; + + it.each([ + { title: 'drawn under an older version', resumed: { rcVersion: 8 } }, + { title: 'drawn under no version at all', resumed: {} }, + ])('ends a collected session $title, before anything is fetched', async ({ resumed }) => { + const sessions = await restart({ trackingType: TrackingType.TRACKED, sampleRate: 100, ...resumed }, STOP); + + expect(sessions.getSession().status).toBe('expired'); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it('leaves it when the kept configuration applies to the next session', async () => { + const sessions = await restart( + { trackingType: TrackingType.TRACKED, sampleRate: 100, rcVersion: 8 }, + { ...STOP, activation: 'next_session' } + ); + + expect(sessions.getSession().status).toBe('active'); + }); + + it('leaves it when it was drawn under that very version', async () => { + const sessions = await restart({ trackingType: TrackingType.TRACKED, sampleRate: 100, rcVersion: 9 }, STOP); + + expect(sessions.getSession().status).toBe('active'); + }); + + it('leaves an on-error session while the switch stays on', async () => { + const sessions = await restart( + { trackingType: TrackingType.TRACKED_ON_ERROR, sampleRate: 0, rcVersion: 8 }, + { ...STOP, values: { sessionSampleRate: 0, sessionOnError: true } } + ); + + expect(sessions.getSession().status).toBe('active'); + }); + + it('ends an on-error session when the kept configuration turned the switch off', async () => { + const sessions = await restart( + { trackingType: TrackingType.TRACKED_ON_ERROR, sampleRate: 0, rcVersion: 8 }, + { ...STOP, values: { sessionSampleRate: 0, sessionOnError: false } } + ); + + expect(sessions.getSession().status).toBe('expired'); + }); + }); }); diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts index 1203ea28..5f8b4943 100644 --- a/src/domain/RemoteConfiguration.ts +++ b/src/domain/RemoteConfiguration.ts @@ -3,10 +3,10 @@ import * as fs from 'node:fs/promises'; import * as path from 'node:path'; import { deepClone, ONE_SECOND, type Subscription } from '@flashcatcloud/browser-core'; import type { Configuration } from '../config'; -import { EventKind, type EventManager, LifecycleKind, type SessionRenewEvent } from '../event'; +import { type AppMayExitEvent, EventKind, type EventManager, LifecycleKind, type SessionRenewEvent } from '../event'; import { displayWarn } from '../tools/display'; import { StateFile } from '../tools/StateFile'; -import { isConfigurationVersion, type SamplingConfiguration } from './session'; +import { isConfigurationVersion, type SamplingConfiguration, type SessionManager } from './session'; import { addError, setTimeout } from './telemetry'; export const REMOTE_CONFIGURATION_FILE_NAME = '_fc_remote_config'; @@ -26,6 +26,14 @@ export const REQUEST_TIMEOUT = 10 * ONE_SECOND; * endpoint incident into a storm. */ export const RETRY_DELAYS = [5 * ONE_SECOND, 60 * ONE_SECOND]; +/** + * How long a configuration may go unrevalidated while the application runs, when the server does + * not say: the backend's own default `ttl`. A desktop session can live four hours, and an emergency + * stop must not wait for it to turn over. + */ +export const DEFAULT_TTL = 600 * ONE_SECOND; +/** The shortest revalidation interval honoured, whatever `ttl` the server sends. */ +export const MIN_TTL = 60 * ONE_SECOND; /** What the console asks a running client to do about the session it already has. */ export const Activation = { @@ -43,14 +51,27 @@ interface RemoteValues { /** The last configuration the server delivered and this SDK accepted. */ interface Delivered { - /** A publish counter that only goes up — a rollback is republished under a new number. */ + /** + * A publish counter that only goes up — a rollback is republished under a new number. 0 is what + * an application with nothing published answers: no version was delivered, and none is reported. + */ version: number; values: RemoteValues; /** The application's own pass-through bag, handed to it verbatim by `getRemoteConfig()`. */ custom?: Record; + /** + * What it asked of a session already running, kept so that a launch resuming a session the + * previous launch should have ended still ends it — see {@link RemoteConfiguration.start}. + */ + activation: string; } -interface StoredConfiguration extends Delivered { +/** What owns the sessions: the one that draws them, and is told when a change applies at once. */ +type SessionOwner = Pick; + +/** A file written before activation was kept has none: it is read as the next session. */ +interface StoredConfiguration extends Omit { + activation?: string; format: typeof FILE_FORMAT; /** * The SDK that wrote the file. Another version may have read the same response differently — a @@ -75,12 +96,13 @@ type RequestOutcome = 'done' | 'retry'; * for a change to apply at once (`activation: immediate`), the owner of the sessions is told and * decides whether the running session has to end — see `SessionManager.applySamplingChange`. * - * Fetching follows the sessions' rhythm, as in the other FlashCat SDKs: once at init and once - * whenever a new session starts — a change can only matter at a draw, and every draw is a new - * session. There is no timer between sessions; the server's `ttl` and `refresh_on_foreground` are - * read by no one here. Nothing waits on the network: the last good answer is read from disk at init, - * so the first session of a launch draws with it, and a request that fails, times out, or answers - * something unreadable leaves the settings in force exactly as they were. + * It asks at init, whenever a new session starts, and again every `ttl` the server gives (at least + * {@link MIN_TTL}, {@link DEFAULT_TTL} when it gives none) while the application runs: a desktop + * session can live four hours, and an emergency stop must not wait for it to turn over. There is + * never more than one request in flight, and `refresh_on_foreground` is not read. Nothing waits on + * the network: the last good answer is read from disk at init, so a session drawn before the server + * answers draws with it, and a request that fails, times out, or answers something unreadable + * leaves the settings in force exactly as they were. * * The server pairs `Cache-Control: no-cache` with an `ETag`. Node's `fetch` has no HTTP cache to * revalidate with, so the ETag is kept here with the values it describes and sent back as @@ -96,8 +118,12 @@ export class RemoteConfiguration { private inFlight: AbortController | undefined; private retryTimeoutId: ReturnType | undefined; private failedAttempts = 0; - private renewSubscription: Subscription | undefined; - private onImmediateChange: () => void = () => undefined; + private ttl = DEFAULT_TTL; + private ttlTimeoutId: ReturnType | undefined; + /** An accepted configuration whose asynchronous write has not landed yet. */ + private unlanded: StoredConfiguration | undefined; + private subscriptions: Subscription[] = []; + private sessions: SessionOwner | undefined; /** Said once per launch: the server keeps answering the same way at every new session. */ private warnedUnsupportedSchema = false; private stopped = false; @@ -121,10 +147,11 @@ export class RemoteConfiguration { */ getSampling(): SamplingConfiguration { const values = this.delivered?.values ?? {}; + const version = this.deliveredVersion(); return { sessionSampleRate: values.sessionSampleRate ?? this.config.sessionSampleRate, sessionOnError: values.sessionOnError ?? this.config.sessionOnError, - ...(this.delivered ? { rcVersion: this.delivered.version } : {}), + ...(version === undefined ? {} : { rcVersion: version }), }; } @@ -134,19 +161,41 @@ export class RemoteConfiguration { } /** - * Fetches now and at every new session. `onImmediateChange` is called when a newly published - * configuration asks to apply at once, after it is in force. + * Judges a session resumed from a previous launch by the configuration that launch kept, when it + * asked to apply at once and the session was drawn before it — under an older version, or none. + * That launch ended the session in memory, but may have ended itself before the session file was + * deleted. Called before anything is collected, so nothing of such a session is. + */ + applyKept(sessions: SessionOwner): void { + const kept = this.delivered; + if (kept?.activation === Activation.IMMEDIATE && (sessions.getSession().rcVersion ?? -1) < kept.version) { + sessions.applySamplingChange(); + } + } + + /** + * Starts keeping the configuration fresh, and tells `sessions` when a configuration that applies + * at once has changed what a draw would read. */ - start(eventManager: EventManager, onImmediateChange: () => void): void { + start(eventManager: EventManager, sessions: SessionOwner): void { if (!this.config.remoteConfigurationEnabled) { return; } - this.onImmediateChange = onImmediateChange; - this.renewSubscription = eventManager.registerHandler({ - canHandle: (event): event is SessionRenewEvent => - event.kind === EventKind.LIFECYCLE && event.lifecycle === LifecycleKind.SESSION_RENEW, - handle: () => this.trigger(), - }); + this.sessions = sessions; + this.subscriptions.push( + eventManager.registerHandler({ + canHandle: (event): event is SessionRenewEvent => + event.kind === EventKind.LIFECYCLE && event.lifecycle === LifecycleKind.SESSION_RENEW, + handle: () => this.trigger(), + }), + eventManager.registerHandler({ + canHandle: (event): event is AppMayExitEvent => + event.kind === EventKind.LIFECYCLE && event.lifecycle === LifecycleKind.APP_MAY_EXIT, + handle: () => this.writePendingSync(), + }) + ); + // `quit` rather than the quit events before it: those can be cancelled, and this cannot be undone. + app.on('quit', () => this.stop()); this.trigger(); } @@ -155,7 +204,27 @@ export class RemoteConfiguration { this.inFlight?.abort(); this.inFlight = undefined; clearTimeout(this.retryTimeoutId); - this.renewSubscription?.unsubscribe(); + clearTimeout(this.ttlTimeoutId); + this.subscriptions.forEach((subscription) => subscription.unsubscribe()); + this.subscriptions = []; + } + + /** + * Writes an accepted configuration whose write has not landed before returning, for a process + * that may be about to exit: an emergency stop the session already obeyed must still be in force + * at the next launch, offline included. A write requested earlier and landing later is discarded + * by the file, so this one is not overwritten by an older state. + */ + private writePendingSync(): void { + if (this.unlanded) { + this.stateFile.writeSync(JSON.stringify(this.unlanded)); + this.unlanded = undefined; + } + } + + /** The version delivered, or `undefined` when none was: version 0 means nothing is published. */ + private deliveredVersion(): number | undefined { + return this.delivered && this.delivered.version > 0 ? this.delivered.version : undefined; } private async restore(): Promise { @@ -170,8 +239,13 @@ export class RemoteConfiguration { if (stored === undefined || stored.identity !== this.identity) { return; } - const { version, values, custom } = stored; - this.delivered = { version, values, ...(custom ? { custom } : {}) }; + const { version, values, custom, activation } = stored; + this.delivered = { + version, + values, + ...(custom ? { custom } : {}), + activation: activation ?? Activation.NEXT_SESSION, + }; this.etag = stored.sdkVersion === __SDK_VERSION__ ? stored.etag : undefined; } @@ -205,9 +279,19 @@ export class RemoteConfiguration { if (outcome === 'retry') { this.scheduleRetry(); } + this.scheduleRevalidation(); }); } + /** One timer, restarted after every request, whatever started it and however it ended. */ + private scheduleRevalidation(): void { + clearTimeout(this.ttlTimeoutId); + this.ttlTimeoutId = setTimeout(() => { + this.ttlTimeoutId = undefined; + this.fetchNow(); + }, this.ttl); + } + private scheduleRetry(): void { if (this.failedAttempts >= RETRY_DELAYS.length) { return; @@ -246,27 +330,31 @@ export class RemoteConfiguration { return response.status === 429 || response.status >= 500 ? 'retry' : 'done'; } const parsed = parseResponse(body); - if (parsed === 'unsupported') { - if (!this.warnedUnsupportedSchema) { - this.warnedUnsupportedSchema = true; - displayWarn('Remote configuration ignored: the server answered with a schema this SDK cannot read.'); - } - return 'done'; - } if (parsed === undefined) { // A 200 is not proof the body came from the configuration endpoint: a captive portal or a // misrouted proxy answers 200 too. Storing that would blank the console's values. return 'retry'; } - this.apply(parsed, response.headers.get('etag') ?? undefined); + if ('unsupportedSchema' in parsed) { + if (!this.warnedUnsupportedSchema) { + this.warnedUnsupportedSchema = true; + displayWarn( + `Remote configuration ignored: the server answered with schema_version ${parsed.unsupportedSchema}, which this SDK (${__SDK_VERSION__}) cannot read. The settings already in force (init values or the last good configuration) still apply; upgrading the SDK is the fix.` + ); + } + return 'done'; + } + const { ttl, ...delivered } = parsed; + this.ttl = ttl; + this.apply(delivered, response.headers.get('etag') ?? undefined); return 'done'; } - private apply({ activation, ...delivered }: ParsedResponse, etag: string | undefined): void { - const heldVersion = this.delivered?.version; + private apply(delivered: Delivered, etag: string | undefined): void { + const held = this.delivered; // Settings only ever change under a higher number, so a lower one is an older answer arriving // late; applying it would put this client back on settings the console has already replaced. - if (heldVersion !== undefined && delivered.version < heldVersion) { + if (held !== undefined && delivered.version < held.version) { return; } this.delivered = delivered; @@ -278,14 +366,22 @@ export class RemoteConfiguration { ...delivered, ...(etag === undefined ? {} : { etag }), }; - void this.stateFile.write(() => JSON.stringify(stored)); - - // A repeat of the version already held is the ordinary answer, and it brings nothing new: the - // running session must hear about published changes only, or it would be re-judged at every - // renewal for nothing. - const isNew = heldVersion === undefined || delivered.version > heldVersion; - if (isNew && activation === Activation.IMMEDIATE) { - this.onImmediateChange(); + this.unlanded = stored; + void this.stateFile + .write(() => JSON.stringify(stored)) + .then(() => { + if (this.unlanded === stored) { + this.unlanded = undefined; + } + }); + + // The running session hears about a newly published version, and about the version it already + // holds when this SDK reads it differently from the one that kept it — after an upgrade, a knob + // the previous SDK did not read. A plain repeat, the ordinary answer, says nothing new; judging + // the session again on it would be harmless, but it is not news. + const isNew = held === undefined || delivered.version > held.version || !sameValues(held.values, delivered.values); + if (isNew && delivered.activation === Activation.IMMEDIATE) { + this.sessions?.applySamplingChange(); } } @@ -302,9 +398,11 @@ export class RemoteConfiguration { if (version) { parameters.push(`app_version=${encodeURIComponent(version)}`); } - // What lets the console tell how far a change has reached. 0 is a version like any other. - if (this.delivered) { - parameters.push(`applied_version=${this.delivered.version}`); + // What lets the console tell how far a change has reached. Not for version 0, which is no + // version: reporting it would count clients as running a configuration nobody published. + const appliedVersion = this.deliveredVersion(); + if (appliedVersion !== undefined) { + parameters.push(`applied_version=${appliedVersion}`); } // Behind a proxy, everything the intake has to see travels inside `ddforward`. const pathAndQuery = `${CONFIG_PATH}?${parameters.join('&')}`; @@ -313,11 +411,16 @@ export class RemoteConfiguration { } interface ParsedResponse extends Delivered { - activation: string; + /** How long until it is asked again, in milliseconds. */ + ttl: number; +} + +function sameValues(a: RemoteValues, b: RemoteValues): boolean { + return a.sessionSampleRate === b.sessionSampleRate && a.sessionOnError === b.sessionOnError; } /** - * The configuration `text` holds, `'unsupported'` for a schema this SDK does not know, or + * The configuration `text` holds, the schema version when it is one this SDK does not know, or * `undefined` for anything that is not recognisably a configuration response. * * Only the envelope decides that. A knob holding something that is not a rate or a switch is @@ -325,7 +428,7 @@ interface ParsedResponse extends Delivered { * response, so one bad value cannot switch every other knob back off. The values and `custom` are * withheld while the kill switch (`enabled`) is off; the version is kept either way. */ -function parseResponse(text: string): ParsedResponse | 'unsupported' | undefined { +function parseResponse(text: string): ParsedResponse | { unsupportedSchema: number } | undefined { let body: unknown; try { body = JSON.parse(text); @@ -336,9 +439,9 @@ function parseResponse(text: string): ParsedResponse | 'unsupported' | undefined return undefined; } if (body.schema_version !== SUPPORTED_SCHEMA_VERSION) { - return typeof body.schema_version === 'number' ? 'unsupported' : undefined; + return typeof body.schema_version === 'number' ? { unsupportedSchema: body.schema_version } : undefined; } - const { version, enabled, rum, custom, activation } = body; + const { version, enabled, rum, custom, activation, ttl } = body; if (!isConfigurationVersion(version) || typeof enabled !== 'boolean') { return undefined; } @@ -361,6 +464,7 @@ function parseResponse(text: string): ParsedResponse | 'unsupported' | undefined values, ...(enabled && isBag(custom) ? { custom } : {}), activation: typeof activation === 'string' ? activation : Activation.NEXT_SESSION, + ttl: isConfigurationVersion(ttl) && ttl > 0 ? Math.max(ttl * ONE_SECOND, MIN_TTL) : DEFAULT_TTL, }; } @@ -373,10 +477,13 @@ function parseStoredConfiguration(value: unknown): StoredConfiguration | undefin if (!isBag(value) || value.format !== FILE_FORMAT) { return undefined; } - const { sdkVersion, identity, version, values, custom, etag } = value; + const { sdkVersion, identity, version, values, custom, etag, activation } = value; if (typeof sdkVersion !== 'string' || typeof identity !== 'string' || !isConfigurationVersion(version)) { return undefined; } + if (activation !== undefined && typeof activation !== 'string') { + return undefined; + } if (!isBag(values) || (custom !== undefined && !isBag(custom)) || (etag !== undefined && typeof etag !== 'string')) { return undefined; } diff --git a/src/index.ts b/src/index.ts index 5c38edfe..d71b9d4e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -60,6 +60,8 @@ export async function init(configuration: InitConfiguration): Promise { remoteConfiguration = remote; const manager = await SessionManager.start(eventManager, hooks, () => remote.getSampling()); sessionManager = manager; + // Before anything is collected: a resumed session may have to end under what the previous launch kept. + remote.applyKept(manager); const rendererRegistry = new RendererRegistry(); const stackPathNormalizer = await StackPathNormalizer.create(config.normalizeStackPaths, config.normalizeStackPath); @@ -96,8 +98,9 @@ export async function init(configuration: InitConfiguration): Promise { const rum = await RumCollection.start(eventManager, hooks, rendererRegistry, stackPathNormalizer, manager); rumApi = rum.getApi(); - // Last, and never awaited: nothing in init waits on the network. - remote.start(eventManager, () => manager.applySamplingChange()); + // Last, once every collector listens for the session ending, and never awaited: nothing in init + // waits on the network. + remote.start(eventManager, manager); return true; } From bcb0012a43f75bc2ae0d76fd15c0bb62e82e4180 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 01:39:56 -0700 Subject: [PATCH 05/25] =?UTF-8?q?=E2=9C=85=20cover=20restarts,=20upgrades?= =?UTF-8?q?=20and=20ttl=20revalidation=20of=20remote=20configuration=20end?= =?UTF-8?q?=20to=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fake config endpoint answers like the real one for an application with nothing published (200, version 0, switched off). The request scenario waits for the configuration to be applied before renewing. --- e2e/app/src/main.ts | 3 + e2e/app/src/preload.ts | 1 + e2e/lib/helpers.ts | 2 +- e2e/lib/intake.ts | 26 ++-- e2e/lib/mainPage.ts | 6 + e2e/scenarios/remote-config.scenario.ts | 175 +++++++++++++++++++++++- 6 files changed, 197 insertions(+), 16 deletions(-) diff --git a/e2e/app/src/main.ts b/e2e/app/src/main.ts index 3390f30f..1082cdac 100644 --- a/e2e/app/src/main.ts +++ b/e2e/app/src/main.ts @@ -12,6 +12,7 @@ import { setUser, getUser, clearUser, + getRemoteConfig, startOperation, succeedOperation, failOperation, @@ -104,6 +105,8 @@ void app.whenReady().then(async () => { ipcMain.handle('getUser', () => getUser()); + ipcMain.handle('getRemoteConfig', () => getRemoteConfig()); + ipcMain.handle('clearUser', () => { clearUser(); }); diff --git a/e2e/app/src/preload.ts b/e2e/app/src/preload.ts index 0ec00718..c078ed8d 100644 --- a/e2e/app/src/preload.ts +++ b/e2e/app/src/preload.ts @@ -11,6 +11,7 @@ contextBridge.exposeInMainWorld('electronAPI', { setUser: (user: Record) => ipcRenderer.invoke('setUser', user), getUser: () => ipcRenderer.invoke('getUser'), clearUser: () => ipcRenderer.invoke('clearUser'), + getRemoteConfig: () => ipcRenderer.invoke('getRemoteConfig'), generateUncaughtException: () => ipcRenderer.invoke('generateUncaughtException'), generateUncaughtExceptionAndExit: () => ipcRenderer.invoke('generateUncaughtExceptionAndExit'), generateManualErrorAndExit: () => ipcRenderer.invoke('generateManualErrorAndExit'), diff --git a/e2e/lib/helpers.ts b/e2e/lib/helpers.ts index 725b09cf..7e629bdc 100644 --- a/e2e/lib/helpers.ts +++ b/e2e/lib/helpers.ts @@ -51,7 +51,7 @@ export interface TestFixtures { rumBrowserSdk: Record | null; /** Merged over the SDK configuration every scenario starts from. */ sdkConfig: Partial; - /** What the fake config endpoint answers from the first request on. Unset: 404. */ + /** What the fake config endpoint answers from the first request on. Unset: nothing published. */ remoteConfig: RemoteConfigBehaviour | undefined; } diff --git a/e2e/lib/intake.ts b/e2e/lib/intake.ts index 118c0c52..5e3185a3 100644 --- a/e2e/lib/intake.ts +++ b/e2e/lib/intake.ts @@ -21,8 +21,8 @@ import * as http from 'node:http'; * It also stands in for the remote configuration endpoint, `GET /api/v2/rum/config`, so remote * configuration is exercised without ever pointing the SDK at a real backend — a fake configuration * version reported to a real one would show up in the console's version statistics. What it answers - * is set per test with {@link Intake.setRemoteConfig}; with nothing set it answers 404, as for an - * application that has none. + * is set per test with {@link Intake.setRemoteConfig}; with nothing set it answers what the real + * endpoint answers for an application with nothing published: 200, version 0, switched off. */ export interface ReceivedEvent { timestamp: number; @@ -52,6 +52,19 @@ export type RemoteConfigBehaviour = /** The connection is dropped without an answer: what an offline client sees. */ | { reset: true }; +/** What the real endpoint answers for an application whose configuration was never published. */ +const NOTHING_PUBLISHED: RemoteConfigBehaviour = { + body: { + schema_version: 1, + version: 0, + ttl: 600, + enabled: false, + activation: 'next_session', + refresh_on_foreground: false, + rum: {}, + }, +}; + /** A request the SDK made to the config endpoint, as the real endpoint would have read it. */ export interface ConfigRequest { timestamp: number; @@ -277,7 +290,7 @@ export class Intake { } } - /** Sets what the config endpoint answers from now on; `undefined` for a 404. */ + /** Sets what the config endpoint answers from now on; `undefined` for an application with nothing published. */ setRemoteConfig(behaviour: RemoteConfigBehaviour | undefined): void { this.remoteConfig = behaviour; } @@ -310,11 +323,8 @@ export class Intake { headers, }); - const behaviour = this.remoteConfig; - if (behaviour === undefined) { - res.writeHead(404); - res.end(); - } else if ('reset' in behaviour) { + const behaviour = this.remoteConfig ?? NOTHING_PUBLISHED; + if ('reset' in behaviour) { req.socket.destroy(); } else if ('status' in behaviour) { res.writeHead(behaviour.status); diff --git a/e2e/lib/mainPage.ts b/e2e/lib/mainPage.ts index bf288f40..2908c4b0 100644 --- a/e2e/lib/mainPage.ts +++ b/e2e/lib/mainPage.ts @@ -10,6 +10,7 @@ interface ElectronAppWindow { setUser: (user: User) => Promise; getUser: () => Promise; clearUser: () => Promise; + getRemoteConfig: () => Promise | undefined>; startOperation: (name: string, options?: FeatureOperationOptions) => Promise; succeedOperation: (name: string, options?: FeatureOperationOptions) => Promise; failOperation: (name: string, failureReason: FailureReason, options?: FeatureOperationOptions) => Promise; @@ -117,6 +118,11 @@ export class MainPage { return this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.getUser()); } + /** The `custom` values the main process holds: undefined until a configuration carrying them is applied. */ + async getRemoteConfig(): Promise | undefined> { + return this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.getRemoteConfig()); + } + async clearUser() { await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.clearUser()); } diff --git a/e2e/scenarios/remote-config.scenario.ts b/e2e/scenarios/remote-config.scenario.ts index 6d8a9168..2f17bcd6 100644 --- a/e2e/scenarios/remote-config.scenario.ts +++ b/e2e/scenarios/remote-config.scenario.ts @@ -1,4 +1,4 @@ -import { mkdir, rm, stat, writeFile } from 'node:fs/promises'; +import { mkdir, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import type { InitConfiguration } from '@flashcatcloud/electron-sdk'; import { test, expect, launchAppManually, createUserDataDir, cleanupUserDataDir } from '../lib/helpers'; @@ -31,13 +31,14 @@ interface SessionEvent { function configuration( version: number, activation: 'immediate' | 'next_session', - rum: { sessionSampleRate?: number; sessionOnError?: boolean } + rum: { sessionSampleRate?: number; sessionOnError?: boolean }, + ttl = 600 ) { return { body: { schema_version: 1, version, - ttl: 600, + ttl, enabled: true, activation, refresh_on_foreground: false, @@ -60,6 +61,20 @@ async function settle(mainPage: MainPage, waitMs = RELEASE_WAIT) { await mainPage.flushTransport(); } +/** + * Waits until the main process holds the answer — its `custom` values are the marker every test + * configuration carries — so that what follows cannot race the request still in flight. + */ +async function waitForApplied(mainPage: MainPage, timeout = 10_000) { + const deadline = Date.now() + timeout; + while ((await mainPage.getRemoteConfig()) === undefined) { + if (Date.now() >= deadline) { + throw new Error(`No configuration applied within ${timeout}ms`); + } + await new Promise((resolve) => setTimeout(resolve, 50)); + } +} + /** Waits for the first answer to have been applied, and for the bridge to have heard of it. */ async function waitForConfiguration(intake: Intake) { await intake.waitForConfigRequests(1); @@ -104,6 +119,8 @@ test.describe('remote configuration', () => { app_version: '1.0.0', }); + await waitForApplied(mainPage); + expect(await mainPage.getRemoteConfig()).toEqual({ scenario: 'e2e' }); await mainPage.renewSession(); const [, second] = await intake.waitForConfigRequests(2); expect(second.params.applied_version).toBe('1'); @@ -132,7 +149,6 @@ test.describe('remote configuration', () => { test('leaves the on-error session running, and its error releases what it held', async ({ intake, mainPage }) => { await waitForConfiguration(intake); - const configuredAt = intake.getConfigRequests()[0].timestamp; const sessionId = await mainPage.getBridgeSessionId(); expect(sessionId).not.toBe(''); await settle(mainPage, 0); @@ -144,9 +160,11 @@ test.describe('remote configuration', () => { const events = rumEvents(intake); expect(events.some((event) => event.type === 'error')).toBe(true); expect(new Set(events.map((event) => event.session.id))).toEqual(new Set([sessionId])); - // The view the session opened with, at init — before the configuration was even asked for. + // The session the launch opened with: its view predates the request, and no other session was + // ever drawn, or it would have asked again. + expect(intake.getConfigRequests()).toHaveLength(1); const views = events.filter((event) => event.type === 'view'); - expect(Math.min(...views.map((view) => view.date))).toBeLessThan(configuredAt); + expect(Math.min(...views.map((view) => view.date))).toBeLessThan(intake.getConfigRequests()[0].timestamp); for (const view of views) { expect(view.session.sampled_for_error).toBe(true); } @@ -224,6 +242,33 @@ test.describe('remote configuration', () => { }); }); + test.describe('an emergency stop published while a session runs (ttl revalidation)', () => { + test.use({ + sdkConfig: { remoteConfigurationEnabled: true }, + // 1 s asks for more than the SDK allows: it revalidates after its 60 s floor. + remoteConfig: configuration(1, 'next_session', {}, 1), + }); + + test('reaches the running session at the next revalidation, with no new session to carry it', async ({ + intake, + mainPage, + }) => { + test.setTimeout(120_000); + await waitForApplied(mainPage); + expect(await mainPage.getBridgeSessionId()).not.toBe(''); + + intake.setRemoteConfig(configuration(2, 'immediate', { sessionSampleRate: 0 })); + const [first, second] = await intake.waitForConfigRequests(2, 90_000); + await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); + + expect(await mainPage.getBridgeSessionId()).toBe(''); + // The timer asked, not a session: nothing renewed in between, and no sooner than the floor. + expect(second.timestamp - first.timestamp).toBeGreaterThanOrEqual(59_000); + expect(second.params.applied_version).toBe('1'); + expect(second.headers['if-none-match']).toMatch(/^".+"$/); + }); + }); + test.describe('next session (deferred)', () => { test.use({ sdkConfig: { remoteConfigurationEnabled: true }, @@ -271,11 +316,19 @@ test.describe('remote configuration', () => { }); test.describe('next session without a configuration (control for deferred)', () => { + // Nothing published: the endpoint answers version 0, switched off. test.use({ sdkConfig: { remoteConfigurationEnabled: true } }); - test('draws the next session under the init values', async ({ intake, mainPage, testServer }) => { + test('draws the next session under the init values, and reports no version 0', async ({ + intake, + mainPage, + testServer, + }) => { await waitForConfiguration(intake); await mainPage.renewSession(); + const [, second] = await intake.waitForConfigRequests(2); + expect(second.params.applied_version).toBeUndefined(); + expect(second.headers['if-none-match']).toMatch(/^".+"$/); await mainPage.mainFetch(testServer.urlFor(200)); await settle(mainPage, 0); @@ -422,6 +475,114 @@ test.describe('remote configuration kept for the next launch', () => { }); }); +test.describe('remote configuration and a launch that ended before it was obeyed', () => { + test.use({ sdkConfig: { sessionSampleRate: 0 } }); + test.describe.configure({ timeout: 90_000 }); + + const ENABLED: Partial = { remoteConfigurationEnabled: true }; + const KEPT = '_fc_remote_config'; + + /** Launch one: receives `served`, keeps it on disk, and leaves its session to be resumed. */ + async function firstLaunch(intake: Intake, userDataDir: string, served: ReturnType) { + intake.setRemoteConfig(served); + const first = await launchAppManually(intake, userDataDir, 'await', ENABLED); + try { + await waitForApplied(first.mainPage); + await waitForFile(join(userDataDir, KEPT)); + return await first.mainPage.getBridgeSessionId(); + } finally { + await first.electronApp.close(); + intake.clear(); + } + } + + /** Rewrites the kept configuration as a launch that ended mid-way could have left it. */ + async function editKept(userDataDir: string, edit: Record) { + const filePath = join(userDataDir, KEPT); + const kept = JSON.parse(await readFile(filePath, 'utf8')) as Record; + await writeFile(filePath, JSON.stringify({ ...kept, ...edit })); + } + + for (const { title, activation, ends } of [ + { + title: 'ends a resumed session drawn before an immediate stop it kept, offline', + activation: 'immediate', + ends: true, + }, + { + title: 'leaves it when what it kept applies to the next session (control)', + activation: 'next_session', + ends: false, + }, + ]) { + test(title, async ({ intake }) => { + const userDataDir = await createUserDataDir(); + try { + const resumed = await firstLaunch(intake, userDataDir, configuration(1, 'next_session', {})); + expect(resumed).not.toBe(''); + // The stop arrived and was kept, but the session file outlived it. + await editKept(userDataDir, { version: 2, values: { sessionSampleRate: 0 }, activation }); + intake.setRemoteConfig({ reset: true }); + + const second = await launchAppManually(intake, userDataDir, 'await', ENABLED); + try { + expect(await second.mainPage.getBridgeSessionId()).toBe(ends ? '' : resumed); + } finally { + await second.electronApp.close(); + } + } finally { + await cleanupUserDataDir(userDataDir); + } + }); + } + + for (const { title, served, redrawn } of [ + { + title: 'redraws a session when the same version, read by this SDK, turns the switch on immediately', + served: configuration(7, 'immediate', { sessionSampleRate: 0, sessionOnError: true }), + redrawn: true, + }, + { + title: 'leaves it when the same version reads the same (control)', + served: configuration(7, 'immediate', { sessionSampleRate: 0 }), + redrawn: false, + }, + ]) { + test(title, async ({ intake }) => { + const userDataDir = await createUserDataDir(); + try { + await firstLaunch(intake, userDataDir, configuration(7, 'next_session', { sessionSampleRate: 0 })); + // As an SDK that did not read the switch would have kept version 7; and a fresh session to draw. + await editKept(userDataDir, { sdkVersion: 'older' }); + await rm(join(userDataDir, '_dd_s'), { force: true }); + intake.setRemoteConfig(served); + + const second = await launchAppManually(intake, userDataDir, 'await', ENABLED); + try { + await waitForApplied(second.mainPage); + await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); + // Drawn at rate 0 from what was kept: nothing collected. + expect(await second.mainPage.getBridgeSessionId()).toBe(''); + // Asked unconditionally: the ETag another SDK version kept is not sent. + expect(intake.getConfigRequests()[0].headers['if-none-match']).toBeUndefined(); + + await second.mainPage.generateActivity(); + const sessionId = await second.mainPage.getBridgeSessionId(); + if (redrawn) { + expect(sessionId).not.toBe(''); + } else { + expect(sessionId).toBe(''); + } + } finally { + await second.electronApp.close(); + } + } finally { + await cleanupUserDataDir(userDataDir); + } + }); + } +}); + async function waitForFile(filePath: string, timeout = 10_000) { const deadline = Date.now() + timeout; for (;;) { From bcfbf44819012b565ca61fc6f471b2bbee9bea43 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 01:39:56 -0700 Subject: [PATCH 06/25] =?UTF-8?q?=F0=9F=93=9D=20describe=20the=20remote=20?= =?UTF-8?q?configuration=20cadence,=20failures=20and=20resumed=20sessions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 27 +++++++++++++++++++-------- docs/ARCHITECTURE.md | 6 ++++-- docs/TESTING.md | 4 +++- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index b75f6ecb..dda80ead 100644 --- a/README.md +++ b/README.md @@ -466,15 +466,26 @@ await init({ - **Precedence.** A value the console sets takes precedence over the init value; a value it does not set leaves the init value in place. While the console's configuration is switched off, the init values apply. -- **When it is asked for.** By the main process, at `init` and whenever a new session starts — a - change can only matter at a draw, and every draw is a new session. A failed request (offline, - timeout, an error status, a body that is not a configuration) changes nothing and is retried after - about 5 s and 60 s, then not until the next session. Nothing in `init` waits for it. +- **When it is asked for.** By the main process: at `init`, whenever a new session starts, and again + every `ttl` the server gives while the application runs (10 minutes by default, never more often + than once a minute), revalidating with the ETag it holds. A desktop session can live four hours, so + this interval — not the session's end — is the longest a change, an emergency stop included, takes + to reach a running client. There is never more than one request in flight, and nothing in `init` + waits for one. +- **Failures.** Network failures, timeouts, malformed responses, HTTP 429 and 5xx responses change + nothing and are retried after about 5 s and 60 s, then at the next session or revalidation. Other + HTTP errors are not retried. Either way the configuration in force stays as it was. - **Kept on disk.** The last configuration accepted is kept in the application's `userData` - directory, so the next launch draws its first session with it before the network answers — or - without the network at all. It applies only to the same intake, application, `env` and `version`. - An SDK upgrade keeps the values but asks for the full configuration again rather than revalidating - the one the previous version read. + directory — written before returning when the application quits, if its write has not landed yet. + New sessions use it before the network answers, or without the network at all. A valid session + resumed from the previous launch keeps its original draw, subject to immediate activation: when + what was kept asks to apply immediately and is newer than what the session was drawn under, the + session is judged by it at startup, as below. It applies only to the same intake, application, + `env` and `version`. An SDK upgrade keeps the values but asks for the full configuration again + rather than revalidating the one the previous version read, and judges the running session again + if this version reads it differently. +- **Nothing published.** An application whose configuration was never published is answered with + version 0, switched off: the init values apply, and no version is reported. - **When a change applies.** A session's draw is locked for its whole life, so by default (`next_session`) a change applies from the next session on. When the console asks for a change to apply immediately, the running session is ended — and the next user activity draws a new one — diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 3103a5b3..ef8c9618 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -107,8 +107,10 @@ not drawn, and of a withheld session that ended without an error (its late event `session.sampled_for_error` and `_dd.configuration.session_sample_rate`. With `remoteConfigurationEnabled`, the sampling a draw reads comes from `RemoteConfiguration`: the -console's values over the init ones, fetched at init and at every new session, and kept on disk -(`_fc_remote_config`, replaced through `StateFile`) for the next launch's first draw. The session +console's values over the init ones, fetched at init, at every new session and every `ttl`, and kept +on disk (`_fc_remote_config`, replaced through `StateFile`, written synchronously on `APP_MAY_EXIT` +if its write has not landed) for later draws. At startup, a resumed session drawn before a kept +`immediate` configuration is judged by it before anything is collected. The session records the configuration version it was drawn under next to its rate, and `SessionContext` reports it as `_dd.configuration.rc_version`. A configuration published with `activation: immediate` asks `SessionManager.applySamplingChange` to end the running session where the new sampling is decisive diff --git a/docs/TESTING.md b/docs/TESTING.md index dbdc5243..32d5312f 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -70,7 +70,9 @@ event type, assert `intake.getProtocolViolations()` is empty. The fake intake also serves the remote configuration endpoint (`GET /api/v2/rum/config`): a test sets what it answers with the `remoteConfig` fixture option or `intake.setRemoteConfig()` — a body -(with an `ETag`, and a 304 when it is sent back), a status, or a dropped connection — and reads what +(with an `ETag`, and a 304 when it is sent back), a status, or a dropped connection; unset, it +answers like the real endpoint for an application with nothing published (200, version 0, switched +off) — and reads what the SDK asked with `intake.getConfigRequests()`. Remote configuration is never tested against a real backend: a fake configuration version reported there shows up in the console's version statistics. From 34b558f338de021273ddabbf4aab7863569eae08 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 02:08:01 -0700 Subject: [PATCH 07/25] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refresh=20remote=20c?= =?UTF-8?q?onfiguration=20on=20a=20return=20to=20the=20foreground,=20when?= =?UTF-8?q?=20the=20console=20allows=20it,=20instead=20of=20on=20a=20timer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fetching stays at init and at every new session, with no timer between sessions. When the response sets refresh_on_foreground, a window of the application gaining focus asks again once what is held is at least ttl old (60 s floor, 600 s default), conditionally and never alongside a request in flight. Both are kept in the cache file, so a client that settled on 304s keeps the permission across restarts. browser-window-focus is the foreground signal: it fires on every platform, unlike did-become-active, and means someone is back, unlike powerMonitor resume. The listeners are removed on stop and on quit. --- src/config.ts | 6 +- src/domain/RemoteConfiguration.spec.ts | 177 +++++++++++++++++++------ src/domain/RemoteConfiguration.ts | 101 +++++++++----- 3 files changed, 203 insertions(+), 81 deletions(-) diff --git a/src/config.ts b/src/config.ts index 3bcf6519..1a8a8b57 100644 --- a/src/config.ts +++ b/src/config.ts @@ -63,9 +63,9 @@ export interface InitConfiguration { * application, and deliver the application's own `custom` values (see `getRemoteConfig()`). * Defaults to `false`: nothing is requested and the init values apply. * - * The main process asks for the configuration at init, whenever a new session starts, and every - * `ttl` the server gives while the application runs, and keeps the last good answer on disk, so new - * sessions draw with it before the network answers. A delivered value takes precedence over the init value; a value the + * The main process asks for the configuration at init and whenever a new session starts — and, + * when the console allows it, when the user comes back to the application — and keeps the last + * good answer on disk, so new sessions draw with it before the network answers. A delivered value takes precedence over the init value; a value the * console did not set leaves the init value in place. A change applies to the next session, * unless the console asks for it to apply at once — see the README, Remote configuration. * diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts index a9ee7c1d..0c5264e3 100644 --- a/src/domain/RemoteConfiguration.spec.ts +++ b/src/domain/RemoteConfiguration.spec.ts @@ -12,6 +12,11 @@ vi.mock('electron', () => ({ app: { getPath: vi.fn(() => '/mock/user/data'), on: vi.fn((event: string, listener: () => void) => appListeners.set(event, listener)), + removeListener: vi.fn((event: string, listener: () => void) => { + if (appListeners.get(event) === listener) { + appListeners.delete(event); + } + }), }, })); vi.mock('../tools/display', () => ({ displayError: vi.fn(), displayWarn: vi.fn() })); @@ -361,6 +366,8 @@ describe('RemoteConfiguration', () => { values: { sessionSampleRate: 25, sessionOnError: true }, custom: { flag: 1 }, activation: 'next_session', + ttl: 600, + refreshOnForeground: false, etag: '"tag-3"', }, ]); @@ -649,101 +656,183 @@ describe('RemoteConfiguration', () => { }); }); - describe('revalidation while the application runs (ttl)', () => { - it('asks again, conditionally, once the ttl the server gave has passed', async () => { - fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl: 120 })))); + describe('refresh when the user comes back (refresh_on_foreground)', () => { + function focus() { + appListeners.get('browser-window-focus')?.(); + } + + it('asks nothing without a trigger, however long the application runs', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))) + ); await start(); - await vi.advanceTimersByTimeAsync(120_000 - 1); + await vi.advanceTimersByTimeAsync(DEFAULT_TTL * 10); + expect(fetchMock).toHaveBeenCalledTimes(1); - await vi.advanceTimersByTimeAsync(1); - expect(fetchMock).toHaveBeenCalledTimes(2); - expect(requestHeaders(1)).toEqual({ 'If-None-Match': '"tag-3"' }); + }); - // And again, ttl after that. + it('asks again, conditionally, on focus once what it holds is ttl old, when the operator allows it', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ ttl: 120, refresh_on_foreground: true }))) + ); + await start(); await vi.advanceTimersByTimeAsync(120_000); - expect(fetchMock).toHaveBeenCalledTimes(3); + + focus(); + await vi.advanceTimersByTimeAsync(0); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(requestHeaders(1)).toEqual({ 'If-None-Match': '"tag-3"' }); + expect(requestedUrl(1).searchParams.get('applied_version')).toBe('3'); }); - it.each([ - ['below the floor', 5, MIN_TTL], - ['missing', undefined, DEFAULT_TTL], - ['not a number', '120', DEFAULT_TTL], - ['zero', 0, DEFAULT_TTL], - ])('uses a sane interval for a ttl %s', async (_, ttl, expected) => { - fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl })))); + it('asks nothing on focus when the operator does not allow it', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ ttl: 120, refresh_on_foreground: false }))) + ); await start(); + await vi.advanceTimersByTimeAsync(DEFAULT_TTL * 2); + + focus(); + await vi.advanceTimersByTimeAsync(0); - await vi.advanceTimersByTimeAsync(expected - 1); expect(fetchMock).toHaveBeenCalledTimes(1); - await vi.advanceTimersByTimeAsync(1); - expect(fetchMock).toHaveBeenCalledTimes(2); }); - it('revalidates on the default interval when nothing was ever answered', async () => { - fetchMock.mockRejectedValue(new TypeError('fetch failed')); - vi.spyOn(Math, 'random').mockReturnValue(0.5); + it('asks nothing on focus before the ttl has passed', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ ttl: 120, refresh_on_foreground: true }))) + ); await start(); - await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] + RETRY_DELAYS[1]); - expect(fetchMock).toHaveBeenCalledTimes(3); + await vi.advanceTimersByTimeAsync(120_000 - 1); - await vi.advanceTimersByTimeAsync(DEFAULT_TTL); + focus(); + await vi.advanceTimersByTimeAsync(0); - expect(fetchMock).toHaveBeenCalledTimes(4); + expect(fetchMock).toHaveBeenCalledTimes(1); }); - it('restarts the interval after a fetch a new session made', async () => { - fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl: 120 })))); + it('measures the ttl from the last request that ended, including a new session one', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ ttl: 120, refresh_on_foreground: true }))) + ); await start(); await vi.advanceTimersByTimeAsync(100_000); - eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(100_000); + + focus(); await vi.advanceTimersByTimeAsync(0); expect(fetchMock).toHaveBeenCalledTimes(2); - await vi.advanceTimersByTimeAsync(120_000 - 1); - expect(fetchMock).toHaveBeenCalledTimes(2); - await vi.advanceTimersByTimeAsync(1); + await vi.advanceTimersByTimeAsync(20_000); + focus(); + await vi.advanceTimersByTimeAsync(0); expect(fetchMock).toHaveBeenCalledTimes(3); }); + it.each([ + ['below the floor', 5, MIN_TTL], + ['missing', undefined, DEFAULT_TTL], + ['not a number', '120', DEFAULT_TTL], + ['zero', 0, DEFAULT_TTL], + ])('uses a sane ttl when the one given is %s', async (_, ttl, expected) => { + fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl, refresh_on_foreground: true })))); + await start(); + + await vi.advanceTimersByTimeAsync(expected - 1); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(1); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + it('never has two requests in flight', async () => { - fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60 }))); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); fetchMock.mockReturnValue(new Promise(() => undefined)); await start(); - await vi.advanceTimersByTimeAsync(60_000); - expect(fetchMock).toHaveBeenCalledTimes(2); - await vi.advanceTimersByTimeAsync(REQUEST_TIMEOUT - 1); + + focus(); + focus(); eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); await vi.advanceTimersByTimeAsync(0); expect(fetchMock).toHaveBeenCalledTimes(2); }); - it('delivers an immediate stop to the running session at revalidation', async () => { - fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, version: 3 }))); + it('delivers an immediate stop to the running session', async () => { + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); await start(); - expect(onImmediateChange).not.toHaveBeenCalled(); - fetchMock.mockResolvedValueOnce( ok(configurationBody({ version: 4, activation: 'immediate', rum: { sessionSampleRate: 0 } }), '"tag-4"') ); await vi.advanceTimersByTimeAsync(60_000); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(onImmediateChange).toHaveBeenCalledTimes(1); expect(remote!.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: false, rcVersion: 4 }); }); - it('stops revalidating once stopped', async () => { - fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ ttl: 60 })))); - const configuration = await start(); + it('keeps the permission and the ttl across a restart, where only a 304 answers', async () => { + mfs.readFile.mockResolvedValue(storedFile({ ttl: 120, refreshOnForeground: true })); + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status: 304 }))); + await start(); - configuration.stop(); + await vi.advanceTimersByTimeAsync(120_000 - 1); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(1); + + await vi.advanceTimersByTimeAsync(1); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('reads a file kept without them as no permission', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status: 304 }))); + await start(); await vi.advanceTimersByTimeAsync(DEFAULT_TTL * 2); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(1); }); + + it.each([ + ['a ttl that is not a positive whole number', { ttl: -5 }], + ['a permission that is not a boolean', { refreshOnForeground: 'yes' }], + ])('ignores a file with %s', async (_, overrides) => { + mfs.readFile.mockResolvedValue(storedFile(overrides)); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + }); + + it('stops listening once stopped', async () => { + fetchMock.mockImplementation(() => + Promise.resolve(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))) + ); + const configuration = await start(); + expect(appListeners.has('browser-window-focus')).toBe(true); + + configuration.stop(); + + expect(appListeners.has('browser-window-focus')).toBe(false); + expect(appListeners.has('quit')).toBe(false); + }); }); describe('when the application exits', () => { diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts index 5f8b4943..ea61fd8f 100644 --- a/src/domain/RemoteConfiguration.ts +++ b/src/domain/RemoteConfiguration.ts @@ -27,12 +27,11 @@ export const REQUEST_TIMEOUT = 10 * ONE_SECOND; */ export const RETRY_DELAYS = [5 * ONE_SECOND, 60 * ONE_SECOND]; /** - * How long a configuration may go unrevalidated while the application runs, when the server does - * not say: the backend's own default `ttl`. A desktop session can live four hours, and an emergency - * stop must not wait for it to turn over. + * How old the configuration must be before a return to the foreground asks again, when the server + * gives no `ttl`: the backend's own default. */ export const DEFAULT_TTL = 600 * ONE_SECOND; -/** The shortest revalidation interval honoured, whatever `ttl` the server sends. */ +/** The shortest `ttl` honoured, whatever the server sends: focus can come back many times a minute. */ export const MIN_TTL = 60 * ONE_SECOND; /** What the console asks a running client to do about the session it already has. */ @@ -64,14 +63,26 @@ interface Delivered { * previous launch should have ended still ends it — see {@link RemoteConfiguration.start}. */ activation: string; + /** + * When to ask again, as the server described it — stored with the values because a 304 or a + * failure carries neither, and a client that settled on 304s must not forget its permission. + * `ttl` is in seconds, absent when the server gave none. + */ + ttl?: number; + /** + * Whether the operator allows asking again when the user comes back to the application. Off by + * default: every client of a fleet comes back at about the same time, a burst on the endpoint. + */ + refreshOnForeground: boolean; } /** What owns the sessions: the one that draws them, and is told when a change applies at once. */ type SessionOwner = Pick; -/** A file written before activation was kept has none: it is read as the next session. */ -interface StoredConfiguration extends Omit { +/** A file written before these were kept has none: the next session, and no foreground refresh. */ +interface StoredConfiguration extends Omit { activation?: string; + refreshOnForeground?: boolean; format: typeof FILE_FORMAT; /** * The SDK that wrote the file. Another version may have read the same response differently — a @@ -96,11 +107,11 @@ type RequestOutcome = 'done' | 'retry'; * for a change to apply at once (`activation: immediate`), the owner of the sessions is told and * decides whether the running session has to end — see `SessionManager.applySamplingChange`. * - * It asks at init, whenever a new session starts, and again every `ttl` the server gives (at least - * {@link MIN_TTL}, {@link DEFAULT_TTL} when it gives none) while the application runs: a desktop - * session can live four hours, and an emergency stop must not wait for it to turn over. There is - * never more than one request in flight, and `refresh_on_foreground` is not read. Nothing waits on - * the network: the last good answer is read from disk at init, so a session drawn before the server + * It asks at init and whenever a new session starts, as the other FlashCat SDKs do; there is no + * timer between sessions. When the operator allows it (`refresh_on_foreground`), it also asks when + * the user comes back to the application — a window of it gains focus — if what it holds is at least + * `ttl` old (at least {@link MIN_TTL}, {@link DEFAULT_TTL} when the server gives none). There is + * never more than one request in flight. Nothing waits on the network: the last good answer is read from disk at init, so a session drawn before the server * answers draws with it, and a request that fails, times out, or answers something unreadable * leaves the settings in force exactly as they were. * @@ -118,8 +129,8 @@ export class RemoteConfiguration { private inFlight: AbortController | undefined; private retryTimeoutId: ReturnType | undefined; private failedAttempts = 0; - private ttl = DEFAULT_TTL; - private ttlTimeoutId: ReturnType | undefined; + /** When the last request ended, however it ended: what a return to the foreground is gated on. */ + private lastFetchAt: number | undefined; /** An accepted configuration whose asynchronous write has not landed yet. */ private unlanded: StoredConfiguration | undefined; private subscriptions: Subscription[] = []; @@ -194,8 +205,13 @@ export class RemoteConfiguration { handle: () => this.writePendingSync(), }) ); + // Focus is how "the user came back" shows on a desktop: on every platform, unlike macOS's + // `did-become-active`, and unlike `powerMonitor`'s `resume`, which is the machine waking rather + // than anyone using the application — and which a focus follows when they do. Moving focus + // between the application's own windows also counts, and is absorbed by the ttl. + app.on('browser-window-focus', this.onForeground); // `quit` rather than the quit events before it: those can be cancelled, and this cannot be undone. - app.on('quit', () => this.stop()); + app.on('quit', this.onQuit); this.trigger(); } @@ -204,11 +220,26 @@ export class RemoteConfiguration { this.inFlight?.abort(); this.inFlight = undefined; clearTimeout(this.retryTimeoutId); - clearTimeout(this.ttlTimeoutId); this.subscriptions.forEach((subscription) => subscription.unsubscribe()); this.subscriptions = []; + app.removeListener('browser-window-focus', this.onForeground); + app.removeListener('quit', this.onQuit); } + private readonly onQuit = () => this.stop(); + + /** Asks again on a return to the foreground, if the operator allows it and what is held is stale. */ + private readonly onForeground = () => { + const held = this.delivered; + if (!held?.refreshOnForeground) { + return; + } + if (this.lastFetchAt !== undefined && Date.now() - this.lastFetchAt < ttlOf(held)) { + return; + } + this.trigger(); + }; + /** * Writes an accepted configuration whose write has not landed before returning, for a process * that may be about to exit: an emergency stop the session already obeyed must still be in force @@ -239,12 +270,14 @@ export class RemoteConfiguration { if (stored === undefined || stored.identity !== this.identity) { return; } - const { version, values, custom, activation } = stored; + const { version, values, custom, activation, ttl, refreshOnForeground } = stored; this.delivered = { version, values, ...(custom ? { custom } : {}), activation: activation ?? Activation.NEXT_SESSION, + ...(ttl === undefined ? {} : { ttl }), + refreshOnForeground: refreshOnForeground ?? false, }; this.etag = stored.sdkVersion === __SDK_VERSION__ ? stored.etag : undefined; } @@ -276,22 +309,13 @@ export class RemoteConfiguration { return; } this.inFlight = undefined; + this.lastFetchAt = Date.now(); if (outcome === 'retry') { this.scheduleRetry(); } - this.scheduleRevalidation(); }); } - /** One timer, restarted after every request, whatever started it and however it ended. */ - private scheduleRevalidation(): void { - clearTimeout(this.ttlTimeoutId); - this.ttlTimeoutId = setTimeout(() => { - this.ttlTimeoutId = undefined; - this.fetchNow(); - }, this.ttl); - } - private scheduleRetry(): void { if (this.failedAttempts >= RETRY_DELAYS.length) { return; @@ -344,9 +368,7 @@ export class RemoteConfiguration { } return 'done'; } - const { ttl, ...delivered } = parsed; - this.ttl = ttl; - this.apply(delivered, response.headers.get('etag') ?? undefined); + this.apply(parsed, response.headers.get('etag') ?? undefined); return 'done'; } @@ -410,9 +432,9 @@ export class RemoteConfiguration { } } -interface ParsedResponse extends Delivered { - /** How long until it is asked again, in milliseconds. */ - ttl: number; +/** How old what is held must be before a return to the foreground asks again, in milliseconds. */ +function ttlOf({ ttl }: Delivered): number { + return ttl === undefined ? DEFAULT_TTL : Math.max(ttl * ONE_SECOND, MIN_TTL); } function sameValues(a: RemoteValues, b: RemoteValues): boolean { @@ -428,7 +450,7 @@ function sameValues(a: RemoteValues, b: RemoteValues): boolean { * response, so one bad value cannot switch every other knob back off. The values and `custom` are * withheld while the kill switch (`enabled`) is off; the version is kept either way. */ -function parseResponse(text: string): ParsedResponse | { unsupportedSchema: number } | undefined { +function parseResponse(text: string): Delivered | { unsupportedSchema: number } | undefined { let body: unknown; try { body = JSON.parse(text); @@ -464,7 +486,8 @@ function parseResponse(text: string): ParsedResponse | { unsupportedSchema: numb values, ...(enabled && isBag(custom) ? { custom } : {}), activation: typeof activation === 'string' ? activation : Activation.NEXT_SESSION, - ttl: isConfigurationVersion(ttl) && ttl > 0 ? Math.max(ttl * ONE_SECOND, MIN_TTL) : DEFAULT_TTL, + ...(isPositiveInteger(ttl) ? { ttl } : {}), + refreshOnForeground: body.refresh_on_foreground === true, }; } @@ -484,6 +507,12 @@ function parseStoredConfiguration(value: unknown): StoredConfiguration | undefin if (activation !== undefined && typeof activation !== 'string') { return undefined; } + if (value.ttl !== undefined && !isPositiveInteger(value.ttl)) { + return undefined; + } + if (value.refreshOnForeground !== undefined && typeof value.refreshOnForeground !== 'boolean') { + return undefined; + } if (!isBag(values) || (custom !== undefined && !isBag(custom)) || (etag !== undefined && typeof etag !== 'string')) { return undefined; } @@ -497,6 +526,10 @@ function parseStoredConfiguration(value: unknown): StoredConfiguration | undefin return value as unknown as StoredConfiguration; } +function isPositiveInteger(value: unknown): value is number { + return isConfigurationVersion(value) && value > 0; +} + function isRate(value: unknown): value is number { return typeof value === 'number' && value >= 0 && value <= 100; } From 5e2a8c416e00b2fcbd90939d99e569d145dec0e5 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 02:08:01 -0700 Subject: [PATCH 08/25] =?UTF-8?q?=E2=9C=85=20cover=20the=20foreground=20re?= =?UTF-8?q?fresh=20of=20remote=20configuration=20end=20to=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An emergency stop reaches the running session on focus once the ttl has passed; no request is made when the console does not allow it, or before the ttl. --- e2e/scenarios/remote-config.scenario.ts | 84 ++++++++++++++++++------- 1 file changed, 61 insertions(+), 23 deletions(-) diff --git a/e2e/scenarios/remote-config.scenario.ts b/e2e/scenarios/remote-config.scenario.ts index 2f17bcd6..e1ca5c22 100644 --- a/e2e/scenarios/remote-config.scenario.ts +++ b/e2e/scenarios/remote-config.scenario.ts @@ -1,5 +1,6 @@ import { mkdir, readFile, rm, stat, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; +import type { ElectronApplication } from '@playwright/test'; import type { InitConfiguration } from '@flashcatcloud/electron-sdk'; import { test, expect, launchAppManually, createUserDataDir, cleanupUserDataDir } from '../lib/helpers'; import type { Intake } from '../lib/intake'; @@ -32,7 +33,8 @@ function configuration( version: number, activation: 'immediate' | 'next_session', rum: { sessionSampleRate?: number; sessionOnError?: boolean }, - ttl = 600 + ttl = 600, + refreshOnForeground = false ) { return { body: { @@ -41,7 +43,7 @@ function configuration( ttl, enabled: true, activation, - refresh_on_foreground: false, + refresh_on_foreground: refreshOnForeground, rum, custom: { scenario: 'e2e' }, }, @@ -242,31 +244,67 @@ test.describe('remote configuration', () => { }); }); - test.describe('an emergency stop published while a session runs (ttl revalidation)', () => { - test.use({ - sdkConfig: { remoteConfigurationEnabled: true }, - // 1 s asks for more than the SDK allows: it revalidates after its 60 s floor. - remoteConfig: configuration(1, 'next_session', {}, 1), - }); + test.describe('refresh when the user comes back (refresh_on_foreground)', () => { + test.use({ sdkConfig: { remoteConfigurationEnabled: true } }); - test('reaches the running session at the next revalidation, with no new session to carry it', async ({ - intake, - mainPage, - }) => { - test.setTimeout(120_000); - await waitForApplied(mainPage); - expect(await mainPage.getBridgeSessionId()).not.toBe(''); + /** + * The user coming back, as the SDK hears it. Emitted on `app` rather than produced by focusing a + * window: the test windows are hidden, and real focus is not reliable on a headless runner. + */ + async function focusApp(electronApp: ElectronApplication) { + await electronApp.evaluate(({ app, BrowserWindow }) => { + app.emit('browser-window-focus', {}, BrowserWindow.getAllWindows()[0]); + }); + } - intake.setRemoteConfig(configuration(2, 'immediate', { sessionSampleRate: 0 })); - const [first, second] = await intake.waitForConfigRequests(2, 90_000); + /** Publishes an emergency stop, then lets `waitMs` pass, then the user comes back. */ + async function stopThenFocus(intake: Intake, electronApp: ElectronApplication, waitMs: number) { + intake.setRemoteConfig(configuration(2, 'immediate', { sessionSampleRate: 0 }, 1, true)); + await new Promise((resolve) => setTimeout(resolve, waitMs)); + await focusApp(electronApp); await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); + } - expect(await mainPage.getBridgeSessionId()).toBe(''); - // The timer asked, not a session: nothing renewed in between, and no sooner than the floor. - expect(second.timestamp - first.timestamp).toBeGreaterThanOrEqual(59_000); - expect(second.params.applied_version).toBe('1'); - expect(second.headers['if-none-match']).toMatch(/^".+"$/); - }); + for (const { title, allowed, waitMs, refreshed } of [ + { + title: 'delivers an emergency stop to the running session once the ttl has passed', + allowed: true, + // 1 s asks for less than the SDK allows: it honours its 60 s floor. + waitMs: 61_000, + refreshed: true, + }, + { + title: 'asks nothing when the operator does not allow it (control)', + allowed: false, + waitMs: 61_000, + refreshed: false, + }, + { title: 'asks nothing before the ttl has passed (control)', allowed: true, waitMs: 1_000, refreshed: false }, + ]) { + test.describe(title, () => { + test.use({ remoteConfig: configuration(1, 'next_session', {}, 1, allowed) }); + + test('on focus', async ({ intake, mainPage, electronApp }) => { + test.setTimeout(120_000); + await waitForApplied(mainPage); + const sessionId = await mainPage.getBridgeSessionId(); + expect(sessionId).not.toBe(''); + + await stopThenFocus(intake, electronApp, waitMs); + + const requests = intake.getConfigRequests(); + if (refreshed) { + expect(requests).toHaveLength(2); + expect(requests[1].params.applied_version).toBe('1'); + expect(requests[1].headers['if-none-match']).toMatch(/^".+"$/); + expect(await mainPage.getBridgeSessionId()).toBe(''); + } else { + expect(requests).toHaveLength(1); + expect(await mainPage.getBridgeSessionId()).toBe(sessionId); + } + }); + }); + } }); test.describe('next session (deferred)', () => { From 25957b4593b518fef37f2873f85ade5572cad9c7 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 02:08:01 -0700 Subject: [PATCH 09/25] =?UTF-8?q?=F0=9F=93=9D=20describe=20the=20foregroun?= =?UTF-8?q?d=20refresh=20of=20remote=20configuration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 15 ++++++++------- docs/ARCHITECTURE.md | 3 ++- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index dda80ead..db8f26b9 100644 --- a/README.md +++ b/README.md @@ -466,14 +466,15 @@ await init({ - **Precedence.** A value the console sets takes precedence over the init value; a value it does not set leaves the init value in place. While the console's configuration is switched off, the init values apply. -- **When it is asked for.** By the main process: at `init`, whenever a new session starts, and again - every `ttl` the server gives while the application runs (10 minutes by default, never more often - than once a minute), revalidating with the ETag it holds. A desktop session can live four hours, so - this interval — not the session's end — is the longest a change, an emergency stop included, takes - to reach a running client. There is never more than one request in flight, and nothing in `init` - waits for one. +- **When it is asked for.** By the main process, at `init` and whenever a new session starts — there + is no timer between sessions. When the console allows it (`refresh_on_foreground`, off by + default), also when the user comes back to the application — a window of it gains focus — and + what it holds is at least the server's `ttl` old (10 minutes by default, never less than one + minute), revalidating with the ETag it holds. Without that permission, a session that never goes + idle keeps the configuration it has until it turns over, after up to four hours. There is never + more than one request in flight, and nothing in `init` waits for one. - **Failures.** Network failures, timeouts, malformed responses, HTTP 429 and 5xx responses change - nothing and are retried after about 5 s and 60 s, then at the next session or revalidation. Other + nothing and are retried after about 5 s and 60 s, then at the next session or foreground refresh. Other HTTP errors are not retried. Either way the configuration in force stays as it was. - **Kept on disk.** The last configuration accepted is kept in the application's `userData` directory — written before returning when the application quits, if its write has not landed yet. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index ef8c9618..b6d4e992 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -107,7 +107,8 @@ not drawn, and of a withheld session that ended without an error (its late event `session.sampled_for_error` and `_dd.configuration.session_sample_rate`. With `remoteConfigurationEnabled`, the sampling a draw reads comes from `RemoteConfiguration`: the -console's values over the init ones, fetched at init, at every new session and every `ttl`, and kept +console's values over the init ones, fetched at init, at every new session and — when the console allows it — on a return to the +foreground once the `ttl` has passed, and kept on disk (`_fc_remote_config`, replaced through `StateFile`, written synchronously on `APP_MAY_EXIT` if its write has not landed) for later draws. At startup, a resumed session drawn before a kept `immediate` configuration is judged by it before anything is collected. The session From 4552760a82d22cf4b7abf852e4b7a8e7e5f711ef Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 05:46:56 -0700 Subject: [PATCH 10/25] =?UTF-8?q?=F0=9F=90=9B=20keep=20an=20accepted=20rem?= =?UTF-8?q?ote=20configuration=20until=20it=20is=20on=20disk,=20and=20read?= =?UTF-8?q?=20what=20it=20carries=20safely?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - StateFile writes answer whether they landed; the configuration stays pending after a failed or superseded write and is written at exit, and stopping on quit writes it first, which covers app.exit() and a fetch that lands after will-quit. - At startup a kept immediate configuration always judges the session the launch starts with; the decision tree is idempotent, so a session drawn under the same version read differently is caught too. - custom is copied with structuredClone: a key such as __proto__ stays a plain key and reaches no prototype. - A foreground refresh leaves a pending retry to ask and does not re-arm the backoff. - A version 0 answer is no configuration, whatever it carries. - The unsupported-schema warning and the invalid-option error say what stays in effect. --- src/config.spec.ts | 2 +- src/config.ts | 12 ++- src/domain/RemoteConfiguration.spec.ts | 142 ++++++++++++++++++++++++- src/domain/RemoteConfiguration.ts | 54 ++++++---- src/tools/StateFile.spec.ts | 22 ++++ src/tools/StateFile.ts | 47 +++++--- 6 files changed, 237 insertions(+), 42 deletions(-) diff --git a/src/config.spec.ts b/src/config.spec.ts index 6c638095..bd02fd45 100644 --- a/src/config.spec.ts +++ b/src/config.spec.ts @@ -529,7 +529,7 @@ describe('buildConfiguration', () => { expect(buildConfiguration(config)?.remoteConfigurationEnabled).toBe(false); expect(display.displayError).toHaveBeenCalledWith( - "Configuration error: 'remoteConfigurationEnabled' must be a boolean" + "Configuration error: 'remoteConfigurationEnabled' must be a boolean; remote configuration remains disabled" ); }); }); diff --git a/src/config.ts b/src/config.ts index 1a8a8b57..10ccf66b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -261,11 +261,13 @@ export function buildConfiguration(initConfig: InitConfiguration): Configuration const proxy = validateOptionalString(initConfig.proxy); const sessionSampleRate = initConfig.sessionSampleRate ?? 100; const sessionOnError = validateOptionalBoolean(initConfig.sessionOnError, 'sessionOnError', false); - const remoteConfigurationEnabled = validateOptionalBoolean( - initConfig.remoteConfigurationEnabled, - 'remoteConfigurationEnabled', - false - ); + const remoteOption: unknown = initConfig.remoteConfigurationEnabled; + if (remoteOption !== undefined && remoteOption !== null && typeof remoteOption !== 'boolean') { + displayError( + "Configuration error: 'remoteConfigurationEnabled' must be a boolean; remote configuration remains disabled" + ); + } + const remoteConfigurationEnabled = remoteOption === true; // Not with remote configuration: the console may lower the rate the switch then applies to. if (sessionOnError && sessionSampleRate === 100 && !remoteConfigurationEnabled) { displayWarn( diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts index 0c5264e3..a8840914 100644 --- a/src/domain/RemoteConfiguration.spec.ts +++ b/src/domain/RemoteConfiguration.spec.ts @@ -290,6 +290,40 @@ describe('RemoteConfiguration', () => { expect(configuration.getCustom()).toEqual({ allowList: ['a', 'b'], level: 2 }); }); + it('hands over keys like __proto__ as plain keys, without touching any prototype, from the network', async () => { + const raw = + '{"schema_version":1,"version":3,"enabled":true,"activation":"next_session","rum":{},' + + '"custom":{"__proto__":{"polluted":true},"nested":{"list":[{"__proto__":{"polluted":true}}],"constructor":{"prototype":{"polluted":true}}}}}'; + fetchMock.mockResolvedValueOnce(ok(raw)); + const configuration = await start(); + + const custom = configuration.getCustom()!; + + expect(({} as Record).polluted).toBeUndefined(); + expect(Object.getPrototypeOf(custom)).toBe(Object.prototype); + expect(Object.keys(custom)).toEqual(['__proto__', 'nested']); + expect(Object.getOwnPropertyDescriptor(custom, '__proto__')?.value).toEqual({ polluted: true }); + const nested = custom.nested as { list: object[] }; + expect(Object.keys(nested.list[0])).toEqual(['__proto__']); + }); + + it('hands over keys like __proto__ as plain keys, without touching any prototype, from disk', async () => { + mfs.readFile.mockResolvedValue( + storedFile().replace( + '"custom":{"flag":"cached"}', + '"custom":{"__proto__":{"polluted":true},"a":{"__proto__":{"polluted":true}}}' + ) + ); + fetchMock.mockReturnValue(new Promise(() => undefined)); + const configuration = await start(); + + const custom = configuration.getCustom()!; + + expect(({} as Record).polluted).toBeUndefined(); + expect(Object.keys(custom)).toEqual(['__proto__', 'a']); + expect(Object.keys(custom.a as object)).toEqual(['__proto__']); + }); + it('drops a custom bag that is not an object', async () => { fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ custom: ['a'] })))); @@ -640,6 +674,19 @@ describe('RemoteConfiguration', () => { }); describe('version 0: an application with nothing published', () => { + it('applies nothing a version 0 answer carries: it is no configuration', async () => { + fetchMock.mockImplementation(() => + Promise.resolve( + ok(configurationBody({ version: 0, enabled: true, rum: { sessionSampleRate: 0 }, custom: { a: 1 } })) + ) + ); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(configuration.getCustom()).toBeUndefined(); + }); + it('reports no version for what it draws, and asks again without one', async () => { fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ version: 0, enabled: false, rum: {} }))) @@ -766,6 +813,38 @@ describe('RemoteConfiguration', () => { expect(fetchMock).toHaveBeenCalledTimes(2); }); + it('neither cancels a pending retry nor re-arms the backoff, so focus cannot outpace it', async () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + await start(); + await vi.advanceTimersByTimeAsync(60_000); + + // A refresh that fails: a retry is now pending 5 s out. + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(2); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(2); + + // The two retries, then silence however often focus comes back within the ttl. + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] + RETRY_DELAYS[1]); + expect(fetchMock).toHaveBeenCalledTimes(4); + for (let i = 0; i < 5; i += 1) { + await vi.advanceTimersByTimeAsync(10_000); + focus(); + } + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(4); + + // Once the ttl has passed, one more ask, and no fresh round of retries behind it. + await vi.advanceTimersByTimeAsync(10_000); + focus(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] + RETRY_DELAYS[1]); + expect(fetchMock).toHaveBeenCalledTimes(5); + }); + it('delivers an immediate stop to the running session', async () => { fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); await start(); @@ -869,6 +948,46 @@ describe('RemoteConfiguration', () => { expect(remote.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: false, rcVersion: 9 }); }); + it('writes an accepted configuration whose write has not landed when the application quits, even past every exit notice', async () => { + // `app.exit()` skips the quit events but reaches `quit`; a fetch can also land after `will-quit`. + mfs.writeFile.mockReturnValue(new Promise(() => undefined)); + fetchMock.mockResolvedValueOnce( + ok(configurationBody({ version: 9, activation: 'immediate', rum: { sessionSampleRate: 0 } }), '"tag-9"') + ); + await start(); + + appListeners.get('quit')!(); + + expect(syncWrites()).toEqual([expect.objectContaining({ version: 9 })]); + // Nothing left to write: the notice the process exit sends finds nothing more. + mayExit(); + expect(syncWrites()).toHaveLength(1); + }); + + it('still writes at exit a configuration whose asynchronous write failed', async () => { + mfs.writeFile.mockRejectedValue(Object.assign(new Error('ENOSPC'), { code: 'ENOSPC' })); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ version: 9 }), '"tag-9"')); + await start(); + + mayExit(); + + expect(syncWrites()).toEqual([expect.objectContaining({ version: 9 })]); + }); + + it('keeps it pending when the write at exit fails too, and tries again at the next notice', async () => { + mfs.writeFile.mockReturnValue(new Promise(() => undefined)); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ version: 9 }), '"tag-9"')); + await start(); + writeFileSync.mockImplementationOnce(() => { + throw new Error('EACCES'); + }); + + mayExit(); + mayExit(); + + expect(writeFileSync).toHaveBeenCalledTimes(2); + }); + it('writes nothing when what it holds has landed already', async () => { fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody()))); await start(); @@ -953,8 +1072,27 @@ describe('RemoteConfiguration', () => { expect(sessions.getSession().status).toBe('active'); }); - it('leaves it when it was drawn under that very version', async () => { - const sessions = await restart({ trackingType: TrackingType.TRACKED, sampleRate: 100, rcVersion: 9 }, STOP); + it('leaves it when it was drawn under that very version, as this SDK reads it', async () => { + const sessions = await restart({ trackingType: TrackingType.NOT_TRACKED, sampleRate: 0, rcVersion: 9 }, STOP); + + expect(sessions.getSession().status).toBe('active'); + }); + + it('ends it when it was drawn under the same version read differently, before an upgrade', async () => { + // Drawn at rate 0 by an SDK that did not read the switch; this one reads it on. + const sessions = await restart( + { trackingType: TrackingType.NOT_TRACKED, sampleRate: 0, rcVersion: 9 }, + { ...STOP, values: { sessionSampleRate: 0, sessionOnError: true } } + ); + + expect(sessions.getSession().status).toBe('expired'); + }); + + it('leaves a same-version session alone when what was kept applies to the next session', async () => { + const sessions = await restart( + { trackingType: TrackingType.NOT_TRACKED, sampleRate: 0, rcVersion: 9 }, + { ...STOP, values: { sessionSampleRate: 0, sessionOnError: true }, activation: 'next_session' } + ); expect(sessions.getSession().status).toBe('active'); }); diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts index ea61fd8f..8723e37d 100644 --- a/src/domain/RemoteConfiguration.ts +++ b/src/domain/RemoteConfiguration.ts @@ -1,7 +1,7 @@ import { app } from 'electron'; import * as fs from 'node:fs/promises'; import * as path from 'node:path'; -import { deepClone, ONE_SECOND, type Subscription } from '@flashcatcloud/browser-core'; +import { ONE_SECOND, type Subscription } from '@flashcatcloud/browser-core'; import type { Configuration } from '../config'; import { type AppMayExitEvent, EventKind, type EventManager, LifecycleKind, type SessionRenewEvent } from '../event'; import { displayWarn } from '../tools/display'; @@ -166,20 +166,27 @@ export class RemoteConfiguration { }; } - /** A copy of the application's `custom` values, or `undefined` when none were delivered. */ + /** + * A copy of the application's `custom` values, or `undefined` when none were delivered. + * + * `structuredClone` rather than a merging deep copy: these keys come from the network, and a key + * like `__proto__` must stay a plain key of the copy, not reach the prototype of every object in + * the main process. + */ getCustom(): Record | undefined { - return this.delivered?.custom && deepClone(this.delivered.custom); + return this.delivered?.custom && structuredClone(this.delivered.custom); } /** - * Judges a session resumed from a previous launch by the configuration that launch kept, when it - * asked to apply at once and the session was drawn before it — under an older version, or none. - * That launch ended the session in memory, but may have ended itself before the session file was - * deleted. Called before anything is collected, so nothing of such a session is. + * Judges the session the launch starts with by the configuration the previous launch kept, when + * it asked to apply at once. That launch may have ended a session in memory and ended itself + * before the session file was deleted; a session drawn under that very version, read differently + * by an SDK from before an upgrade, is the same case. The judgement ends nothing a draw under the + * configuration in force would keep, so it needs no version to compare. Called before anything is + * collected, so nothing of such a session is. */ applyKept(sessions: SessionOwner): void { - const kept = this.delivered; - if (kept?.activation === Activation.IMMEDIATE && (sessions.getSession().rcVersion ?? -1) < kept.version) { + if (this.delivered?.activation === Activation.IMMEDIATE) { sessions.applySamplingChange(); } } @@ -215,7 +222,9 @@ export class RemoteConfiguration { this.trigger(); } + /** Stops for good, writing first what has not landed: nothing after this point will. */ stop(): void { + this.writePendingSync(); this.stopped = true; this.inFlight?.abort(); this.inFlight = undefined; @@ -228,16 +237,20 @@ export class RemoteConfiguration { private readonly onQuit = () => this.stop(); - /** Asks again on a return to the foreground, if the operator allows it and what is held is stale. */ + /** + * Asks again on a return to the foreground, if the operator allows it and what is held is stale. + * It is not a natural trigger like a new session: a retry already pending is left to ask, and the + * backoff is not re-armed, so focus coming back again and again cannot outpace it. + */ private readonly onForeground = () => { const held = this.delivered; - if (!held?.refreshOnForeground) { + if (!held?.refreshOnForeground || this.retryTimeoutId !== undefined) { return; } if (this.lastFetchAt !== undefined && Date.now() - this.lastFetchAt < ttlOf(held)) { return; } - this.trigger(); + this.fetchNow(); }; /** @@ -247,8 +260,7 @@ export class RemoteConfiguration { * by the file, so this one is not overwritten by an older state. */ private writePendingSync(): void { - if (this.unlanded) { - this.stateFile.writeSync(JSON.stringify(this.unlanded)); + if (this.unlanded && this.stateFile.writeSync(JSON.stringify(this.unlanded))) { this.unlanded = undefined; } } @@ -363,7 +375,7 @@ export class RemoteConfiguration { if (!this.warnedUnsupportedSchema) { this.warnedUnsupportedSchema = true; displayWarn( - `Remote configuration ignored: the server answered with schema_version ${parsed.unsupportedSchema}, which this SDK (${__SDK_VERSION__}) cannot read. The settings already in force (init values or the last good configuration) still apply; upgrading the SDK is the fix.` + `Remote configuration ignored: the server answered with schema_version ${parsed.unsupportedSchema}, which this SDK (${__SDK_VERSION__}) cannot read. Check that the SDK and the server agree on the schema; the settings already in force (init values or the last good configuration) remain in effect.` ); } return 'done'; @@ -391,8 +403,9 @@ export class RemoteConfiguration { this.unlanded = stored; void this.stateFile .write(() => JSON.stringify(stored)) - .then(() => { - if (this.unlanded === stored) { + .then((landed) => { + // Only a write that landed takes it off the list: a failed one is tried again at exit. + if (landed && this.unlanded === stored) { this.unlanded = undefined; } }); @@ -471,8 +484,11 @@ function parseResponse(text: string): Delivered | { unsupportedSchema: number } if (rum !== undefined && rum !== null && !isBag(rum)) { return undefined; } + // Version 0 is what an application with nothing published answers: no configuration, whatever + // else the body carries. + const published = enabled && version > 0; const values: RemoteValues = {}; - if (enabled && isBag(rum)) { + if (published && isBag(rum)) { if (isRate(rum.sessionSampleRate)) { values.sessionSampleRate = rum.sessionSampleRate; } @@ -484,7 +500,7 @@ function parseResponse(text: string): Delivered | { unsupportedSchema: number } return { version, values, - ...(enabled && isBag(custom) ? { custom } : {}), + ...(published && isBag(custom) ? { custom } : {}), activation: typeof activation === 'string' ? activation : Activation.NEXT_SESSION, ...(isPositiveInteger(ttl) ? { ttl } : {}), refreshOnForeground: body.refresh_on_foreground === true, diff --git a/src/tools/StateFile.spec.ts b/src/tools/StateFile.spec.ts index a8d06a96..3b8b0e7c 100644 --- a/src/tools/StateFile.spec.ts +++ b/src/tools/StateFile.spec.ts @@ -106,6 +106,28 @@ describe('StateFile', () => { expect(renameSync).toHaveBeenCalledTimes(1); }); + it('answers whether each write landed: true when it did, false when it failed or was superseded', async () => { + const file = new StateFile(FILE_PATH, 'state'); + + expect(await file.write(() => 'landed')).toBe(true); + + mfs.writeFile.mockRejectedValueOnce(new Error('ENOSPC')); + expect(await file.write(() => 'failed')).toBe(false); + + let finishWrite!: () => void; + mfs.writeFile.mockImplementationOnce(() => new Promise((resolve) => (finishWrite = resolve))); + const superseded = file.write(() => 'old'); + await vi.advanceTimersByTimeAsync(0); + expect(file.writeSync('new')).toBe(true); + finishWrite(); + expect(await superseded).toBe(false); + + writeFileSync.mockImplementationOnce(() => { + throw new Error('EACCES'); + }); + expect(file.writeSync('failed')).toBe(false); + }); + it('sweeps the temp files an earlier launch left next to it', () => { readdirSync.mockReturnValue(['_dd_state', '_dd_state.123.4.tmp', '_dd_state.123.5.tmp', 'other.1.1.tmp']); diff --git a/src/tools/StateFile.ts b/src/tools/StateFile.ts index a392c129..741e9364 100644 --- a/src/tools/StateFile.ts +++ b/src/tools/StateFile.ts @@ -13,7 +13,9 @@ import { displayError } from './display'; * be pending, nor undone by a delete requested before it. * * Asynchronous writes serialize when they run, not when they are requested, so the last to land - * carries the latest state. A failure is reported and does not stop later writes. + * carries the latest state. A failure is reported and does not stop later writes. Every write + * answers whether it landed, so a caller holding state that must reach disk can tell a failed or + * superseded write from one that did. */ export class StateFile { private requested = 0; @@ -41,36 +43,45 @@ export class StateFile { } } - /** Queues a write of what `serialize` returns when the write runs, behind what was requested before. */ - write(serialize: () => string): Promise { + /** + * Queues a write of what `serialize` returns when the write runs, behind what was requested before. + * Resolves to whether this write landed: `false` when it failed or a later change landed first. + * Never rejects. + */ + write(serialize: () => string): Promise { const generation = this.nextGeneration(); - this.queue = this.queue + const written = this.queue .then(async () => { if (generation < this.landed) { // Superseded while queued: what landed since is newer than anything requested before it. - return; + return false; } const tmpPath = this.tempPath(generation); await fs.writeFile(tmpPath, serialize(), 'utf-8'); - this.land( + return this.land( generation, () => renameSync(tmpPath, this.filePath), () => unlinkSync(tmpPath) ); }) - .catch((error) => displayError(`Failed to write ${this.what}:`, error)); - return this.queue; + .catch((error) => { + displayError(`Failed to write ${this.what}:`, error); + return false; + }); + this.queue = written.then(() => undefined); + return written; } - /** Writes before returning, for a process that may be about to exit. */ - writeSync(content: string): void { + /** Writes before returning, for a process that may be about to exit. Answers whether it landed. */ + writeSync(content: string): boolean { const generation = this.nextGeneration(); const tmpPath = this.tempPath(generation); try { writeFileSync(tmpPath, content, 'utf-8'); - this.land(generation, () => renameSync(tmpPath, this.filePath)); + return this.land(generation, () => renameSync(tmpPath, this.filePath)); } catch (error) { displayError(`Failed to write ${this.what}:`, error); + return false; } } @@ -78,7 +89,9 @@ export class StateFile { delete(): Promise { const generation = this.nextGeneration(); this.queue = this.queue - .then(() => this.land(generation, () => this.unlink())) + .then(() => { + this.land(generation, () => this.unlink()); + }) .catch((error) => displayError(`Failed to delete ${this.what}:`, error)); return this.queue; } @@ -102,14 +115,18 @@ export class StateFile { return `${this.filePath}.${process.pid}.${generation}.tmp`; } - /** Applies the change unless a later one has landed already, in which case it is discarded. */ - private land(generation: number, apply: () => void, discard?: () => void): void { + /** + * Applies the change unless a later one has landed already, in which case it is discarded. + * Answers whether it was applied. + */ + private land(generation: number, apply: () => void, discard?: () => void): boolean { if (generation < this.landed) { discard?.(); - return; + return false; } apply(); this.landed = generation; + return true; } private unlink(): void { From 0845066ac5b87182116eb68ce0ef6aaa6637c8c7 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 05:46:56 -0700 Subject: [PATCH 11/25] =?UTF-8?q?=F0=9F=90=9B=20schedule=20nothing=20for?= =?UTF-8?q?=20a=20session=20that=20ended=20while=20it=20was=20being=20save?= =?UTF-8?q?d?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the awaited write in session creation, resumption and activity, check that the session is still the active one before scheduling timers or announcing a renewal, and keep a single four-hour timer, so a session ended by an immediate configuration mid-write leaves no timer that could end a later session early. --- src/domain/session/SessionManager.spec.ts | 53 +++++++++++++++++++++++ src/domain/session/SessionManager.ts | 42 ++++++++++++++---- 2 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/domain/session/SessionManager.spec.ts b/src/domain/session/SessionManager.spec.ts index 82b8bf2e..41b30bcc 100644 --- a/src/domain/session/SessionManager.spec.ts +++ b/src/domain/session/SessionManager.spec.ts @@ -678,6 +678,20 @@ describe('sessionManager', () => { lifecycleEvents.length = 0; } + /** Holds the next write of the session state file until the returned function is called. */ + function holdNextSessionStateWrite(): () => void { + let finish: () => void = () => undefined; + let held = false; + mfs.writeFile.mockImplementation((filePath: string) => { + if (!held && filePath.includes(`/${SESSION_FILE_NAME}.`)) { + held = true; + return new Promise((resolve) => (finish = resolve)); + } + return Promise.resolve(); + }); + return () => finish(); + } + function applyChange(next: SamplingConfiguration) { sampling = next; sessionManager.applySamplingChange(); @@ -786,6 +800,45 @@ describe('sessionManager', () => { expect(lifecycleEvents).toEqual(renewed); }); + it('neither schedules timers for nor announces a renewal of a session that ended while it was being saved', async () => { + await startSession({ sessionSampleRate: 100, sessionOnError: false }); + sessionManager.expire(); + lifecycleEvents.length = 0; + const finishWrite = holdNextSessionStateWrite(); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.END_USER_ACTIVITY }); + await vi.advanceTimersByTimeAsync(0); + // An immediate stop lands while the new session's state is being written. + applyChange({ sessionSampleRate: 0, sessionOnError: false }); + finishWrite(); + await vi.advanceTimersByTimeAsync(0); + + expect(lifecycleEvents).toEqual([LifecycleKind.END_USER_ACTIVITY, LifecycleKind.SESSION_EXPIRED]); + expect(sessionManager.getSession().status).toBe('expired'); + expect(vi.getTimerCount()).toBe(0); + }); + + it('leaves no timer behind that could end a later session early', async () => { + await startSession({ sessionSampleRate: 100, sessionOnError: false }); + sessionManager.expire(); + const finishWrite = holdNextSessionStateWrite(); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.END_USER_ACTIVITY }); + await vi.advanceTimersByTimeAsync(0); + applyChange({ sessionSampleRate: 0, sessionOnError: false }); + finishWrite(); + await vi.advanceTimersByTimeAsync(0); + + // A minute later, before any stale inactivity timer could fire, a kept session is renewed. + sampling = { sessionSampleRate: 100, sessionOnError: false }; + await vi.advanceTimersByTimeAsync(60_000); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.END_USER_ACTIVITY }); + await vi.advanceTimersByTimeAsync(0); + + expect(sessionManager.getSession().status).toBe('active'); + // Its own inactivity and four-hour timers, and nothing left over from the ended session. + expect(vi.getTimerCount()).toBe(2); + }); + it('does nothing to a session that has already ended', async () => { await startSession({ sessionSampleRate: 100, sessionOnError: false }); sessionManager.expire(); diff --git a/src/domain/session/SessionManager.ts b/src/domain/session/SessionManager.ts index 6029650a..52742156 100644 --- a/src/domain/session/SessionManager.ts +++ b/src/domain/session/SessionManager.ts @@ -102,8 +102,9 @@ export class SessionManager { * Only two cases are decisive: * - A session that was not collected and was drawn at rate 0 lost no lottery: nothing was ever * drawn for it. A rate leaving 0, or the on-error switch turning on, would now keep some such - * sessions, so it draws again. One that lost a draw at a real rate keeps its outcome, or the - * fleet would be re-rolled and a rate `p` would turn into `p²`. + * sessions, so it draws again. One that lost a draw at a real rate keeps its outcome: drawing + * it again would give it a second chance, and `n` independent draws at rate `p` keep a session + * with probability `1 − (1 − p)ⁿ`, more than the rate promises. * - A collected session meets the emergency stop, rate 0 — unless it is an on-error session and * the switch stays on: rate 0 next to the switch is that switch's ordinary setting, and ending * the session would throw away exactly the history the switch exists to keep. @@ -186,8 +187,10 @@ export class SessionManager { existingState.lastActivity = now; this.setCurrent(existingState); await this.saveCurrentState(); - this.scheduleInactivityTimeout(); - this.scheduleSessionTimeout(existingState.created); + if (this.isCurrent(existingState)) { + this.scheduleInactivityTimeout(); + this.scheduleSessionTimeout(existingState.created); + } } else { await this.createNewSession(); } @@ -201,7 +204,12 @@ export class SessionManager { }); } - private async createNewSession(): Promise { + /** + * Creates and saves a session, and answers whether it is still the active one once saved: a + * configuration that applies at once can end it while it is being written, and nothing may then + * be scheduled for it or announced about it. + */ + private async createNewSession(): Promise { const now = Date.now(); const sampling = this.getSampling(); const state: SessionState = { @@ -215,9 +223,18 @@ export class SessionManager { this.setCurrent(state); await this.saveCurrentState(); + if (!this.isCurrent(state)) { + return false; + } this.scheduleInactivityTimeout(); this.scheduleSessionTimeout(state.created); + return true; + } + + /** Whether `state` is still the session in force, and still active, after an `await`. */ + private isCurrent(state: SessionState): boolean { + return this.status === 'active' && this.currentState === state; } private setCurrent(state: SessionState): void { @@ -230,7 +247,7 @@ export class SessionManager { * Queue a write of the current state. Each write serializes the state as of when it runs, and they * run one after another, so the last one on disk is always the latest state. */ - private saveCurrentState(): Promise { + private saveCurrentState(): Promise { return this.stateFile.write(() => JSON.stringify(this.currentState)); } @@ -246,8 +263,9 @@ export class SessionManager { private async updateActivity(): Promise { if (this.isExpired()) { - await this.createNewSession(); - this.eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + if (await this.createNewSession()) { + this.eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + } return; } @@ -263,8 +281,12 @@ export class SessionManager { // Written from memory rather than from what was just read: the error mark reaches memory first // and disk afterwards, and writing the read state back could undo it. - this.currentState.lastActivity = Date.now(); + const current = this.currentState; + current.lastActivity = Date.now(); await this.saveCurrentState(); + if (!this.isCurrent(current)) { + return; + } this.scheduleInactivityTimeout(); } @@ -282,6 +304,8 @@ export class SessionManager { } private scheduleSessionTimeout(createdAt: number): void { + // One four-hour timer at a time: one left behind would end a later session early. + clearTimeout(this.sessionTimeoutId); const now = Date.now(); const remainingTime = SESSION_TIME_OUT_DELAY - (now - createdAt); if (remainingTime > 0) { From 6315f846d63a3df6c493b3d43cd2d457595fd462 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 05:46:56 -0700 Subject: [PATCH 12/25] =?UTF-8?q?=E2=9C=85=20wait=20for=20the=20answer=20o?= =?UTF-8?q?f=20each=20remote=20configuration=20scenario=20by=20a=20marker?= =?UTF-8?q?=20it=20carries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each served configuration carries its own marker in custom; the scenarios wait until the main process holds it instead of sleeping, and only the negative controls wait for an absence. --- e2e/scenarios/remote-config.scenario.ts | 69 +++++++++++++------------ 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/e2e/scenarios/remote-config.scenario.ts b/e2e/scenarios/remote-config.scenario.ts index e1ca5c22..800043ba 100644 --- a/e2e/scenarios/remote-config.scenario.ts +++ b/e2e/scenarios/remote-config.scenario.ts @@ -17,8 +17,11 @@ import type { MainPage } from '../lib/mainPage'; /** Comfortably past the release jitter of an on-error session, which is at most 3 s. */ const RELEASE_WAIT = 3_500; -/** Long enough for a configuration answer to be applied and the bridge to hear of a session change. */ -const APPLY_WAIT = 1_000; +/** + * How long a negative control watches for something that must not happen — a request, a session + * ending. Only an absence needs it: everything that does happen is waited for by what it shows. + */ +const ABSENCE_WAIT = 1_000; interface SessionEvent { type: string; @@ -34,7 +37,9 @@ function configuration( activation: 'immediate' | 'next_session', rum: { sessionSampleRate?: number; sessionOnError?: boolean }, ttl = 600, - refreshOnForeground = false + refreshOnForeground = false, + /** What tells this answer apart once applied: it rides in `custom`, which the app can read. */ + marker = `v${version}` ) { return { body: { @@ -45,7 +50,7 @@ function configuration( activation, refresh_on_foreground: refreshOnForeground, rum, - custom: { scenario: 'e2e' }, + custom: { scenario: 'e2e', marker }, }, }; } @@ -64,12 +69,14 @@ async function settle(mainPage: MainPage, waitMs = RELEASE_WAIT) { } /** - * Waits until the main process holds the answer — its `custom` values are the marker every test - * configuration carries — so that what follows cannot race the request still in flight. + * Waits until the main process holds the answer carrying `marker`, so that what follows cannot race + * the request still in flight — and not an earlier answer kept on disk. The answer travels back over + * the same IPC channel as the session pushes, behind them, so by the time it is seen the renderer + * has also heard of any session the answer ended. */ -async function waitForApplied(mainPage: MainPage, timeout = 10_000) { +async function waitForApplied(mainPage: MainPage, marker: string, timeout = 10_000) { const deadline = Date.now() + timeout; - while ((await mainPage.getRemoteConfig()) === undefined) { + while ((await mainPage.getRemoteConfig())?.marker !== marker) { if (Date.now() >= deadline) { throw new Error(`No configuration applied within ${timeout}ms`); } @@ -77,12 +84,6 @@ async function waitForApplied(mainPage: MainPage, timeout = 10_000) { } } -/** Waits for the first answer to have been applied, and for the bridge to have heard of it. */ -async function waitForConfiguration(intake: Intake) { - await intake.waitForConfigRequests(1); - await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); -} - test.afterEach(async ({ intake }, testInfo) => { const captureDir = process.env.FC_E2E_CAPTURE_DIR; if (!captureDir) { @@ -121,8 +122,8 @@ test.describe('remote configuration', () => { app_version: '1.0.0', }); - await waitForApplied(mainPage); - expect(await mainPage.getRemoteConfig()).toEqual({ scenario: 'e2e' }); + await waitForApplied(mainPage, 'v1'); + expect(await mainPage.getRemoteConfig()).toEqual({ scenario: 'e2e', marker: 'v1' }); await mainPage.renewSession(); const [, second] = await intake.waitForConfigRequests(2); expect(second.params.applied_version).toBe('1'); @@ -135,7 +136,7 @@ test.describe('remote configuration', () => { test.use({ remoteConfig: configuration(1, 'immediate', { sessionSampleRate: 0 }) }); test('asks nothing, and a rate of 0 published for it changes nothing', async ({ intake, mainPage }) => { - await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); + await new Promise((resolve) => setTimeout(resolve, ABSENCE_WAIT)); expect(intake.getConfigRequests()).toEqual([]); expect(await mainPage.getBridgeSessionId()).not.toBe(''); @@ -150,7 +151,7 @@ test.describe('remote configuration', () => { }); test('leaves the on-error session running, and its error releases what it held', async ({ intake, mainPage }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v1'); const sessionId = await mainPage.getBridgeSessionId(); expect(sessionId).not.toBe(''); await settle(mainPage, 0); @@ -186,7 +187,7 @@ test.describe('remote configuration', () => { }); test('ends the on-error session, and an error then releases nothing', async ({ intake, mainPage }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v1'); expect(await mainPage.getBridgeSessionId()).toBe(''); await mainPage.generateManualError(); @@ -202,7 +203,7 @@ test.describe('remote configuration', () => { }); test('ends the drawn session, and the next one is not collected', async ({ intake, mainPage, testServer }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v2'); expect(await mainPage.getBridgeSessionId()).toBe(''); await settle(mainPage, 0); const ended = rumEvents(intake).filter((event) => event.type === 'view'); @@ -228,7 +229,7 @@ test.describe('remote configuration', () => { }); test('leaves the drawn session collecting', async ({ intake, mainPage, testServer }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v2'); const sessionId = await mainPage.getBridgeSessionId(); expect(sessionId).not.toBe(''); @@ -262,7 +263,6 @@ test.describe('remote configuration', () => { intake.setRemoteConfig(configuration(2, 'immediate', { sessionSampleRate: 0 }, 1, true)); await new Promise((resolve) => setTimeout(resolve, waitMs)); await focusApp(electronApp); - await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); } for (const { title, allowed, waitMs, refreshed } of [ @@ -286,11 +286,16 @@ test.describe('remote configuration', () => { test('on focus', async ({ intake, mainPage, electronApp }) => { test.setTimeout(120_000); - await waitForApplied(mainPage); + await waitForApplied(mainPage, 'v1'); const sessionId = await mainPage.getBridgeSessionId(); expect(sessionId).not.toBe(''); await stopThenFocus(intake, electronApp, waitMs); + if (refreshed) { + await waitForApplied(mainPage, 'v2'); + } else { + await new Promise((resolve) => setTimeout(resolve, ABSENCE_WAIT)); + } const requests = intake.getConfigRequests(); if (refreshed) { @@ -318,7 +323,7 @@ test.describe('remote configuration', () => { mainPage, testServer, }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v3'); const current = await mainPage.getBridgeSessionId(); await mainPage.mainFetch(testServer.urlFor(200)); await settle(mainPage, 0); @@ -362,7 +367,7 @@ test.describe('remote configuration', () => { mainPage, testServer, }) => { - await waitForConfiguration(intake); + await intake.waitForConfigRequests(1); await mainPage.renewSession(); const [, second] = await intake.waitForConfigRequests(2); expect(second.params.applied_version).toBeUndefined(); @@ -386,7 +391,7 @@ test.describe('remote configuration', () => { }); test('ends the session so the next activity draws again, as an on-error session', async ({ intake, mainPage }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v4'); expect(await mainPage.getBridgeSessionId()).toBe(''); await mainPage.generateActivity(); @@ -415,7 +420,7 @@ test.describe('remote configuration', () => { }); test('keeps the session it has, which collects nothing', async ({ intake, mainPage }) => { - await waitForConfiguration(intake); + await waitForApplied(mainPage, 'v4'); await mainPage.generateActivity(); expect(await mainPage.getBridgeSessionId()).toBe(''); @@ -525,7 +530,7 @@ test.describe('remote configuration and a launch that ended before it was obeyed intake.setRemoteConfig(served); const first = await launchAppManually(intake, userDataDir, 'await', ENABLED); try { - await waitForApplied(first.mainPage); + await waitForApplied(first.mainPage, (served.body.custom as { marker: string }).marker); await waitForFile(join(userDataDir, KEPT)); return await first.mainPage.getBridgeSessionId(); } finally { @@ -577,12 +582,12 @@ test.describe('remote configuration and a launch that ended before it was obeyed for (const { title, served, redrawn } of [ { title: 'redraws a session when the same version, read by this SDK, turns the switch on immediately', - served: configuration(7, 'immediate', { sessionSampleRate: 0, sessionOnError: true }), + served: configuration(7, 'immediate', { sessionSampleRate: 0, sessionOnError: true }, 600, false, 'second'), redrawn: true, }, { title: 'leaves it when the same version reads the same (control)', - served: configuration(7, 'immediate', { sessionSampleRate: 0 }), + served: configuration(7, 'immediate', { sessionSampleRate: 0 }, 600, false, 'second'), redrawn: false, }, ]) { @@ -597,8 +602,8 @@ test.describe('remote configuration and a launch that ended before it was obeyed const second = await launchAppManually(intake, userDataDir, 'await', ENABLED); try { - await waitForApplied(second.mainPage); - await new Promise((resolve) => setTimeout(resolve, APPLY_WAIT)); + // The answer of this launch, not the one kept on disk. + await waitForApplied(second.mainPage, 'second'); // Drawn at rate 0 from what was kept: nothing collected. expect(await second.mainPage.getBridgeSessionId()).toBe(''); // Asked unconditionally: the ETag another SDK version kept is not sent. From 329faebcb7baaa2dbb5ac06cdb9a2e13d2e6cc1b Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 05:46:56 -0700 Subject: [PATCH 13/25] =?UTF-8?q?=F0=9F=93=9D=20describe=20the=20retry,=20?= =?UTF-8?q?foreground=20and=20startup=20behaviour=20of=20remote=20configur?= =?UTF-8?q?ation=20precisely,=20and=20who=20can=20set=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 23 +++++++++++++---------- docs/ARCHITECTURE.md | 14 +++++++------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index db8f26b9..22e01bd6 100644 --- a/README.md +++ b/README.md @@ -469,24 +469,27 @@ await init({ - **When it is asked for.** By the main process, at `init` and whenever a new session starts — there is no timer between sessions. When the console allows it (`refresh_on_foreground`, off by default), also when the user comes back to the application — a window of it gains focus — and - what it holds is at least the server's `ttl` old (10 minutes by default, never less than one - minute), revalidating with the ETag it holds. Without that permission, a session that never goes + the last completed request, successful or failed, finished at least the server's `ttl` ago (10 + minutes by default, never less than one minute), revalidating with the ETag it holds. Such a + refresh waits for a retry already pending and does not start a new round of retries. Without that permission, a session that never goes idle keeps the configuration it has until it turns over, after up to four hours. There is never more than one request in flight, and nothing in `init` waits for one. - **Failures.** Network failures, timeouts, malformed responses, HTTP 429 and 5xx responses change - nothing and are retried after about 5 s and 60 s, then at the next session or foreground refresh. Other - HTTP errors are not retried. Either way the configuration in force stays as it was. + nothing and are retried with successive delays of approximately 5 s and 60 s, each with ±20% + jitter, then at the next session. Other HTTP errors are not retried. Either way the configuration in force stays as it was. - **Kept on disk.** The last configuration accepted is kept in the application's `userData` - directory — written before returning when the application quits, if its write has not landed yet. - New sessions use it before the network answers, or without the network at all. A valid session - resumed from the previous launch keeps its original draw, subject to immediate activation: when - what was kept asks to apply immediately and is newer than what the session was drawn under, the - session is judged by it at startup, as below. It applies only to the same intake, application, + directory — written before returning when the application quits, if its write has not landed or + failed. New sessions use it before the network answers, or without the network at all. A valid + session resumed from the previous launch keeps its original draw, subject to immediate activation: + when what was kept asks to apply immediately, the session is judged by it at startup, as below, and + ended only where it decides it. It applies only to the same intake, application, `env` and `version`. An SDK upgrade keeps the values but asks for the full configuration again rather than revalidating the one the previous version read, and judges the running session again if this version reads it differently. - **Nothing published.** An application whose configuration was never published is answered with - version 0, switched off: the init values apply, and no version is reported. + version 0: whatever else the answer carries, the init values apply and no version is reported. +- **Trust.** Anyone holding the client token can read these values, so put nothing secret in them; + and whoever can answer the configured `proxy` or intake can also set them, the sampling included. - **When a change applies.** A session's draw is locked for its whole life, so by default (`next_session`) a change applies from the next session on. When the console asks for a change to apply immediately, the running session is ended — and the next user activity draws a new one — diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b6d4e992..e19100dd 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -107,13 +107,13 @@ not drawn, and of a withheld session that ended without an error (its late event `session.sampled_for_error` and `_dd.configuration.session_sample_rate`. With `remoteConfigurationEnabled`, the sampling a draw reads comes from `RemoteConfiguration`: the -console's values over the init ones, fetched at init, at every new session and — when the console allows it — on a return to the -foreground once the `ttl` has passed, and kept -on disk (`_fc_remote_config`, replaced through `StateFile`, written synchronously on `APP_MAY_EXIT` -if its write has not landed) for later draws. At startup, a resumed session drawn before a kept -`immediate` configuration is judged by it before anything is collected. The session -records the configuration version it was drawn under next to its rate, and `SessionContext` reports it -as `_dd.configuration.rc_version`. A configuration published with `activation: immediate` asks +console's values over the init ones, fetched at init, at every new session and — when the console +allows it — on a return to the foreground once the `ttl` has passed, and kept on disk +(`_fc_remote_config`, replaced through `StateFile`, written synchronously on `APP_MAY_EXIT` if its +write has not landed) for later draws. At startup, a kept `immediate` configuration judges the +session the launch starts with before anything is collected. The session records the configuration +version it was drawn under next to its rate, and `SessionContext` reports it as +`_dd.configuration.rc_version`. A configuration published with `activation: immediate` asks `SessionManager.applySamplingChange` to end the running session where the new sampling is decisive for it; anything else applies from the next session. From f05f70decc6d567961ac104e1e45dd33c4698817 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 06:45:28 -0700 Subject: [PATCH 14/25] =?UTF-8?q?=F0=9F=90=9B=20never=20let=20a=20kept=20c?= =?UTF-8?q?onfiguration=20override=20a=20newer=20session=20draw,=20and=20r?= =?UTF-8?q?ead=20a=20kept=20version=200=20as=20nothing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Startup enforcement of a kept immediate configuration skips a session drawn under a newer version: the configuration and the session live in different files, and the newer configuration's write may be the one that was lost. Same, older and missing versions are still judged. - A version 0 record restored from disk applies no values and no custom, as a version 0 answer from the network does. - The cache identity's exclusion of the client token, and the backend's monotonic versions behind the rejection of lower ones, are stated where they are relied on; the unsupported-schema warning is shorter. --- src/domain/RemoteConfiguration.spec.ts | 19 ++++++++++++++++++- src/domain/RemoteConfiguration.ts | 26 ++++++++++++++++++-------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts index a8840914..e5b92a52 100644 --- a/src/domain/RemoteConfiguration.spec.ts +++ b/src/domain/RemoteConfiguration.spec.ts @@ -361,7 +361,7 @@ describe('RemoteConfiguration', () => { expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); expect(displayWarn).toHaveBeenCalledTimes(1); - expect(displayWarn).toHaveBeenCalledWith(expect.stringContaining('schema_version 2, which this SDK (test)')); + expect(displayWarn).toHaveBeenCalledWith(expect.stringContaining('unsupported schema_version 2 (SDK test)')); await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 2); expect(fetchMock).toHaveBeenCalledTimes(1); @@ -438,6 +438,16 @@ describe('RemoteConfiguration', () => { expect(requestHeaders()).toEqual({}); }); + it('reads a kept version 0 as no configuration, whatever it holds', async () => { + mfs.readFile.mockResolvedValue(storedFile({ version: 0, values: { sessionSampleRate: 0 } })); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(configuration.getCustom()).toBeUndefined(); + }); + it('ignores a file that is not JSON', async () => { mfs.readFile.mockResolvedValue('{not json'); fetchMock.mockReturnValue(new Promise(() => undefined)); @@ -1088,6 +1098,13 @@ describe('RemoteConfiguration', () => { expect(sessions.getSession().status).toBe('expired'); }); + it('leaves a session drawn under a newer version than what was kept: the kept one is the older', async () => { + // The newer configuration's write was lost; the session drawn under it was saved. + const sessions = await restart({ trackingType: TrackingType.TRACKED, sampleRate: 100, rcVersion: 10 }, STOP); + + expect(sessions.getSession().status).toBe('active'); + }); + it('leaves a same-version session alone when what was kept applies to the next session', async () => { const sessions = await restart( { trackingType: TrackingType.NOT_TRACKED, sampleRate: 0, rcVersion: 9 }, diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts index 8723e37d..18d99d1f 100644 --- a/src/domain/RemoteConfiguration.ts +++ b/src/domain/RemoteConfiguration.ts @@ -122,7 +122,7 @@ type RequestOutcome = 'done' | 'retry'; * Off unless `remoteConfigurationEnabled`: no request, no file, and the init values apply. */ export class RemoteConfiguration { - private readonly stateFile = new StateFile(getFilePath(), 'remote configuration'); + private readonly stateFile = new StateFile(getFilePath(), 'the remote configuration cache'); private readonly identity: string; private delivered: Delivered | undefined; private etag: string | undefined; @@ -140,6 +140,8 @@ export class RemoteConfiguration { private stopped = false; private constructor(private readonly config: Configuration) { + // The client token is not part of it: the configuration belongs to the application, so a token + // rotated for the same application, env and version is answered the same. this.identity = JSON.stringify([config.proxy ?? config.site, config.applicationId, config.env, config.version]); } @@ -181,12 +183,15 @@ export class RemoteConfiguration { * Judges the session the launch starts with by the configuration the previous launch kept, when * it asked to apply at once. That launch may have ended a session in memory and ended itself * before the session file was deleted; a session drawn under that very version, read differently - * by an SDK from before an upgrade, is the same case. The judgement ends nothing a draw under the - * configuration in force would keep, so it needs no version to compare. Called before anything is - * collected, so nothing of such a session is. + * by an SDK from before an upgrade, is the same case. Same-version configurations are judged again + * — the judgement ends nothing a draw under them would keep. A session drawn under a newer version + * is left alone: the configuration and the session live in different files, the newer one's write + * may be what was lost, and an older configuration must not override a newer draw. Called before + * anything is collected, so nothing of an ended session is. */ applyKept(sessions: SessionOwner): void { - if (this.delivered?.activation === Activation.IMMEDIATE) { + const kept = this.delivered; + if (kept?.activation === Activation.IMMEDIATE && (sessions.getSession().rcVersion ?? 0) <= kept.version) { sessions.applySamplingChange(); } } @@ -283,10 +288,12 @@ export class RemoteConfiguration { return; } const { version, values, custom, activation, ttl, refreshOnForeground } = stored; + // Version 0 is no configuration, whatever a file says it holds — as it is from the network. + const published = version > 0; this.delivered = { version, - values, - ...(custom ? { custom } : {}), + values: published ? values : {}, + ...(published && custom ? { custom } : {}), activation: activation ?? Activation.NEXT_SESSION, ...(ttl === undefined ? {} : { ttl }), refreshOnForeground: refreshOnForeground ?? false, @@ -375,7 +382,7 @@ export class RemoteConfiguration { if (!this.warnedUnsupportedSchema) { this.warnedUnsupportedSchema = true; displayWarn( - `Remote configuration ignored: the server answered with schema_version ${parsed.unsupportedSchema}, which this SDK (${__SDK_VERSION__}) cannot read. Check that the SDK and the server agree on the schema; the settings already in force (init values or the last good configuration) remain in effect.` + `Remote configuration ignored: unsupported schema_version ${parsed.unsupportedSchema} (SDK ${__SDK_VERSION__}). Existing settings remain active. Check SDK/server compatibility.` ); } return 'done'; @@ -388,6 +395,9 @@ export class RemoteConfiguration { const held = this.delivered; // Settings only ever change under a higher number, so a lower one is an older answer arriving // late; applying it would put this client back on settings the console has already replaced. + // The backend allocates each published version as MAX(version) + 1, republishes a rollback under + // a new number, and prunes only versions older than the latest; version 0 means nothing was ever + // published, so a published application never answers 0 or a lower number again. if (held !== undefined && delivered.version < held.version) { return; } From 0213eb34a05c1a5c4c974332e53075d0ede3fb53 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 06:45:28 -0700 Subject: [PATCH 15/25] =?UTF-8?q?=F0=9F=90=9B=20announce=20a=20session=20r?= =?UTF-8?q?enewal=20in=20the=20same=20continuation=20as=20the=20check=20th?= =?UTF-8?q?at=20the=20session=20is=20still=20active?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The renewal is announced by session creation itself, right after its last check, so a session ended by an immediate configuration once saved is never announced as renewed. --- src/config.ts | 7 ++++--- src/domain/session/SessionManager.spec.ts | 24 +++++++++++++++++++++++ src/domain/session/SessionManager.ts | 21 ++++++++++---------- src/index.ts | 4 ++-- 4 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/config.ts b/src/config.ts index 10ccf66b..c1b12b7a 100644 --- a/src/config.ts +++ b/src/config.ts @@ -65,9 +65,10 @@ export interface InitConfiguration { * * The main process asks for the configuration at init and whenever a new session starts — and, * when the console allows it, when the user comes back to the application — and keeps the last - * good answer on disk, so new sessions draw with it before the network answers. A delivered value takes precedence over the init value; a value the - * console did not set leaves the init value in place. A change applies to the next session, - * unless the console asks for it to apply at once — see the README, Remote configuration. + * good answer on disk, so new sessions draw with it before the network answers. A delivered value + * takes precedence over the init value; a value the console did not set leaves the init value in + * place. A change applies to the next session, unless the console asks for it to apply at once — + * see the README, Remote configuration. * * Set it here, in the main process, and not in the renderers' browser SDK: the main process owns * the sessions and their sampling, and a renderer under the bridge ignores its own. diff --git a/src/domain/session/SessionManager.spec.ts b/src/domain/session/SessionManager.spec.ts index 41b30bcc..8a560f1c 100644 --- a/src/domain/session/SessionManager.spec.ts +++ b/src/domain/session/SessionManager.spec.ts @@ -818,6 +818,30 @@ describe('sessionManager', () => { expect(vi.getTimerCount()).toBe(0); }); + it('does not announce a renewal of a session that ended once saved, before the renewal was announced', async () => { + await startSession({ sessionSampleRate: 100, sessionOnError: false }); + sessionManager.expire(); + lifecycleEvents.length = 0; + // An immediate stop that lands right after the new session's save was checked: as its timers + // are being scheduled, before the renewal is announced. + const schedule = globalThis.setTimeout; + let stopped = false; + vi.spyOn(globalThis, 'setTimeout').mockImplementation(((callback: () => void, delay?: number) => { + if (!stopped) { + stopped = true; + applyChange({ sessionSampleRate: 0, sessionOnError: false }); + } + return schedule(callback, delay); + }) as typeof setTimeout); + + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.END_USER_ACTIVITY }); + await vi.advanceTimersByTimeAsync(0); + + expect(stopped).toBe(true); + expect(sessionManager.getSession().status).toBe('expired'); + expect(lifecycleEvents).not.toContain(LifecycleKind.SESSION_RENEW); + }); + it('leaves no timer behind that could end a later session early', async () => { await startSession({ sessionSampleRate: 100, sessionOnError: false }); sessionManager.expire(); diff --git a/src/domain/session/SessionManager.ts b/src/domain/session/SessionManager.ts index 52742156..6fb82bcf 100644 --- a/src/domain/session/SessionManager.ts +++ b/src/domain/session/SessionManager.ts @@ -192,7 +192,7 @@ export class SessionManager { this.scheduleSessionTimeout(existingState.created); } } else { - await this.createNewSession(); + await this.createNewSession(false); } this.activitySubscription = this.eventManager.registerHandler({ @@ -205,11 +205,12 @@ export class SessionManager { } /** - * Creates and saves a session, and answers whether it is still the active one once saved: a - * configuration that applies at once can end it while it is being written, and nothing may then - * be scheduled for it or announced about it. + * Creates and saves a session, then schedules its timers and, for a `renewal`, announces it — only + * while it is still the active one: a configuration that applies at once can end it while it is + * being written. The announcement follows the last check in the same continuation, with no `await` + * between them for anything to end the session in. */ - private async createNewSession(): Promise { + private async createNewSession(renewal: boolean): Promise { const now = Date.now(); const sampling = this.getSampling(); const state: SessionState = { @@ -224,12 +225,14 @@ export class SessionManager { this.setCurrent(state); await this.saveCurrentState(); if (!this.isCurrent(state)) { - return false; + return; } this.scheduleInactivityTimeout(); this.scheduleSessionTimeout(state.created); - return true; + if (renewal && this.isCurrent(state)) { + this.eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + } } /** Whether `state` is still the session in force, and still active, after an `await`. */ @@ -263,9 +266,7 @@ export class SessionManager { private async updateActivity(): Promise { if (this.isExpired()) { - if (await this.createNewSession()) { - this.eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); - } + await this.createNewSession(true); return; } diff --git a/src/index.ts b/src/index.ts index d71b9d4e..86ff8ddc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -176,8 +176,8 @@ export function clearUser(): void { /** * The application's own `custom` values, as delivered by the console's remote configuration — * a copy, or `undefined` when `remoteConfigurationEnabled` is off, nothing has been delivered yet, - * or the console set none. Answers from the last configuration kept on disk until the server - * answers, so it can be read right after `init` resolves. + * or the console set none. Until the server answers, it is read from the last configuration + * kept on disk, so it can be read right after `init` resolves. * * The name matches `flashcatRum.getRemoteConfig()` in `@flashcatcloud/browser-rum`. Anyone holding * the client token can read these values: put nothing secret in them. From 8a0d4de1df7b4362e9b791fc751f1ac0a456cdb3 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 06:45:28 -0700 Subject: [PATCH 16/25] =?UTF-8?q?=E2=9C=85=20cover=20the=20remote=20config?= =?UTF-8?q?uration=20kill=20switch=20end=20to=20end?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- e2e/scenarios/remote-config.scenario.ts | 45 +++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/e2e/scenarios/remote-config.scenario.ts b/e2e/scenarios/remote-config.scenario.ts index 800043ba..d9af7f2e 100644 --- a/e2e/scenarios/remote-config.scenario.ts +++ b/e2e/scenarios/remote-config.scenario.ts @@ -312,6 +312,51 @@ test.describe('remote configuration', () => { } }); + test.describe('the kill switch (enabled: false)', () => { + test.use({ sdkConfig: { remoteConfigurationEnabled: true } }); + + /** An emergency stop, published with the kill switch in the given position. */ + function stop(enabled: boolean) { + const { body } = configuration(5, 'immediate', { sessionSampleRate: 0 }, 1, true); + return { body: { ...body, enabled } }; + } + + test.describe('off', () => { + test.use({ remoteConfig: stop(false) }); + + test('applies none of the values, yet honours when to ask again', async ({ intake, mainPage, electronApp }) => { + test.setTimeout(120_000); + await intake.waitForConfigRequests(1); + const sessionId = await mainPage.getBridgeSessionId(); + expect(sessionId).not.toBe(''); + + // Past the ttl floor: the stop would long have ended the session if it applied. + await new Promise((resolve) => setTimeout(resolve, 61_000)); + expect(await mainPage.getBridgeSessionId()).toBe(sessionId); + expect(await mainPage.getRemoteConfig()).toBeUndefined(); + + // The answer's ttl and refresh_on_foreground are kept all the same: focus asks again. + await electronApp.evaluate(({ app, BrowserWindow }) => { + app.emit('browser-window-focus', {}, BrowserWindow.getAllWindows()[0]); + }); + const [, second] = await intake.waitForConfigRequests(2); + expect(second.params.applied_version).toBe('5'); + expect(second.headers['if-none-match']).toMatch(/^".+"$/); + expect(await mainPage.getBridgeSessionId()).toBe(sessionId); + }); + }); + + test.describe('on (control)', () => { + test.use({ remoteConfig: stop(true) }); + + test('applies the stop at once', async ({ mainPage }) => { + await waitForApplied(mainPage, 'v5'); + + expect(await mainPage.getBridgeSessionId()).toBe(''); + }); + }); + }); + test.describe('next session (deferred)', () => { test.use({ sdkConfig: { remoteConfigurationEnabled: true }, From 905bc6be96b8a440045140ba5e06913fee3f5d41 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 06:45:28 -0700 Subject: [PATCH 17/25] =?UTF-8?q?=F0=9F=93=9D=20describe=20the=20per-sessi?= =?UTF-8?q?on=20retry=20budget,=20the=20exit=20write,=20resumed=20sessions?= =?UTF-8?q?=20and=20upgrades=20of=20remote=20configuration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 27 ++++++++++++++++----------- docs/TESTING.md | 6 +++--- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 22e01bd6..1c6aec67 100644 --- a/README.md +++ b/README.md @@ -375,7 +375,7 @@ before the logout still resolve to the user who was logged in then. The application's own `custom` values, as delivered by the console's remote configuration — a copy, or `undefined` when `remoteConfigurationEnabled` is off, nothing has been delivered yet, or the -console set none. It answers from the configuration kept on disk until the server answers, so it can +console set none. Until the server answers, it is read from the configuration kept on disk, so it can be read right after `init` resolves. Anyone holding the client token can read these values: put nothing secret in them. See [Remote configuration](#remote-configuration). @@ -471,21 +471,26 @@ await init({ default), also when the user comes back to the application — a window of it gains focus — and the last completed request, successful or failed, finished at least the server's `ttl` ago (10 minutes by default, never less than one minute), revalidating with the ETag it holds. Such a - refresh waits for a retry already pending and does not start a new round of retries. Without that permission, a session that never goes + refresh leaves a retry already pending to ask in its place, and does not start a new round of + retries. Without that permission, a session that never goes idle keeps the configuration it has until it turns over, after up to four hours. There is never more than one request in flight, and nothing in `init` waits for one. - **Failures.** Network failures, timeouts, malformed responses, HTTP 429 and 5xx responses change nothing and are retried with successive delays of approximately 5 s and 60 s, each with ±20% - jitter, then at the next session. Other HTTP errors are not retried. Either way the configuration in force stays as it was. + jitter. That two-attempt budget belongs to the session: a new session resets it, and once it is + spent, a foreground refresh the console allows still makes one attempt, with no retries behind it. + Other HTTP errors are not retried. Either way the configuration in force stays as it was. - **Kept on disk.** The last configuration accepted is kept in the application's `userData` - directory — written before returning when the application quits, if its write has not landed or - failed. New sessions use it before the network answers, or without the network at all. A valid - session resumed from the previous launch keeps its original draw, subject to immediate activation: - when what was kept asks to apply immediately, the session is judged by it at startup, as below, and - ended only where it decides it. It applies only to the same intake, application, - `env` and `version`. An SDK upgrade keeps the values but asks for the full configuration again - rather than revalidating the one the previous version read, and judges the running session again - if this version reads it differently. + directory. A graceful exit retries pending or failed writes; a forced termination can leave an + older cache. New sessions use it before the network answers, or without the network at all. It + applies only to the same intake, application, `env` and `version`. +- **Resumed sessions.** A valid session resumed from the previous launch keeps its original draw, + subject to immediate activation: when what was kept asks to apply immediately, the session is + judged by it at startup, as below, unless it was drawn under a newer version than the one kept — + an older configuration never overrides a newer draw. +- **SDK upgrades.** An upgrade keeps the values but asks for the full configuration again rather + than revalidating the one the previous version read, and judges the running session again if this + version reads it differently. - **Nothing published.** An application whose configuration was never published is answered with version 0: whatever else the answer carries, the init values apply and no version is reported. - **Trust.** Anyone holding the client token can read these values, so put nothing secret in them; diff --git a/docs/TESTING.md b/docs/TESTING.md index 32d5312f..e1f0423f 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -72,9 +72,9 @@ The fake intake also serves the remote configuration endpoint (`GET /api/v2/rum/ sets what it answers with the `remoteConfig` fixture option or `intake.setRemoteConfig()` — a body (with an `ETag`, and a 304 when it is sent back), a status, or a dropped connection; unset, it answers like the real endpoint for an application with nothing published (200, version 0, switched -off) — and reads what -the SDK asked with `intake.getConfigRequests()`. Remote configuration is never tested against a real -backend: a fake configuration version reported there shows up in the console's version statistics. +off) — and reads what the SDK asked with `intake.getConfigRequests()`. Remote configuration is never +tested against a real backend: a fake configuration version reported there shows up in the console's +version statistics. ### Not covered From 6cdbf79fc4ff7fe32e66f093bb2214f53a92b08f Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 08:05:17 -0700 Subject: [PATCH 18/25] =?UTF-8?q?=F0=9F=90=9B=20never=20let=20an=20older?= =?UTF-8?q?=20immediate=20answer=20end=20a=20session=20drawn=20under=20a?= =?UTF-8?q?=20newer=20version,=20and=20bound=20what=20is=20read?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The immediate decision made on a network answer now skips a session drawn under a newer version, as startup already did: versions are published in order, but a backend node can serve an older snapshot for a while, so answers do not arrive in order. The same version is still judged. - An answer is read as a stream and only for a 200; past 256 KiB the rest is cancelled and the answer is treated as malformed, keeping the last good configuration and retrying. A kept file larger than that is not read. - The pending-retry guard of the foreground refresh is covered. - The unsupported-schema warning says what stays in force and when to upgrade. --- src/domain/RemoteConfiguration.spec.ts | 115 +++++++++++++++++++++++++ src/domain/RemoteConfiguration.ts | 87 ++++++++++++++++--- 2 files changed, 189 insertions(+), 13 deletions(-) diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts index e5b92a52..0c87d010 100644 --- a/src/domain/RemoteConfiguration.spec.ts +++ b/src/domain/RemoteConfiguration.spec.ts @@ -87,6 +87,7 @@ describe('RemoteConfiguration', () => { vi.stubGlobal('fetch', fetchMock); mfs.readFile.mockRejectedValue(Object.assign(new Error('ENOENT'), { code: 'ENOENT' })); mfs.writeFile.mockResolvedValue(undefined); + mfs.stat.mockResolvedValue({ size: 1024 }); eventManager = new EventManager(); onImmediateChange = vi.fn(); config = createTestConfiguration({ @@ -354,6 +355,59 @@ describe('RemoteConfiguration', () => { expect(fetchMock).toHaveBeenCalledTimes(2); }); + function streamed(chunks: string[], headers: Record = {}) { + const encoder = new TextEncoder(); + let cancelled = false; + const body = new ReadableStream({ + pull(controller) { + const chunk = chunks.shift(); + if (chunk === undefined) { + controller.close(); + } else { + controller.enqueue(encoder.encode(chunk)); + } + }, + cancel() { + cancelled = true; + }, + }); + return { response: new Response(body, { status: 200, headers }), wasCancelled: () => cancelled }; + } + + it.each([ + ['with a Content-Length', true], + ['streamed without a Content-Length', false], + ])( + 'refuses an answer larger than %s allows: stops reading it, keeps what it holds, and retries', + async (_, withLength) => { + mfs.readFile.mockResolvedValue(storedFile()); + const chunk = 'x'.repeat(64 * 1024); + const prefix = JSON.stringify(configurationBody({ custom: { big: '' } })).slice(0, -3); + const chunks = [prefix, ...Array.from({ length: 16 }, () => chunk), '"}}']; + const length = chunks.reduce((total, part) => total + part.length, 0); + const { response, wasCancelled } = streamed(chunks, withLength ? { 'Content-Length': String(length) } : {}); + fetchMock.mockResolvedValueOnce(response); + + const configuration = await start(); + + expect(wasCancelled()).toBe(true); + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(writtenFiles()).toEqual([]); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + expect(fetchMock).toHaveBeenCalledTimes(2); + } + ); + + it('reads an answer streamed in pieces under the ceiling', async () => { + const text = JSON.stringify(configurationBody({ custom: { flag: 'x'.repeat(10_000) } })); + const { response } = streamed([text.slice(0, 100), text.slice(100, 5000), text.slice(5000)]); + fetchMock.mockResolvedValueOnce(response); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 25, sessionOnError: true, rcVersion: 3 }); + }); + it('refuses a schema it does not know without retrying, and says so', async () => { fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ schema_version: 2 })))); @@ -448,6 +502,17 @@ describe('RemoteConfiguration', () => { expect(configuration.getCustom()).toBeUndefined(); }); + it('does not read a kept file larger than any answer it could have kept', async () => { + mfs.stat.mockResolvedValue({ size: 1024 * 1024 }); + mfs.readFile.mockResolvedValue(storedFile()); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(mfs.readFile).not.toHaveBeenCalled(); + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + }); + it('ignores a file that is not JSON', async () => { mfs.readFile.mockResolvedValue('{not json'); fetchMock.mockReturnValue(new Promise(() => undefined)); @@ -823,6 +888,26 @@ describe('RemoteConfiguration', () => { expect(fetchMock).toHaveBeenCalledTimes(2); }); + it('leaves a pending retry to ask, even once what it holds is past the ttl', async () => { + // Spread pinned high: the second retry waits 72 s, longer than the 60 s ttl floor. + vi.spyOn(Math, 'random').mockReturnValue(0.999); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + await start(); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + expect(fetchMock).toHaveBeenCalledTimes(3); + + // Past the ttl since the last request ended, before the second retry is due. + await vi.advanceTimersByTimeAsync(65_000); + focus(); + await vi.advanceTimersByTimeAsync(0); + expect(fetchMock).toHaveBeenCalledTimes(3); + + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[1] * 1.2 - 65_000); + expect(fetchMock).toHaveBeenCalledTimes(4); + }); + it('neither cancels a pending retry nor re-arms the backoff, so focus cannot outpace it', async () => { vi.spyOn(Math, 'random').mockReturnValue(0.5); fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); @@ -1063,6 +1148,36 @@ describe('RemoteConfiguration', () => { const STOP = { version: 9, values: { sessionSampleRate: 0 }, activation: 'immediate' }; + describe('then a network answer, which may come from a backend node holding an older snapshot', () => { + it.each([ + { version: 8, activation: 'immediate', ends: false, why: 'older than what is kept: refused as late' }, + { version: 10, activation: 'immediate', ends: false, why: 'older than the session draw' }, + { version: 10, activation: 'next_session', ends: false, why: 'for the next session' }, + { version: 11, activation: 'immediate', ends: true, why: 'the very version the session was drawn under' }, + { version: 12, activation: 'immediate', ends: true, why: 'newer than the session draw' }, + ])( + 'a stop at v$version ($activation) $why: ends the v11 session = $ends', + async ({ version, activation, ends }) => { + // Kept: v9 at rate 100; the session was drawn under v11, whose write was lost. + const sessions = await restart( + { trackingType: TrackingType.TRACKED, sampleRate: 100, rcVersion: 11 }, + { ...STOP, values: { sessionSampleRate: 100 } } + ); + expect(sessions.getSession().status).toBe('active'); + fetchMock.mockReset(); + fetchMock.mockResolvedValueOnce( + ok(configurationBody({ version, activation, rum: { sessionSampleRate: 0 } })) + ); + + remote!.start(eventManager, sessions); + await vi.advanceTimersByTimeAsync(0); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(sessions.getSession().status).toBe(ends ? 'expired' : 'active'); + } + ); + }); + it.each([ { title: 'drawn under an older version', resumed: { rcVersion: 8 } }, { title: 'drawn under no version at all', resumed: {} }, diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts index 18d99d1f..7d580f20 100644 --- a/src/domain/RemoteConfiguration.ts +++ b/src/domain/RemoteConfiguration.ts @@ -20,6 +20,12 @@ const SUPPORTED_SCHEMA_VERSION = 1; /** The shape of the file written here, not the SDK version: only a change of shape orphans it. */ const FILE_FORMAT = 1; export const REQUEST_TIMEOUT = 10 * ONE_SECOND; +/** + * The most this SDK reads of an answer, or of the file it keeps one in. Generous: the backend caps + * the application's `custom` values at 16 KiB and the rest of an answer is a few hundred bytes. A + * misbehaving proxy must not make the main process buffer, keep and copy whatever it sends. + */ +export const MAX_CONFIGURATION_BYTES = 256 * 1024; /** * A failed request is retried quickly, then patiently, then not until the next natural trigger (a * new session, or the next launch). Two extra requests per outage, so a fleet can never turn an @@ -190,9 +196,8 @@ export class RemoteConfiguration { * anything is collected, so nothing of an ended session is. */ applyKept(sessions: SessionOwner): void { - const kept = this.delivered; - if (kept?.activation === Activation.IMMEDIATE && (sessions.getSession().rcVersion ?? 0) <= kept.version) { - sessions.applySamplingChange(); + if (this.delivered) { + judge(sessions, this.delivered); } } @@ -279,6 +284,10 @@ export class RemoteConfiguration { this.stateFile.sweep(); let stored: StoredConfiguration | undefined; try { + if ((await fs.stat(getFilePath())).size > MAX_CONFIGURATION_BYTES) { + // Larger than anything this SDK keeps: not a file to read whole into memory. + return; + } stored = parseStoredConfiguration(JSON.parse(await fs.readFile(getFilePath(), 'utf-8'))); } catch { // No file yet, or one that is not ours to read: the init values apply until the server answers. @@ -351,13 +360,11 @@ export class RemoteConfiguration { private async request(signal: AbortSignal): Promise { let response: Response; - let body: string; try { response = await fetch(this.buildUrl(), { headers: this.etag ? { 'If-None-Match': this.etag } : {}, signal, }); - body = await response.text(); } catch { // Offline, refused, timed out, or stopped. return 'retry'; @@ -365,14 +372,26 @@ export class RemoteConfiguration { if (signal.aborted) { return 'done'; } - if (response.status === 304) { - // Only ever asked with an ETag, which only ever sits next to the values it describes. - return this.delivered ? 'done' : 'retry'; - } if (response.status !== 200) { + // Nothing in it is read. + void response.body?.cancel().catch(() => undefined); + if (response.status === 304) { + // Only ever asked with an ETag, which only ever sits next to the values it describes. + return this.delivered ? 'done' : 'retry'; + } return response.status === 429 || response.status >= 500 ? 'retry' : 'done'; } - const parsed = parseResponse(body); + let body: string | undefined; + try { + body = await readBounded(response); + } catch { + return 'retry'; + } + if (signal.aborted) { + return 'done'; + } + // Too large to be a configuration: as unrecognisable as a body that does not parse. + const parsed = body === undefined ? undefined : parseResponse(body); if (parsed === undefined) { // A 200 is not proof the body came from the configuration endpoint: a captive portal or a // misrouted proxy answers 200 too. Storing that would blank the console's values. @@ -382,7 +401,7 @@ export class RemoteConfiguration { if (!this.warnedUnsupportedSchema) { this.warnedUnsupportedSchema = true; displayWarn( - `Remote configuration ignored: unsupported schema_version ${parsed.unsupportedSchema} (SDK ${__SDK_VERSION__}). Existing settings remain active. Check SDK/server compatibility.` + `Remote configuration ignored: unsupported schema_version ${parsed.unsupportedSchema} (SDK ${__SDK_VERSION__}). The last accepted configuration, or the init values if none, remains in force; upgrade the SDK if the server uses a newer schema.` ); } return 'done'; @@ -425,8 +444,8 @@ export class RemoteConfiguration { // the previous SDK did not read. A plain repeat, the ordinary answer, says nothing new; judging // the session again on it would be harmless, but it is not news. const isNew = held === undefined || delivered.version > held.version || !sameValues(held.values, delivered.values); - if (isNew && delivered.activation === Activation.IMMEDIATE) { - this.sessions?.applySamplingChange(); + if (isNew && this.sessions) { + judge(this.sessions, delivered); } } @@ -460,6 +479,48 @@ function ttlOf({ ttl }: Delivered): number { return ttl === undefined ? DEFAULT_TTL : Math.max(ttl * ONE_SECOND, MIN_TTL); } +/** + * Ends the running session where `configuration` decides it, when it asks to apply at once — unless + * the session was drawn under a newer version. Versions are published in order, but answers do not + * arrive in order: a backend node can serve an older snapshot for a while after a newer one is + * live, and a newer configuration's write to disk can be the one a launch lost. An older + * configuration must never override a newer draw; the same version is judged again, which ends + * nothing a draw under it would keep, and covers a draw an older SDK read differently. + */ +function judge(sessions: SessionOwner, configuration: Delivered): void { + if ( + configuration.activation === Activation.IMMEDIATE && + (sessions.getSession().rcVersion ?? 0) <= configuration.version + ) { + sessions.applySamplingChange(); + } +} + +/** + * The body as text, or `undefined` once it grows past {@link MAX_CONFIGURATION_BYTES}, in which case + * the rest is not read. Read as a stream: a `Content-Length` may be absent, or wrong. + */ +async function readBounded(response: Response): Promise { + const reader: ReadableStreamDefaultReader | undefined = response.body?.getReader(); + if (!reader) { + return ''; + } + const chunks: Uint8Array[] = []; + let size = 0; + for (;;) { + const { done, value } = await reader.read(); + if (done) { + return Buffer.concat(chunks).toString('utf-8'); + } + size += value.byteLength; + if (size > MAX_CONFIGURATION_BYTES) { + await reader.cancel().catch(() => undefined); + return undefined; + } + chunks.push(value); + } +} + function sameValues(a: RemoteValues, b: RemoteValues): boolean { return a.sessionSampleRate === b.sessionSampleRate && a.sessionOnError === b.sessionOnError; } From 88392e0348e37dd0e6d049bdd73f793492ed1d24 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 08:05:17 -0700 Subject: [PATCH 19/25] =?UTF-8?q?=F0=9F=92=AC=20say=20what=20happens=20whe?= =?UTF-8?q?n=20remoteConfigurationEnabled=20is=20not=20true=20or=20false?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/config.spec.ts | 2 +- src/config.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/config.spec.ts b/src/config.spec.ts index bd02fd45..d76e5822 100644 --- a/src/config.spec.ts +++ b/src/config.spec.ts @@ -529,7 +529,7 @@ describe('buildConfiguration', () => { expect(buildConfiguration(config)?.remoteConfigurationEnabled).toBe(false); expect(display.displayError).toHaveBeenCalledWith( - "Configuration error: 'remoteConfigurationEnabled' must be a boolean; remote configuration remains disabled" + "Configuration error: 'remoteConfigurationEnabled' must be true or false; remote configuration is disabled" ); }); }); diff --git a/src/config.ts b/src/config.ts index c1b12b7a..bc22f8c6 100644 --- a/src/config.ts +++ b/src/config.ts @@ -265,7 +265,7 @@ export function buildConfiguration(initConfig: InitConfiguration): Configuration const remoteOption: unknown = initConfig.remoteConfigurationEnabled; if (remoteOption !== undefined && remoteOption !== null && typeof remoteOption !== 'boolean') { displayError( - "Configuration error: 'remoteConfigurationEnabled' must be a boolean; remote configuration remains disabled" + "Configuration error: 'remoteConfigurationEnabled' must be true or false; remote configuration is disabled" ); } const remoteConfigurationEnabled = remoteOption === true; From 641d517578d8566dff0b00a35c8683a0501f2134 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 08:05:17 -0700 Subject: [PATCH 20/25] =?UTF-8?q?=E2=9C=85=20give=20each=20remote=20config?= =?UTF-8?q?uration=20scenario=20more=20than=20one=20request=20timeout=20an?= =?UTF-8?q?d=20retry=20to=20see=20its=20answer=20applied?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- e2e/scenarios/remote-config.scenario.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/e2e/scenarios/remote-config.scenario.ts b/e2e/scenarios/remote-config.scenario.ts index d9af7f2e..7a1d069c 100644 --- a/e2e/scenarios/remote-config.scenario.ts +++ b/e2e/scenarios/remote-config.scenario.ts @@ -22,6 +22,11 @@ const RELEASE_WAIT = 3_500; * ending. Only an absence needs it: everything that does happen is waited for by what it shows. */ const ABSENCE_WAIT = 1_000; +/** + * How long an answer may take to be applied: more than one request that hangs until the SDK's 10 s + * timeout and the first retry after it, about 5 s (±20%), with slack for a loaded machine. + */ +const APPLY_TIMEOUT = 25_000; interface SessionEvent { type: string; @@ -74,7 +79,7 @@ async function settle(mainPage: MainPage, waitMs = RELEASE_WAIT) { * the same IPC channel as the session pushes, behind them, so by the time it is seen the renderer * has also heard of any session the answer ended. */ -async function waitForApplied(mainPage: MainPage, marker: string, timeout = 10_000) { +async function waitForApplied(mainPage: MainPage, marker: string, timeout = APPLY_TIMEOUT) { const deadline = Date.now() + timeout; while ((await mainPage.getRemoteConfig())?.marker !== marker) { if (Date.now() >= deadline) { From 6c1974daeda3cc52d5eba9cb8876096295d8223b Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 9 Oct 2026 08:05:17 -0700 Subject: [PATCH 21/25] =?UTF-8?q?=F0=9F=93=9D=20note=20that=20focus=20betw?= =?UTF-8?q?een=20windows=20counts=20and=20that=20older=20answers=20never?= =?UTF-8?q?=20end=20newer=20sessions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 1c6aec67..01d0f7f7 100644 --- a/README.md +++ b/README.md @@ -468,7 +468,8 @@ await init({ values apply. - **When it is asked for.** By the main process, at `init` and whenever a new session starts — there is no timer between sessions. When the console allows it (`refresh_on_foreground`, off by - default), also when the user comes back to the application — a window of it gains focus — and + default), also when the user comes back to the application — a window of it gains focus; moving + focus between the application's own windows counts too, and the ttl absorbs it — and the last completed request, successful or failed, finished at least the server's `ttl` ago (10 minutes by default, never less than one minute), revalidating with the ETag it holds. Such a refresh leaves a retry already pending to ask in its place, and does not start a new round of @@ -506,16 +507,18 @@ await init({ or `sessionOnError` turns on, so that it is drawn again. One that lost a draw at a real rate keeps its outcome. - Any other change waits for the next session. + Any other change waits for the next session, and none ends a session drawn under a newer version + than its own: answers can arrive out of order, and an older configuration never overrides a newer + draw. - **What events report.** A session reports the rate it was drawn at (`0` for a session kept by `sessionOnError`) and, when its draw read a delivered configuration, that configuration's version as `_dd.configuration.rc_version` — on main process and renderer events alike, resumed sessions and next-launch crash reports included. -**Renderers.** The main process owns the sessions and their sampling, so enable remote configuration -in the main process only. A renderer's Browser SDK under the bridge does not fetch a configuration -of its own, and the main process overwrites the sampling attributes and `rc_version` of every event +**Renderers.** The main process owns the sessions and their sampling. A renderer's Browser SDK under +the bridge needs no configuration of its own (enable it in the main process only), and the main +process overwrites the sampling attributes and `rc_version` of every event a renderer sends, so nothing is applied twice. ### Pre-warmed windows From bae2467d43da314bea8deb89169eb190557ae59a Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 10 Oct 2026 00:55:04 -0700 Subject: [PATCH 22/25] =?UTF-8?q?=F0=9F=90=9B=20refuse=20custom=20values?= =?UTF-8?q?=20nested=20past=20any=20configuration,=20retry=20an=20unexpect?= =?UTF-8?q?ed=202xx,=20and=20re-arm=20retries=20after=20an=20answer?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Custom values nesting deeper than 16 levels (the backend allows 3) make an answer malformed — the last good configuration stays and the request is retried — and make a kept file unreadable. The depth is measured without recursion, and a serialization failure at exit is reported instead of thrown: stopping always aborts the request in flight, clears the retry and removes its listeners. - A 2xx other than 200, such as a 204 from a proxy, is treated like a malformed answer and retried. - The two-retry budget is re-armed by any answer that lands, as well as by a new session, so a later outage gets its retries. --- src/domain/RemoteConfiguration.spec.ts | 93 ++++++++++++++++++++++++++ src/domain/RemoteConfiguration.ts | 89 +++++++++++++++++++----- 2 files changed, 166 insertions(+), 16 deletions(-) diff --git a/src/domain/RemoteConfiguration.spec.ts b/src/domain/RemoteConfiguration.spec.ts index 0c87d010..f33dd756 100644 --- a/src/domain/RemoteConfiguration.spec.ts +++ b/src/domain/RemoteConfiguration.spec.ts @@ -325,6 +325,30 @@ describe('RemoteConfiguration', () => { expect(Object.keys(custom.a as object)).toEqual(['__proto__']); }); + it('refuses an answer whose custom values nest deeper than any configuration can, keeps what it holds, and retries', async () => { + mfs.readFile.mockResolvedValue(storedFile()); + const body = JSON.stringify(configurationBody({ custom: { nested: 'DEEP' } })).replace( + '"DEEP"', + '['.repeat(10_000) + ']'.repeat(10_000) + ); + fetchMock.mockResolvedValueOnce(ok(body)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 0, sessionOnError: true, rcVersion: 2 }); + expect(writtenFiles()).toEqual([]); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('accepts custom values nested as deep as the backend allows', async () => { + fetchMock.mockResolvedValueOnce(ok(configurationBody({ custom: { list: [{ group: { field: 1 } }] } }))); + + const configuration = await start(); + + expect(configuration.getCustom()).toEqual({ list: [{ group: { field: 1 } }] }); + }); + it('drops a custom bag that is not an object', async () => { fetchMock.mockImplementation(() => Promise.resolve(ok(configurationBody({ custom: ['a'] })))); @@ -513,6 +537,21 @@ describe('RemoteConfiguration', () => { expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); }); + it('ignores a kept file whose custom values nest deeper than any configuration can', async () => { + mfs.readFile.mockResolvedValue( + storedFile().replace( + '"custom":{"flag":"cached"}', + '"custom":{"nested":' + '['.repeat(10_000) + ']'.repeat(10_000) + '}' + ) + ); + fetchMock.mockReturnValue(new Promise(() => undefined)); + + const configuration = await start(); + + expect(configuration.getSampling()).toEqual({ sessionSampleRate: 100, sessionOnError: false }); + expect(configuration.getCustom()).toBeUndefined(); + }); + it('ignores a file that is not JSON', async () => { mfs.readFile.mockResolvedValue('{not json'); fetchMock.mockReturnValue(new Promise(() => undefined)); @@ -639,6 +678,33 @@ describe('RemoteConfiguration', () => { expect(fetchMock).toHaveBeenCalledTimes(2); }); + it.each([204, 202, 206])('retries on an unexpected %s, as on a malformed answer', async (status) => { + fetchMock.mockImplementation(() => Promise.resolve(new Response(null, { status }))); + + await start(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] * 1.2); + + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + it('gives a later outage its retries again once an answer has landed', async () => { + vi.spyOn(Math, 'random').mockReturnValue(0.5); + fetchMock.mockRejectedValueOnce(new TypeError('fetch failed')); + fetchMock.mockRejectedValueOnce(new TypeError('fetch failed')); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ ttl: 60, refresh_on_foreground: true }))); + fetchMock.mockRejectedValue(new TypeError('fetch failed')); + await start(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] + RETRY_DELAYS[1]); + expect(fetchMock).toHaveBeenCalledTimes(3); + + // A later outage, past the ttl: the refresh fails, and its two retries follow. + await vi.advanceTimersByTimeAsync(60_000); + appListeners.get('browser-window-focus')!(); + await vi.advanceTimersByTimeAsync(RETRY_DELAYS[0] + RETRY_DELAYS[1]); + + expect(fetchMock).toHaveBeenCalledTimes(6); + }); + it.each([500, 503, 429])('retries on %s', async (status) => { fetchMock.mockImplementation(() => Promise.resolve(new Response('', { status }))); @@ -1102,6 +1168,33 @@ describe('RemoteConfiguration', () => { expect(syncWrites()).toEqual([]); }); + it('stops whole even when what it must write cannot be serialized', async () => { + mfs.writeFile.mockReturnValue(new Promise(() => undefined)); + fetchMock.mockResolvedValueOnce(ok(configurationBody({ version: 9 }), '"tag-9"')); + fetchMock.mockReturnValue(new Promise(() => undefined)); + await start(); + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + await vi.advanceTimersByTimeAsync(0); + const stringify = vi.spyOn(JSON, 'stringify').mockImplementation(() => { + throw new RangeError('Maximum call stack size exceeded'); + }); + + let thrown: unknown; + try { + appListeners.get('quit')!(); + } catch (error) { + thrown = error; + } finally { + stringify.mockRestore(); + } + + expect(thrown).toBeUndefined(); + + expect((fetchMock.mock.calls[1][1]!.signal as AbortSignal).aborted).toBe(true); + expect(appListeners.has('browser-window-focus')).toBe(false); + expect(appListeners.has('quit')).toBe(false); + }); + it('stops when the application quits: the request in flight is aborted and nothing is asked again', async () => { fetchMock.mockReturnValue(new Promise(() => undefined)); await start(); diff --git a/src/domain/RemoteConfiguration.ts b/src/domain/RemoteConfiguration.ts index 7d580f20..27424d3f 100644 --- a/src/domain/RemoteConfiguration.ts +++ b/src/domain/RemoteConfiguration.ts @@ -4,7 +4,7 @@ import * as path from 'node:path'; import { ONE_SECOND, type Subscription } from '@flashcatcloud/browser-core'; import type { Configuration } from '../config'; import { type AppMayExitEvent, EventKind, type EventManager, LifecycleKind, type SessionRenewEvent } from '../event'; -import { displayWarn } from '../tools/display'; +import { displayError, displayWarn } from '../tools/display'; import { StateFile } from '../tools/StateFile'; import { isConfigurationVersion, type SamplingConfiguration, type SessionManager } from './session'; import { addError, setTimeout } from './telemetry'; @@ -26,10 +26,17 @@ export const REQUEST_TIMEOUT = 10 * ONE_SECOND; * misbehaving proxy must not make the main process buffer, keep and copy whatever it sends. */ export const MAX_CONFIGURATION_BYTES = 256 * 1024; +/** + * The deepest the application's `custom` values may nest. Generous: the backend allows three levels. + * A small answer can still nest thousands of levels deep, which no configuration does, and which + * would overflow the stack of every copy and serialization of it in the main process. + */ +export const MAX_CUSTOM_DEPTH = 16; /** * A failed request is retried quickly, then patiently, then not until the next natural trigger (a - * new session, or the next launch). Two extra requests per outage, so a fleet can never turn an - * endpoint incident into a storm. + * new session, or the next launch). Two extra requests per run of failures — the budget is re-armed + * by a new session and by an answer that lands — so a fleet can never turn an endpoint incident + * into a storm. */ export const RETRY_DELAYS = [5 * ONE_SECOND, 60 * ONE_SECOND]; /** @@ -234,15 +241,19 @@ export class RemoteConfiguration { /** Stops for good, writing first what has not landed: nothing after this point will. */ stop(): void { - this.writePendingSync(); - this.stopped = true; - this.inFlight?.abort(); - this.inFlight = undefined; - clearTimeout(this.retryTimeoutId); - this.subscriptions.forEach((subscription) => subscription.unsubscribe()); - this.subscriptions = []; - app.removeListener('browser-window-focus', this.onForeground); - app.removeListener('quit', this.onQuit); + try { + this.writePendingSync(); + } finally { + // Whatever happened to the write, nothing of this instance may outlive the stop. + this.stopped = true; + this.inFlight?.abort(); + this.inFlight = undefined; + clearTimeout(this.retryTimeoutId); + this.subscriptions.forEach((subscription) => subscription.unsubscribe()); + this.subscriptions = []; + app.removeListener('browser-window-focus', this.onForeground); + app.removeListener('quit', this.onQuit); + } } private readonly onQuit = () => this.stop(); @@ -270,7 +281,18 @@ export class RemoteConfiguration { * by the file, so this one is not overwritten by an older state. */ private writePendingSync(): void { - if (this.unlanded && this.stateFile.writeSync(JSON.stringify(this.unlanded))) { + if (!this.unlanded) { + return; + } + let content: string; + try { + content = JSON.stringify(this.unlanded); + } catch (error) { + // Reported, not thrown: this runs on the way out of the process. + displayError('Failed to serialize the remote configuration cache:', error); + return; + } + if (this.stateFile.writeSync(content)) { this.unlanded = undefined; } } @@ -375,11 +397,17 @@ export class RemoteConfiguration { if (response.status !== 200) { // Nothing in it is read. void response.body?.cancel().catch(() => undefined); - if (response.status === 304) { + if (response.status === 304 && this.delivered) { // Only ever asked with an ETag, which only ever sits next to the values it describes. - return this.delivered ? 'done' : 'retry'; + this.failedAttempts = 0; + return 'done'; } - return response.status === 429 || response.status >= 500 ? 'retry' : 'done'; + // A 304 with nothing it could refer to, another 2xx a proxy made up (a 204, say), a 429 or a + // 5xx: as unrecognisable as a malformed answer, and worth asking again. Any other status is an + // answer about this client, and asking again would get the same one. + const status = response.status; + const retry = status < 300 || status === 304 || status === 429 || status >= 500; + return retry ? 'retry' : 'done'; } let body: string | undefined; try { @@ -406,6 +434,7 @@ export class RemoteConfiguration { } return 'done'; } + this.failedAttempts = 0; this.apply(parsed, response.headers.get('etag') ?? undefined); return 'done'; } @@ -555,6 +584,9 @@ function parseResponse(text: string): Delivered | { unsupportedSchema: number } if (rum !== undefined && rum !== null && !isBag(rum)) { return undefined; } + if (nestsDeeperThan(custom, MAX_CUSTOM_DEPTH)) { + return undefined; + } // Version 0 is what an application with nothing published answers: no configuration, whatever // else the body carries. const published = enabled && version > 0; @@ -603,6 +635,9 @@ function parseStoredConfiguration(value: unknown): StoredConfiguration | undefin if (!isBag(values) || (custom !== undefined && !isBag(custom)) || (etag !== undefined && typeof etag !== 'string')) { return undefined; } + if (nestsDeeperThan(custom, MAX_CUSTOM_DEPTH)) { + return undefined; + } const { sessionSampleRate, sessionOnError } = values; if (sessionSampleRate !== undefined && !isRate(sessionSampleRate)) { return undefined; @@ -613,6 +648,28 @@ function parseStoredConfiguration(value: unknown): StoredConfiguration | undefin return value as unknown as StoredConfiguration; } +/** + * Whether `value` nests objects or arrays more than `limit` levels deep, a scalar counting none. + * Walked with a stack of its own rather than by recursion, so that the check itself cannot overflow + * on the value it is there to refuse; it stops at the first branch past the limit. + */ +function nestsDeeperThan(value: unknown, limit: number): boolean { + const pending: { node: unknown; depth: number }[] = [{ node: value, depth: 0 }]; + while (pending.length > 0) { + const { node, depth } = pending.pop()!; + if (typeof node !== 'object' || node === null) { + continue; + } + if (depth + 1 > limit) { + return true; + } + for (const child of Object.values(node)) { + pending.push({ node: child, depth: depth + 1 }); + } + } + return false; +} + function isPositiveInteger(value: unknown): value is number { return isConfigurationVersion(value) && value > 0; } From 862001734301a84988102d4ef28e6ec3056f91fc Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 10 Oct 2026 00:55:09 -0700 Subject: [PATCH 23/25] =?UTF-8?q?=F0=9F=93=9D=20say=20when=20the=20retry?= =?UTF-8?q?=20budget=20is=20re-armed,=20and=20that=20only=20the=20automate?= =?UTF-8?q?d=20suites=20use=20the=20fake=20intake?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- README.md | 5 +++-- docs/TESTING.md | 7 ++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 01d0f7f7..79ad4c33 100644 --- a/README.md +++ b/README.md @@ -478,8 +478,9 @@ await init({ more than one request in flight, and nothing in `init` waits for one. - **Failures.** Network failures, timeouts, malformed responses, HTTP 429 and 5xx responses change nothing and are retried with successive delays of approximately 5 s and 60 s, each with ±20% - jitter. That two-attempt budget belongs to the session: a new session resets it, and once it is - spent, a foreground refresh the console allows still makes one attempt, with no retries behind it. + jitter. That two-attempt budget is re-armed by a new session and by any answer that lands; once a + run of failures has spent it, a foreground refresh the console allows still makes one attempt, with + no retries behind it. Other HTTP errors are not retried. Either way the configuration in force stays as it was. - **Kept on disk.** The last configuration accepted is kept in the application's `userData` directory. A graceful exit retries pending or failed writes; a forced termination can leave an diff --git a/docs/TESTING.md b/docs/TESTING.md index e1f0423f..2a756aea 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -72,9 +72,10 @@ The fake intake also serves the remote configuration endpoint (`GET /api/v2/rum/ sets what it answers with the `remoteConfig` fixture option or `intake.setRemoteConfig()` — a body (with an `ETag`, and a 304 when it is sent back), a status, or a dropped connection; unset, it answers like the real endpoint for an application with nothing published (200, version 0, switched -off) — and reads what the SDK asked with `intake.getConfigRequests()`. Remote configuration is never -tested against a real backend: a fake configuration version reported there shows up in the console's -version statistics. +off) — and reads what the SDK asked with `intake.getConfigRequests()`. These automated suites use the fake +intake only: never send a fabricated configuration version to a real backend, where it shows up in +the console's version statistics. Manual acceptance runs against the dev backend, with versions +published through the console's own API and the application's configuration restored afterwards. ### Not covered From 48438c7681ef7a8ba68c43fad67e4e41a0a1725a Mon Sep 17 00:00:00 2001 From: Fiona Date: Wed, 14 Oct 2026 03:39:57 -0700 Subject: [PATCH 24/25] =?UTF-8?q?=F0=9F=92=AC=20prefix=20the=20SDK's=20con?= =?UTF-8?q?sole=20messages=20with=20FlashCat?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/tools/display.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/display.ts b/src/tools/display.ts index c4eacb36..90191988 100644 --- a/src/tools/display.ts +++ b/src/tools/display.ts @@ -1,4 +1,4 @@ -const PREFIX = '[Datadog electron-sdk]'; +const PREFIX = '[FlashCat electron-sdk]'; /** * Display an error in the console with the SDK prefix From 950fd6ff088655ff34ba44fe93b6e77650bb030f Mon Sep 17 00:00:00 2001 From: Fiona Date: Wed, 14 Oct 2026 03:40:47 -0700 Subject: [PATCH 25/25] =?UTF-8?q?=F0=9F=93=9D=20add=20acceptance=20steps?= =?UTF-8?q?=20for=20sampling,=20sessionOnError=20and=20remote=20configurat?= =?UTF-8?q?ion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/ACCEPTANCE.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/docs/ACCEPTANCE.md b/docs/ACCEPTANCE.md index 395c6533..7b90ca20 100644 --- a/docs/ACCEPTANCE.md +++ b/docs/ACCEPTANCE.md @@ -195,6 +195,43 @@ Chromium actually report — which differs between a packaged (asar) and an unpa > Filtering on `source:electron` alone silently drops every renderer event — which is most of the > volume (views, actions, resources, web vitals). +## 4b. Sampling and `sessionOnError` + +Run against the real backend, on a dedicated test application. Verify each result in the event +store or the console, not only in the upload log: the intake answers `202` before it decodes. + +- [ ] `sessionSampleRate: 0` + `sessionOnError: true`, a session that reports no error and is then + stopped: nothing is uploaded and **no session is stored** +- [ ] Same, with a main-process error and a renderer error: the session is stored with + `session.sampled_for_error: true` and `_dd.configuration.session_sample_rate: 0`, with both the + main-process and the renderer views and the activity that preceded the error +- [ ] Same, with `sessionOnError` off (negative control): no session is stored, errors included +- [ ] `Crash` in a withheld session: the crash is reported on the next launch, `is_crash: true`, under + a session stored with the marker and rate `0` +- [ ] Default configuration (no new options): sessions are stored as before, rate `100`, no marker + +## 4c. Remote configuration + +Use a dedicated test application, and publish through the same API the console uses — **never** +fabricate a configuration version toward a real backend: it shows up in the console's version +statistics. Record the application's configuration before you start. + +- [ ] With `remoteConfigurationEnabled: true`, the request to `/api/v2/rum/config` carries + `sdk=electron`; a rule matching `sdk: electron` applies, and the console preview for another + `sdk` returns no values +- [ ] A session drawn under a published version is stored with `rc_version` equal to that version +- [ ] The next request carries `applied_version` and `If-None-Match`, and the backend answers `304` +- [ ] An `immediate` stop (rate `0`, switch off) ends the running session; nothing more is stored +- [ ] An `immediate` rate `0` with the switch on leaves an on-error session running, and its error + releases what it held +- [ ] Restart with the configuration endpoint unreachable: the first session draws with the cached + configuration (`rc_version` of the cached version); with the cache deleted (negative control), + it draws with the init values +- [ ] Publish a version restoring the recorded configuration, and check the endpoint serves it + +> An application that never published answers version `0`; versions only go up, so restoring it +> means publishing `enabled: false` with no rules — clients then behave exactly as before. + ## 5. Package ```sh