diff --git a/packages/capture/src/index.ts b/packages/capture/src/index.ts index cac54251..8dcb2ead 100644 --- a/packages/capture/src/index.ts +++ b/packages/capture/src/index.ts @@ -67,6 +67,7 @@ export { deriveRefreshMs, type SystemTracesProviderOptions, type TraceSample, + withoutTraceNames, } from './system-traces-provider'; export { type TraceEmitOptions, diff --git a/packages/capture/src/system-traces-provider.test.ts b/packages/capture/src/system-traces-provider.test.ts index 949ec446..ef4fc0af 100644 --- a/packages/capture/src/system-traces-provider.test.ts +++ b/packages/capture/src/system-traces-provider.test.ts @@ -17,6 +17,7 @@ import { createSystemTracesProvider, deriveRefreshMs, type TraceSample, + withoutTraceNames, } from './system-traces-provider'; import { type TraceProducer, TraceProducerBase, type TraceSink } from './trace-producer'; import { createTraceSuppression } from './trace-suppression'; @@ -1251,3 +1252,25 @@ describe('deriveRefreshMs', () => { expect(deriveRefreshMs(windowMs)).toBe(10_000); }); }); + +describe('withoutTraceNames', () => { + it('drops rows under the owned names on every call and keeps the rest in order', () => { + let n = 0; + const sample = withoutTraceNames( + () => [ + { name: 'a', value: ++n }, + { name: 'b', value: 2 }, + { name: 'c', value: 3 }, + ], + new Set(['b']), + ); + expect(sample()).toEqual([ + { name: 'a', value: 1 }, + { name: 'c', value: 3 }, + ]); + expect(sample()).toEqual([ + { name: 'a', value: 2 }, + { name: 'c', value: 3 }, + ]); + }); +}); diff --git a/packages/capture/src/system-traces-provider.ts b/packages/capture/src/system-traces-provider.ts index 37e8e8b2..fa311555 100644 --- a/packages/capture/src/system-traces-provider.ts +++ b/packages/capture/src/system-traces-provider.ts @@ -11,6 +11,17 @@ import type { TraceSuppression } from './trace-suppression'; // FPS_REPORT_INTERVAL_MS parity). The timer is unref'd by the default scheduler so it never keeps a // process alive. Mobile-only traces (orientation/battery/fps/displays) are simply not sampled. +/** + * Wrap a caller's polled `sample()` so rows under `owned` names are dropped on every call: a name a still-running + * producer owns keeps ONE writer (the sampler's other rows pass through). + */ +export function withoutTraceNames( + sample: () => readonly TraceSample[], + owned: ReadonlySet, +): () => readonly TraceSample[] { + return () => sample().filter((row) => !owned.has(row.name)); +} + /** One sampled system trace: a named value at the sampling instant. */ export interface TraceSample { name: string; diff --git a/packages/webview/src/launch.test.ts b/packages/webview/src/launch.test.ts index 0fcca7f8..2c79b90a 100644 --- a/packages/webview/src/launch.test.ts +++ b/packages/webview/src/launch.test.ts @@ -1695,6 +1695,93 @@ describe('launch (webview)', () => { } }); + it('a caller sampler cannot give a producer-owned name a second writer (battery handed back by native, route, view, url); its other rows pass', async () => { + const fake = fakeGlobal(); + batteryGlobals(); + const tickCbs: Array<() => void> = []; + const scheduler: Scheduler = { + setInterval: (cb) => { + tickCbs.push(cb as () => void); + return 0 as unknown as ReturnType; + }, + clearInterval: () => {}, + }; + try { + track( + 'tok', + baseOptions({ + global: fake.global, + window: { + location: { href: 'https://a.com/o' }, + get navigator() { + return (globalThis as { navigator?: unknown }).navigator; + }, + addEventListener() {}, + removeEventListener() {}, + } as never, + scheduler, + systemMetricsSampler: () => [ + { name: 'wv_metric', value: 7 }, + { name: 'battery', value: 7 }, + { name: 'charging', value: { state: 1 } }, + { name: 'battery_charging_time_s', value: 111 }, + { name: 'battery_discharging_time_s', value: 222 }, + { name: 'route', value: 'sampler-route' }, + { name: 'view', value: 'sampler-view' }, + { name: 'url', value: 'sampler-url' }, + ], + }), + ); + sendControl(fake.global, { config: { hostOwnedTraces: [] } }); // native hands every name back: only the producers may write them + await new Promise((r) => setTimeout(r, 0)); + for (const cb of tickCbs) cb(); // a tick runs the sampler + const traces = tracesOf(fake); + expect(traces.filter((t) => t.name === 'battery').map((t) => t.value)).toEqual([100]); + expect(traces.filter((t) => t.name === 'charging').map((t) => t.value)).toEqual([ + { state: 3 }, + ]); + expect(traces).toContainEqual(expect.objectContaining({ name: 'wv_metric', value: 7 })); + const values = traces.map((t) => t.value); + for (const leaked of [111, 222, 'sampler-route', 'sampler-view', 'sampler-url']) { + expect(values).not.toContain(leaked); + } + } finally { + vi.unstubAllGlobals(); + } + }); + + it('a caller sampler may still write url when no url producer exists (no readable window)', async () => { + const fake = fakeGlobal(); + track( + 'tok', + baseOptions({ + global: fake.global, + window: undefined, + systemMetricsSampler: () => [{ name: 'url', value: 'sampler-url' }], + }), + ); + await new Promise((r) => setTimeout(r, 0)); + expect(tracesOf(fake)).toContainEqual( + expect.objectContaining({ name: 'url', value: 'sampler-url' }), + ); + }); + + it('a caller sampler may still write url when URL capture is off', async () => { + const fake = fakeGlobal(); + track( + 'tok', + baseOptions({ + global: fake.global, + captureUrl: false, + systemMetricsSampler: () => [{ name: 'url', value: 'sampler-url' }], + }), + ); + await new Promise((r) => setTimeout(r, 0)); + expect(tracesOf(fake)).toContainEqual( + expect.objectContaining({ name: 'url', value: 'sampler-url' }), + ); + }); + it('a malformed hostOwnedTraces leaves the built-in list in force', async () => { const fake = fakeGlobal(); batteryGlobals(); diff --git a/packages/webview/src/launch.ts b/packages/webview/src/launch.ts index c62f9b7e..d1cb2ce6 100644 --- a/packages/webview/src/launch.ts +++ b/packages/webview/src/launch.ts @@ -32,6 +32,7 @@ import { installNetworkCapture, type TraceSample, WEBVIEW_HOST_OWNED_TRACES, + withoutTraceNames, } from '@bugsee/capture'; import { BlackoutToken, @@ -68,7 +69,7 @@ import { setCarrierClient, setCarrierStatus, } from '@bugsee/core'; -import { BugseeOption } from '@bugsee/protocol'; +import { BugseeOption, TraceName } from '@bugsee/protocol'; import { randomId } from '@bugsee/util'; import { mintControlToken } from './control-token'; import { createHostBridge, type HostBridgeTransport } from './host-bridge'; @@ -449,6 +450,19 @@ export function launch(appToken: string, options: BugseeWebViewLaunchOptions = { return launchWithSeams(appToken, safe as BugseeWebViewLaunchOptions); } +/** Names the producers that run alongside a caller sampler (`sensors: false`) own: battery x4, route, view, and url when built. */ +function producerOwnedNames(withUrl: boolean): ReadonlySet { + return new Set([ + TraceName.BATTERY, + TraceName.CHARGING, + TraceName.BATTERY_CHARGING_TIME_S, + TraceName.BATTERY_DISCHARGING_TIME_S, + TraceName.ROUTE, + TraceName.VIEW, + ...(withUrl ? [TraceName.URL] : []), + ]); +} + /** {@link launch} with the injectable seams honoured. NOT for page-reachable surfaces (see {@link launch}). */ export function launchWithSeams( appToken: string, @@ -749,11 +763,27 @@ export function launchWithSeams( }, options.onError, ); + const captureUrl = resolved.options.get(CAPTURE_URL_OPTION_KEY, true); + // Undefined when URL capture is off or the window cannot be read: then no url producer is built and the sampler may write `url`. + const urlDeps = captureUrl + ? urlProducerDepsFor(win, { + ...(carrier !== undefined ? { carrier } : {}), + urlSanitizer: options.urlSanitizer, + ...(options.onError !== undefined ? { onError: options.onError } : {}), + }) + : undefined; client.addCaptureProvider( createSystemTracesProvider({ - // A `systemMetricsSampler` REPLACES the sensor traces (heap, connection, orientation, app_state, fps), as on Node; battery still runs. + // A `systemMetricsSampler` REPLACES the sensor traces (heap, connection, orientation, app_state, fps), as on Node; battery, + // route, view and url (when captured) still run and OWN their names: a sampler row under them is dropped (one writer per + // name, #158), even when native hands a host-owned name back. ...(options.systemMetricsSampler !== undefined - ? { sample: options.systemMetricsSampler } + ? { + sample: withoutTraceNames( + options.systemMetricsSampler, + producerOwnedNames(urlDeps !== undefined), + ), + } : {}), suppression: hostOwned, producers: createBrowserSystemTraceProducers( @@ -767,15 +797,7 @@ export function launchWithSeams( }), ...(options.systemMetricsSampler !== undefined ? { sensors: false } : {}), captureUserIdle: resolved.options.get(CAPTURE_USER_IDLE_OPTION_KEY, false), - ...(resolved.options.get(CAPTURE_URL_OPTION_KEY, true) - ? withUrl( - urlProducerDepsFor(win, { - ...(carrier !== undefined ? { carrier } : {}), - urlSanitizer: options.urlSanitizer, - ...(options.onError !== undefined ? { onError: options.onError } : {}), - }), - ) - : {}), + ...withUrl(urlDeps), // `route` / `view`: the page's own router state (`client.setRoute` / `setView`, fed by the framework adapters) routeViewState: screenTrackerFor(carrier, options.onError), lifecycle,