From a7c7ed6e6f352758bdeb0148b7458fe44f2ceb78 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 6 Aug 2026 00:57:58 -0700 Subject: [PATCH 1/2] feat: identify the logged-in user from the main process MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds `setUser` / `getUser` / `clearUser`. Until now the SDK only knew the device, through the anonymous id, so a logged-in session could not be attributed to a person and Electron UV could only ever be anonymous UV. The names match `flashcatRum.setUser()` in `@flashcatcloud/browser-rum`, which is what customers already call one file away in the same application. Upstream's `@datadog/electron-sdk` names it `setUserInfo`, a name Datadog's own browser SDK has since dropped, and we do not track that fork — consistency inside the product wins over parity with a fork we are not following. Scope is set/get/clear. No `extraInfo`, no account APIs: nothing needs them yet. `usr.anonymous_id` is untouched, and `usr.id` is still never backfilled with it. The two coexist so unique users can be counted off `COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))` across a login. `clearUser` removes `usr.id` rather than blanking it, because `NULLIF(usr_id, '')` treats an absent field and an empty string as different rows. The existing `Object.keys(usr) === ['anonymous_id']` guard is kept and extended rather than relaxed. Only `id`, `name` and `email` are copied out of the caller's object, which is what makes `setUser({ anonymous_id })` structurally impossible; the hook also writes `anonymous_id` last so the guarantee does not rest on the sanitizer alone. Upstream's `filterReservedKeys` excludes only the standard fields from its `extraInfo` bag, so `addUserExtraInfo({ anonymous_id })` there overwrites the device id — recorded in a comment so a later `extraInfo` reserves the key. Two questions, two stores. `get()` answers "who is logged in now" from a plain field, so `clearUser()` takes effect in the millisecond it happens. Assembly asks `find(startTime)`, backed by a `DiskValueHistory` like `ViewContext`'s, because a main-process event is not always assembled in the moment it describes: a native crash is parsed on the next startup carrying the crash's own timestamp. `set()` closes the previous entry and opens the next from a single clock read, the millisecond race `ViewCollection.createNewView` avoids the same way. Renderer events are stamped in `Assembly.assembleRendererRumEvent`, which needs no Browser SDK change; the bridge's `getUser()` and the identity push exist for what a renderer uploads itself. The main process wins and **replaces** rather than merges: `combine` merges per key and skips `undefined`, so merging `{ id, name }` over `{ id, name, email }` would emit one person's id beside another's email. The deciding reason is `clearUser()` — a stale renderer identity surviving a logout would leave the user's name and email on everything that window kept reporting. `usr.anonymous_id` survives the replacement, and with no user set nothing is touched, so renderer-only `setUser` callers are unaffected. Two things dev verification caught, neither visible from unit tests: - **Views resolve at emit time, not at their start time.** A view is an interval, re-reported as it grows, and the backend takes the session's identity from the last view row (`lastView.UserID`). The main process emits one synthetic view per session and `setUser` cannot run before `init`, so resolving at the view start kept `t_sessions.usr_id` empty for the whole session while the errors beside it carried the identity. - **A restored history is closed at startup.** Quitting is not logging out, so the previous run's entry is still open on disk; restoring it active attributed this process's events to whoever used the machine last. Closing at exactly `now` was not enough — `find` treats `endTime` as inclusive and the first view is created in the same millisecond as `init`, so a second run stamped its opening view with the first run's user. Closed one millisecond earlier, which is also the truthful bound. Testing: `yarn test:unit` 657, `yarn test:e2e` 46, `yarn test:integration` 24. Every behaviour above has a guard that fails without it — 24 reverts applied one at a time, all caught. `yarn typecheck`, `format:check`, `test:e2e:typecheck` clean; `lint` still reports only the two pre-existing errors on `publish`. Verified against dev (appid `mKESnRV4wGs5nwcbTwotmW`): `t_sessions` shows `usr_id` and `usr_anonymous_id` populated together, `t_errors` carries id, name and email, and a second run on the same profile no longer inherits the first run's identity. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 + README.md | 64 +++++- docs/ARCHITECTURE.md | 25 ++- e2e/app/src/main.ts | 14 ++ e2e/app/src/preload.ts | 3 + e2e/lib/bridgeWindowPage.ts | 6 + e2e/lib/mainPage.ts | 17 +- e2e/scenarios/user.scenario.ts | 118 ++++++++++ src/assembly/Assembly.spec.ts | 141 +++++++++++- src/assembly/Assembly.ts | 47 +++- src/assembly/commonContext.spec.ts | 132 ++++++++++- src/assembly/commonContext.ts | 36 ++- src/bridge/BridgeHandler.spec.ts | 50 +++++ src/bridge/BridgeHandler.ts | 26 ++- src/common/bridge.types.ts | 9 + src/domain/UserContext.spec.ts | 350 +++++++++++++++++++++++++++++ src/domain/UserContext.ts | 191 ++++++++++++++++ src/event/event.constants.ts | 1 + src/event/event.types.ts | 8 +- src/index.ts | 60 ++++- src/preload/preloadScript.spec.ts | 54 +++++ src/preload/preloadScript.ts | 26 ++- 22 files changed, 1346 insertions(+), 38 deletions(-) create mode 100644 e2e/scenarios/user.scenario.ts create mode 100644 src/domain/UserContext.spec.ts create mode 100644 src/domain/UserContext.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2a2f0f38..2feff20f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ First FlashCat release. Forked from `@datadog/electron-sdk` v0.3.0 and rebranded - New `normalizeStackPath` option: rewrite a frame's absolute path yourself, before the built-in normalization runs, for build layouts a single application root cannot express (e.g. emitting to `/public/dist` but uploading under `/dist`). Returning `undefined` falls through to `app:///`. It applies to main-process and renderer frames alike, and a callback that throws is reported as an SDK error and falls back to the built-in behaviour. See the README. +- New `setUser` / `getUser` / `clearUser`: identify the logged-in user from the main process. The identity is attached to main-process events and to the renderer events that arrive over the bridge, and is served to renderers through `DatadogEventBridge.getUser()` for what they upload themselves. `id` is required; only `id`, `name` and `email` are read. The names match `flashcatRum.setUser()` in `@flashcatcloud/browser-rum` so both processes of an application share one vocabulary. See the README. + + `usr.anonymous_id` is untouched by all three, and `usr.id` is still never backfilled with it: the two coexist so unique users can be counted off `COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))` across a login. `clearUser` removes `usr.id` rather than blanking it, since `NULLIF(usr_id, '')` distinguishes an absent field from an empty string. + + An identity set in the main process takes precedence over one set in a renderer, and replaces it wholesale rather than merging field by field — a merge could emit one person's id beside another's email. Applications that only call `flashcatRum.setUser()` in their renderers are unaffected. + ### ⚠️ Breaking Changes / Notes - Package renamed to `@flashcatcloud/electron-sdk` (internal `dd-`/`Datadog` names and the `DatadogEventBridge` global are kept per the fork convention). diff --git a/README.md b/README.md index 8d4d70a0..5078bf69 100644 --- a/README.md +++ b/README.md @@ -138,16 +138,20 @@ retry. ##### Identifiers the bridge answers -`window.DatadogEventBridge` exposes the two identifiers the main process owns, so a renderer can -attribute anything it uploads itself to the same session and device: +`window.DatadogEventBridge` exposes the identifiers the main process owns, so a renderer can +attribute anything it uploads itself to the same session, device and user: | Method | Returns | | ------------------ | ----------------------------------------------------------------------------------------------------- | | `getSessionId()` | Id of the session the main process considers active, or `''` while none is (expired, not renewed yet) | | `getAnonymousId()` | Device-scoped id, generated once and kept under `app.getPath('userData')` across restarts | +| `getUser()` | Identity set through [`setUser`](#setuseruser-user-void) as JSON, or `'{}'` when nobody is logged in | -Both answer synchronously and without IPC: the anonymous id is delivered when the bridge is set up, -and the main process pushes every session change to open renderers. +All three answer synchronously and without IPC: the anonymous id is delivered when the bridge is set +up, and the main process pushes every session and identity change to open renderers. + +Renderer events do not need `getUser()` — the main process stamps the identity on them as they pass +through. It is there for what a renderer uploads itself, such as Session Replay segments. ##### How to find your events @@ -333,6 +337,58 @@ try { } ``` +### `setUser(user: User): void` + +Identify the logged-in user. The identity is attached to every subsequent main-process event, and +to the renderer events that reach the main process over the bridge. + +```ts +import { setUser, getUser, clearUser } from '@flashcatcloud/electron-sdk'; + +setUser({ id: 'user-123', name: 'Alice', email: 'alice@example.com' }); + +// Later, when the user logs out: +clearUser(); +``` + +```ts +interface User { + /** Required. */ + id: string; + name?: string; + email?: string; +} +``` + +`id` is required: a call without one is ignored with a warning, as is one whose `name` or `email` is +not a string — a half-applied identity is harder to notice than none at all. Only `id`, `name` and +`email` are read; any other property is dropped. + +This does **not** touch `usr.anonymous_id`. The two identifiers coexist by design: the anonymous id +is device-scoped and stable across logins, and unique users are counted off it first. Before the +first `setUser`, events carry `usr.anonymous_id` and no `usr.id` at all — the SDK never backfills +one with the other. + +The name matches `flashcatRum.setUser()` in `@flashcatcloud/browser-rum`, so both processes of the +same application use one vocabulary. When the main process has an identity, it takes precedence over +one set in a renderer, and it **replaces** it rather than merging field by field — see +`docs/ARCHITECTURE.md`. When it has none, renderer identities are left exactly as they arrive. + +### `getUser(): User | undefined` + +The identity currently set through `setUser`, or `undefined` when nobody is logged in. Returns a +copy — mutating it changes nothing. + +### `clearUser(): void` + +Forget the identity, for instance on logout. Subsequent events carry no `usr.id` **at all**, rather +than an empty one: unique users are counted off `NULLIF(usr_id, '')`, where an absent field and an +empty string are different rows. `usr.anonymous_id` is unaffected — the device is still the same +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. + ### `startOperation(name: string, options?: FeatureOperationOptions): void` Start a RUM Operation step. Pair every `startOperation` with exactly one `succeedOperation` or `failOperation`. Use `options.operationKey` to distinguish parallel operations sharing the same `name`. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 8283e9e9..2861dbe0 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -206,7 +206,25 @@ The device-scoped anonymous id (`src/domain/AnonymousId.ts`) is generated once a **`usr.id` is never backfilled with it.** Electron is counted off `COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))`, which reads the anonymous id first and is stable across a login. The browser SDK does backfill, because its count is `COUNT(DISTINCT usr_id)` and that is the only way it can see logged-out users; copying that here would buy nothing and would split one device into two people at login, when `usr.id` flips from the anonymous id to the real one. `src/assembly/commonContext.spec.ts` pins this down. -Renderer events are left alone: the renderer reads the same id off the bridge itself, so the main process must not stamp a second one over it. +Renderer events keep the `anonymous_id` they arrive with: the renderer reads the same id off the bridge itself, so the main process must not stamp a second one over it. + +#### The logged-in identity (`setUser`) + +`setUser` / `getUser` / `clearUser` (`src/domain/UserContext.ts`) record who the application says is logged in, next to — never instead of — the anonymous id. The names match `flashcatRum.setUser()` in `@flashcatcloud/browser-rum` so both processes of one application share a vocabulary; upstream's `@datadog/electron-sdk` calls it `setUserInfo`, a name Datadog's own browser SDK has since dropped, and we do not track that fork. + +Only `id`, `name` and `email` are copied out of the caller's object, and `id` is required. Dropping everything else is what makes `setUser({ anonymous_id })` structurally impossible; the hook also writes `anonymous_id` last, so the guarantee does not rest on the sanitizer alone. Upstream's `ContextManager.filterReservedKeys` excludes only the standard fields from its free-form `extraInfo` bag, so `addUserExtraInfo({ anonymous_id })` there overwrites the device id — **if `extraInfo` is ever added here, `anonymous_id` has to be reserved with it.** + +An invalid call is rejected whole rather than half-applied, and does not notify renderers: a partly-applied identity is harder to notice than none. + +**Two questions, two stores.** `get()` answers "who is logged in now" from a plain field, so `clearUser()` takes effect in the millisecond it happens. Event assembly instead asks `find(startTime)`, backed by a `DiskValueHistory` like `ViewContext`'s, because a main-process event is not always assembled in the moment it describes — a native crash is parsed on the _next_ startup carrying the crash's own timestamp, and `addError` accepts a caller-supplied `startTime`. Resolving those at assembly time would hand one user's crash to another. `set()` closes the previous entry and opens the next from a **single** clock read, the millisecond race `ViewCollection.createNewView` avoids the same way. + +Persistence has a consequence worth knowing: the identity is written to `userData` in plain text, beside the anonymous id and the session file. + +**View events are the exception: they resolve at the moment they are emitted.** A view is an interval, re-reported as it grows (`_dd.document_version`), and the backend derives the session's identity from the _last_ view row it receives (`buildSessionViewUpdates` in fc-rum reads `lastView.UserID`). The main process emits exactly one synthetic view per session, spanning the whole session, and `setUser` cannot run before `init` — so resolving that view at its start time would keep the identity off `t_sessions.usr_id` for the entire session. Verified against dev: without this the session row carried an empty `usr_id` while the error events beside it carried the full identity. + +**A restored history is closed at startup, one millisecond before now.** Quitting is not logging out, so a previous run normally leaves its entry open; restoring it still-active attributed this process's events to whoever used the machine last. Closing it at exactly `now` is not enough, because `find` treats `endTime` as inclusive and the first main-process view is created in the same millisecond as `init` — observed against dev, where a second run stamped its opening view with the first run's user. Earlier timestamps still resolve to that user, which is what a crash from the previous run needs. + +**`clearUser` removes `usr.id`; it never blanks it.** `NULLIF(usr_id, '')` treats an empty string and an absent field as different rows. ### Renderer identifiers @@ -214,6 +232,11 @@ The renderer needs the main process's session id to attribute anything it upload - `getAnonymousId()` — the id above. It never changes, so the synchronous config channel carries it once. - `getSessionId()` — the session the main process considers active, or `''` while none is. Sessions expire and renew, so the main process **pushes** every change over `datadog:bridge-identity` to the renderers that asked for a configuration; the preload caches the value and answers from the cache. A synchronous IPC call per event would be far too slow. +- `getUser()` — the identity set through `setUser`, as JSON, or `'{}'` when nobody is logged in. A string rather than an object, matching `getCapabilities` and `getAllowedWebViewHosts`; it rides the same identity channel and the same preload cache as the session id. + +**The bridge getter is not how bridged renderer events get their identity.** The Browser SDK's bridge contract has no user getter, so nothing would read it today; `Assembly.assembleRendererRumEvent` stamps the identity on renderer events as they pass through the main process instead, which needs no Browser SDK change. The getter is there so a renderer can attribute what it uploads _itself_ — Session Replay segments — to the same person, and is what a future Browser SDK would consume. + +**The main process wins, and it replaces rather than merges.** `combine` merges per key and skips `undefined`, so merging a main-process `{ id, name }` over a renderer's `{ id, name, email }` would emit one person's id and name beside another's email — an identity belonging to nobody, and worse than either source alone. The deciding reason for main-process precedence is `clearUser()`: if a stale renderer-side identity could survive it, logging out would leave the user's name and email on everything that window kept reporting, so a logout has to be enforceable from one place. `usr.anonymous_id` is carried across the replacement untouched, and when no user is set nothing is touched at all — an application that only calls `flashcatRum.setUser()` in its renderers keeps the behaviour it had. **`''` is load-bearing, and is not the same as not implementing the getter.** The Browser SDK reads an empty answer as "the host has no session right now" and stops attributing data until the host answers with an id again; it only falls back to its own placeholder session id for a host too old to implement `getSessionId()` at all. That distinction is what keeps Session Replay off a fake session: the renderer uploads its segments itself instead of handing them to the main process, so nothing here can discard them after the fact, and a placeholder id is a constant every application built on this SDK would share. It also means the main process must never answer with the id an expired session used to have — that would attach segments to a session that has ended. `getActiveSessionId` in `src/index.ts` answers `''` for anything but an active session, and `bridge-window.scenario.ts` pins both the expiry and the renewal down. diff --git a/e2e/app/src/main.ts b/e2e/app/src/main.ts index 71ca1209..ca708d43 100644 --- a/e2e/app/src/main.ts +++ b/e2e/app/src/main.ts @@ -9,10 +9,14 @@ import { _generateTelemetryError, _flushTransport, stopSession, + setUser, + getUser, + clearUser, startOperation, succeedOperation, failOperation, type FailureReason, + type User, type FeatureOperationOptions, type InitConfiguration, } from '@flashcatcloud/electron-sdk'; @@ -78,6 +82,16 @@ void app.whenReady().then(async () => { stopSession(); }); + ipcMain.handle('setUser', (_event, user: User) => { + setUser(user); + }); + + ipcMain.handle('getUser', () => getUser()); + + ipcMain.handle('clearUser', () => { + clearUser(); + }); + ipcMain.handle('generateUncaughtException', () => { setTimeout(() => { throw new Error('test uncaught exception'); diff --git a/e2e/app/src/preload.ts b/e2e/app/src/preload.ts index 1389eb46..628418c4 100644 --- a/e2e/app/src/preload.ts +++ b/e2e/app/src/preload.ts @@ -8,6 +8,9 @@ contextBridge.exposeInMainWorld('e2eConfig', { rumBrowserSdk: rumBrowserSdkConfi contextBridge.exposeInMainWorld('electronAPI', { generateTelemetryErrors: (count: number) => ipcRenderer.invoke('generateTelemetryErrors', count), stopSession: () => ipcRenderer.invoke('stopSession'), + setUser: (user: Record) => ipcRenderer.invoke('setUser', user), + getUser: () => ipcRenderer.invoke('getUser'), + clearUser: () => ipcRenderer.invoke('clearUser'), generateUncaughtException: () => ipcRenderer.invoke('generateUncaughtException'), generateUnhandledRejection: () => ipcRenderer.invoke('generateUnhandledRejection'), generateManualError: (startTime?: number) => ipcRenderer.invoke('generateManualError', startTime), diff --git a/e2e/lib/bridgeWindowPage.ts b/e2e/lib/bridgeWindowPage.ts index e6b4aece..025ba47f 100644 --- a/e2e/lib/bridgeWindowPage.ts +++ b/e2e/lib/bridgeWindowPage.ts @@ -6,6 +6,7 @@ interface BridgeWindow { getCapabilities: () => string; getSessionId: () => string; getAnonymousId: () => string; + getUser: () => string; }; } @@ -46,6 +47,11 @@ export class BridgeWindowPage { return await this.page.evaluate(() => (globalThis as unknown as BridgeWindow).DatadogEventBridge.getAnonymousId()); } + /** The identity the main process set, as the bridge hands it over: JSON, `'{}'` when none. */ + async getUser(): Promise { + return await this.page.evaluate(() => (globalThis as unknown as BridgeWindow).DatadogEventBridge.getUser()); + } + async getCapabilities(): Promise { return await this.page.evaluate(() => (globalThis as unknown as BridgeWindow).DatadogEventBridge.getCapabilities()); } diff --git a/e2e/lib/mainPage.ts b/e2e/lib/mainPage.ts index 9e302abc..98821d5d 100644 --- a/e2e/lib/mainPage.ts +++ b/e2e/lib/mainPage.ts @@ -1,5 +1,5 @@ import type { ElectronApplication, Page } from '@playwright/test'; -import type { FailureReason, FeatureOperationOptions } from '@flashcatcloud/electron-sdk'; +import type { FailureReason, FeatureOperationOptions, User } from '@flashcatcloud/electron-sdk'; import { BridgeWindowPage } from './bridgeWindowPage'; // declare exposed IPC methods called directly in tests @@ -7,6 +7,9 @@ interface ElectronAppWindow { electronAPI: { generateTelemetryErrors: (count: number) => Promise; generateManualError: (startTime?: number) => Promise; + setUser: (user: User) => Promise; + getUser: () => Promise; + clearUser: () => Promise; startOperation: (name: string, options?: FeatureOperationOptions) => Promise; succeedOperation: (name: string, options?: FeatureOperationOptions) => Promise; failOperation: (name: string, failureReason: FailureReason, options?: FeatureOperationOptions) => Promise; @@ -73,6 +76,18 @@ export class MainPage { ); } + async setUser(user: User) { + await this.page.evaluate((u) => (globalThis as unknown as ElectronAppWindow).electronAPI.setUser(u), user); + } + + async getUser(): Promise { + return this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.getUser()); + } + + async clearUser() { + await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.clearUser()); + } + async startOperation(name: string, options?: FeatureOperationOptions) { await this.page.evaluate( ({ name, options }) => (globalThis as unknown as ElectronAppWindow).electronAPI.startOperation(name, options), diff --git a/e2e/scenarios/user.scenario.ts b/e2e/scenarios/user.scenario.ts new file mode 100644 index 00000000..fa8327cd --- /dev/null +++ b/e2e/scenarios/user.scenario.ts @@ -0,0 +1,118 @@ +import { test, expect } from '../lib/helpers'; +import type { RumErrorEvent, RumViewEvent } from '@flashcatcloud/electron-sdk'; + +const ALICE = { id: 'e2e-user-alice', name: 'Alice', email: 'alice@example.com' }; + +const isMainProcessView = (event: { body: unknown }) => + (event.body as RumViewEvent).view.url === 'electron://main-process'; + +const isBridgeView = (event: { body: unknown }) => (event.body as RumViewEvent).view.url !== 'electron://main-process'; + +/** The bridge hands the identity over as JSON; `'{}'` means nobody is logged in. */ +const parseBridgeUser = (json: string) => JSON.parse(json) as Record; + +test.describe('setUser — main process events', () => { + test('reports no usr.id until the application identifies the user', async ({ mainPage, intake }) => { + await mainPage.flushTransport(); + + const view = (await intake.getEventsByType('view')).find(isMainProcessView)!.body as RumViewEvent; + + // The device is known, the person is not. `usr.id` has to be absent rather than backfilled + // with the anonymous id: unique users are counted off + // `COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))`. + expect(view.usr?.anonymous_id).toBeTruthy(); + expect(view.usr?.id).toBeUndefined(); + }); + + test('attaches the identity to subsequent events, alongside the anonymous id', async ({ mainPage, intake }) => { + await mainPage.flushTransport(); + const before = (await intake.getEventsByType('view')).find(isMainProcessView)!.body as RumViewEvent; + const anonymousId = before.usr?.anonymous_id; + + await mainPage.setUser(ALICE); + await mainPage.generateManualError(); + await mainPage.flushTransport(); + + const error = (await intake.waitForEventCount('error', 1))[0].body as RumErrorEvent; + expect(error.usr?.id).toBe(ALICE.id); + expect(error.usr?.name).toBe(ALICE.name); + expect(error.usr?.email).toBe(ALICE.email); + // Both identifiers travel together — this is the pair the backend counts on. + expect(error.usr?.anonymous_id).toBe(anonymousId); + }); + + test('reads back the identity it was given', async ({ mainPage }) => { + await mainPage.setUser(ALICE); + + expect(await mainPage.getUser()).toEqual(ALICE); + }); + + test('removes usr.id on logout without disturbing the anonymous id', async ({ mainPage, intake }) => { + await mainPage.setUser(ALICE); + await mainPage.generateManualError(); + await mainPage.flushTransport(); + const identified = (await intake.waitForEventCount('error', 1))[0].body as RumErrorEvent; + + await mainPage.clearUser(); + await mainPage.generateManualError(); + await mainPage.flushTransport(); + + const errors = await intake.waitForEventCount('error', 2); + const afterLogout = errors[errors.length - 1].body as RumErrorEvent; + + expect(await mainPage.getUser()).toBeFalsy(); + // Absent, not empty: `NULLIF(usr_id, '')` treats the two differently. + expect(afterLogout.usr).not.toHaveProperty('id'); + expect(afterLogout.usr?.anonymous_id).toBe(identified.usr?.anonymous_id); + }); + + test('ignores a call without an id, keeping the identity already in force', async ({ mainPage }) => { + await mainPage.setUser(ALICE); + + await mainPage.setUser({ name: 'Nobody' } as unknown as typeof ALICE); + + expect(await mainPage.getUser()).toEqual(ALICE); + }); +}); + +test.describe('setUser — renderer events over the bridge', () => { + test('stamps the main process identity on bridged renderer events', async ({ electronApp, mainPage, intake }) => { + await mainPage.flushTransport(); + await intake.getEventsByType('view'); + + await mainPage.setUser(ALICE); + await mainPage.openBridgeFileWindow(electronApp); + await mainPage.flushTransport(); + + const view = (await intake.waitForEventCount('view', 1, { predicate: isBridgeView }))[0].body as RumViewEvent; + + expect(view.container?.source).toBe('electron'); + expect(view.usr?.id).toBe(ALICE.id); + }); + + test('serves the identity to the renderer through the bridge', async ({ electronApp, mainPage }) => { + await mainPage.setUser(ALICE); + + const bridgeWindow = await mainPage.openBridgeFileWindow(electronApp); + + expect(parseBridgeUser(await bridgeWindow.getUser())).toEqual(ALICE); + }); + + test('pushes a later identity change to an already open renderer', async ({ electronApp, mainPage }) => { + const bridgeWindow = await mainPage.openBridgeFileWindow(electronApp); + expect(await bridgeWindow.getUser()).toBe('{}'); + + await mainPage.setUser(ALICE); + + await expect.poll(async () => parseBridgeUser(await bridgeWindow.getUser())).toEqual(ALICE); + }); + + test('pushes a logout to an already open renderer', async ({ electronApp, mainPage }) => { + await mainPage.setUser(ALICE); + const bridgeWindow = await mainPage.openBridgeFileWindow(electronApp); + + await mainPage.clearUser(); + + await expect.poll(async () => bridgeWindow.getUser()).toBe('{}'); + }); +}); diff --git a/src/assembly/Assembly.spec.ts b/src/assembly/Assembly.spec.ts index cd4d9550..2df27074 100644 --- a/src/assembly/Assembly.spec.ts +++ b/src/assembly/Assembly.spec.ts @@ -13,6 +13,7 @@ import { type ServerEvent, } from '../event'; import type { RumEvent, RawRumData } from '../domain/rum'; +import type { User } from '../domain/UserContext'; import { createTestConfiguration } from '../mocks.specUtil'; const RAW_ERROR_DATA: RawRumData = { @@ -90,7 +91,7 @@ describe('Assembly', () => { }); describe('Assembly — renderer events', () => { - function setup() { + function setup(getUser: () => User | undefined = () => undefined) { const config = createTestConfiguration({ applicationId: 'main-app-id', service: 'main-service' }); const eventManager = new EventManager(); const hooks = createFormatHooks(); @@ -98,7 +99,7 @@ describe('Assembly — renderer events', () => { registerCommonContext(config, hooks, 'device-anonymous-id'); hooks.registerRum(() => ({ session: { id: 'main-session-id' }, view: { id: 'main-view-id' } })); - new Assembly(eventManager, hooks); + new Assembly(eventManager, hooks, getUser); return { eventManager, hooks }; } @@ -173,6 +174,142 @@ describe('Assembly — renderer events', () => { expect(collected[0].track).toBe(EventTrack.RUM); }); + describe('user identity', () => { + const ALICE: User = { id: 'alice', name: 'Alice', email: 'alice@example.com' }; + + function assembleRendererEvent(getUser: () => User | undefined, usr?: Record): RumEvent { + const { eventManager } = setup(getUser); + 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: 'error', + source: 'browser', + date: 12345 as TimeStamp, + error: { message: 'renderer error', source: 'source' }, + view: { id: 'renderer-view' }, + session: { id: 'renderer-session' }, + application: { id: 'renderer-app' }, + ...(usr ? { usr } : {}), + }, + } as unknown as RawRumEvent); + + return collected[0].data as RumEvent; + } + + it('should leave a renderer event alone when no identity is set', () => { + const data = assembleRendererEvent(() => undefined, { id: 'renderer-user', anonymous_id: 'device-id' }); + + expect(data.usr).toEqual({ id: 'renderer-user', anonymous_id: 'device-id' }); + }); + + it('should not invent a usr on a renderer event that carries none', () => { + const data = assembleRendererEvent(() => undefined); + + expect(data.usr).toBeUndefined(); + }); + + it('should stamp the main process identity on a renderer event that carries none', () => { + const data = assembleRendererEvent(() => ALICE); + + expect(data.usr).toEqual(ALICE); + }); + + /** + * The reason this replaces rather than merges. `combine` merges per key and skips `undefined`, + * so a merge of `{ id, name }` over `{ id, name, email }` would emit Alice's id and name beside + * Bob's email — an identity belonging to nobody, and worse than either source alone. + */ + it('should replace the renderer identity wholesale, never stitch the two together', () => { + const data = assembleRendererEvent(() => ({ id: 'alice', name: 'Alice' }), { + id: 'bob', + name: 'Bob', + email: 'bob@example.com', + }); + + expect(data.usr).toEqual({ id: 'alice', name: 'Alice' }); + expect(data.usr).not.toHaveProperty('email'); + }); + + /** + * The logout guarantee. If a stale renderer-side identity could outlive `clearUser()`, logging + * out would leave the user's name and email on everything that window kept reporting. + */ + it('should drop a renderer identity that the main process has cleared', () => { + const data = assembleRendererEvent(() => ({ id: 'alice' }), { id: 'bob', email: 'bob@example.com' }); + + expect(data.usr).toEqual({ id: 'alice' }); + }); + + it('should carry the anonymous id across the replacement untouched', () => { + const data = assembleRendererEvent(() => ALICE, { id: 'bob', anonymous_id: 'device-id' }); + + expect(data.usr?.anonymous_id).toBe('device-id'); + expect(data.usr?.id).toBe('alice'); + }); + + it('should not stamp the main process anonymous id on a renderer event', () => { + const data = assembleRendererEvent(() => ALICE); + + expect(data.usr).not.toHaveProperty('anonymous_id'); + }); + + it('should resolve the identity as of the event date, not of assembly time', () => { + const seen: number[] = []; + assembleRendererEvent((startTime?: unknown) => { + seen.push(startTime as number); + return ALICE; + }); + + expect(seen).toContain(12345); + }); + + /** + * Same rule as the main-process view: a page view that spans a login is re-reported, and the + * backend takes the session's identity from the last view row. + */ + it('should resolve a renderer view at emit time, not at the view start', () => { + const viewStart = 1000; + const asked: number[] = []; + const { eventManager } = setup(((startTime: TimeStamp) => { + asked.push(startTime); + return ALICE; + }) as unknown as () => User | undefined); + + eventManager.notify({ + kind: EventKind.RAW, + source: EventSource.RENDERER, + format: EventFormat.RUM, + data: { + type: 'view', + source: 'browser', + date: viewStart as TimeStamp, + view: { id: 'renderer-view' }, + session: { id: 'renderer-session' }, + application: { id: 'renderer-app' }, + }, + } as unknown as RawRumEvent); + + expect(asked).not.toHaveLength(0); + expect(asked).not.toContain(viewStart); + }); + + it('should leave the rest of the event untouched', () => { + const data = assembleRendererEvent(() => ALICE, { id: 'bob' }); + + expect(data.session.id).toBe('main-session-id'); + expect(data.view.id).toBe('renderer-view'); + expect(data.source).toBe('browser'); + }); + }); + it('passes event.data.date as startTime for hook context resolution', () => { const { eventManager, hooks } = setup(); let capturedStartTime: TimeStamp | undefined; diff --git a/src/assembly/Assembly.ts b/src/assembly/Assembly.ts index f17523a6..2ec54cc4 100644 --- a/src/assembly/Assembly.ts +++ b/src/assembly/Assembly.ts @@ -3,6 +3,7 @@ import type { RecursivePartial } from '../tools/coreCompat'; import { EventFormat, EventKind, EventManager, EventSource, EventTrack, type RawEvent, ServerEvent } from '../event'; import type { RawRumEvent } from '../event'; import type { FormatHooks } from './hooks'; +import { resolveEventUser, type User } from '../domain/UserContext'; import { RumEvent } from '../domain/rum'; import { TelemetryEvent } from '../domain/telemetry'; @@ -21,7 +22,12 @@ import { TelemetryEvent } from '../domain/telemetry'; export class Assembly { constructor( private eventManager: EventManager, - private hooks: FormatHooks + private hooks: FormatHooks, + /** + * Identity in force at an event's start time — see `UserContext`. Defaults to "never anyone", + * so a caller that does not wire it up leaves renderer events exactly as they arrived. + */ + private getUser: (startTime: TimeStamp) => User | undefined = () => undefined ) { this.eventManager.registerHandler({ canHandle: (event) => event.kind === EventKind.RAW, @@ -66,15 +72,52 @@ export class Assembly { container: { view: { id: view?.id }, source: 'electron' }, }; + // Note `usr` is not in `mainProcessAttributes`: the anonymous id must not be stamped here (the + // renderer reads the same id off the bridge itself), and the identity needs replacing rather + // than merging — see below. + const data = combine(event.data, mainProcessAttributes) as RumEvent; + return { kind: EventKind.SERVER, track: EventTrack.RUM, source: EventSource.RENDERER, // override some renderer event attributes by main process attributes - data: combine(event.data, mainProcessAttributes) as RumEvent, + data: this.applyUserIdentity(data), }; } + /** + * Replace a bridged event's identity with the main process's, when one is set. + * + * **Replacement, not a merge.** `combine` merges per key and skips `undefined`, so merging a + * main-process `{ id, name }` over a renderer's `{ id, name, email }` would emit the main + * process's id and name next to the previous user's email — an identity that belongs to nobody, + * and worse than either source alone. + * + * **The main process wins.** It is the one place that knows who is logged in, which is why it + * already overrides `session.id` and `application.id` on the way through. The deciding reason is + * `clearUser()`: if a stale renderer-side identity could survive it, logging out would leave the + * user's name and email attached to everything the window kept reporting. A logout has to be + * enforceable from one place. + * + * `usr.anonymous_id` is carried over untouched — it is device-scoped, the renderer took it from + * this same bridge, and it has to stay put across a login and a logout. + * + * When no user is set, nothing is touched, so an application that only ever calls + * `flashcatRum.setUser()` in its renderers keeps the behaviour it had before this existed. + */ + private applyUserIdentity(data: RumEvent): RumEvent { + const user = resolveEventUser(this.getUser, data.type, data.date as TimeStamp); + if (!user) { + return data; + } + + const anonymousId = data.usr?.anonymous_id; + const usr = anonymousId === undefined ? { ...user } : { ...user, anonymous_id: anonymousId }; + + return { ...data, usr } as RumEvent; + } + /** * Main-process events are assembled by combining raw data with the full * hook chain (commonContext, session, view), producing a complete diff --git a/src/assembly/commonContext.spec.ts b/src/assembly/commonContext.spec.ts index 10234893..05f30cce 100644 --- a/src/assembly/commonContext.spec.ts +++ b/src/assembly/commonContext.spec.ts @@ -12,9 +12,11 @@ import { type RawTelemetryEvent, } from '../event'; import type { RumEvent, RawRumData } from '../domain/rum'; +import type { User } from '../domain/UserContext'; import { createTestConfiguration } from '../mocks.specUtil'; const ANONYMOUS_ID = 'device-anonymous-id'; +const ALICE: User = { id: 'alice', name: 'Alice', email: 'alice@example.com' }; const RAW_ERROR_DATA: RawRumData = { type: 'error', @@ -39,17 +41,25 @@ describe('registerCommonContext', () => { return serverEvents[serverEvents.length - 1].data as RumEvent; } + /** Stands in for `UserContext.find`: whatever the test last handed to `login`. */ + let currentUser: User | undefined; + + function login(user: User | undefined) { + currentUser = user; + } + beforeEach(() => { eventManager = new EventManager(); hooks = createFormatHooks(); serverEvents = []; + currentUser = undefined; eventManager.registerHandler({ canHandle: (event): event is ServerEvent => event.kind === EventKind.SERVER, handle: (event) => serverEvents.push(event), }); - registerCommonContext(createTestConfiguration(), hooks, ANONYMOUS_ID); + registerCommonContext(createTestConfiguration(), hooks, ANONYMOUS_ID, () => currentUser); new Assembly(eventManager, hooks); }); @@ -123,6 +133,126 @@ describe('registerCommonContext', () => { expect(lastRumEvent().usr).toBeUndefined(); }); + it('should stamp the identity set through setUser alongside the anonymous id', () => { + login(ALICE); + + notifyMainProcessRumEvent(); + + expect(lastRumEvent().usr).toEqual({ ...ALICE, anonymous_id: ANONYMOUS_ID }); + }); + + it('should stamp only the fields the identity actually carries', () => { + login({ id: 'alice' }); + + notifyMainProcessRumEvent(); + + expect(lastRumEvent().usr).toEqual({ id: 'alice', anonymous_id: ANONYMOUS_ID }); + }); + + /** + * The logout guard. `usr.id` has to disappear rather than turn into `''`: unique users are + * counted off `NULLIF(usr_id, '')`, where an empty string and an absent field are not the same + * row. + */ + it('should drop usr.id once the identity is cleared, rather than blank it', () => { + login(ALICE); + notifyMainProcessRumEvent(); + + login(undefined); + notifyMainProcessRumEvent(); + + const usr = lastRumEvent().usr; + expect(usr).not.toHaveProperty('id'); + expect(Object.keys(usr!)).toEqual(['anonymous_id']); + }); + + it('should keep the anonymous id identical across a login and a logout', () => { + notifyMainProcessRumEvent(); + login(ALICE); + notifyMainProcessRumEvent(); + login(undefined); + notifyMainProcessRumEvent(); + + const anonymousIds = serverEvents.map((event) => (event.data as RumEvent).usr?.anonymous_id); + expect(anonymousIds).toEqual([ANONYMOUS_ID, ANONYMOUS_ID, ANONYMOUS_ID]); + }); + + /** + * `setUser` cannot carry `anonymous_id` — `sanitizeUser` drops it — but the hook also writes + * the anonymous id last so the guarantee does not depend on that. This pins the ordering. + */ + it('should not let an identity displace the anonymous id', () => { + login({ id: 'alice', anonymous_id: 'forged' } as unknown as User); + + notifyMainProcessRumEvent(); + + expect(lastRumEvent().usr?.anonymous_id).toBe(ANONYMOUS_ID); + }); + + it('should resolve the identity as of the event start time, not of assembly time', () => { + const startTimes: number[] = []; + registerCommonContext(createTestConfiguration(), hooks, ANONYMOUS_ID, (startTime) => { + startTimes.push(startTime); + return ALICE; + }); + + eventManager.notify({ + kind: EventKind.RAW, + source: EventSource.MAIN, + format: EventFormat.RUM, + data: RAW_ERROR_DATA, + startTime: 1234, + } as unknown as RawRumEvent); + + expect(startTimes).toContain(1234); + }); + + /** + * The main process emits exactly one synthetic view per session, spanning the whole session, + * and the backend derives the session's identity from the **last** view row it receives. + * `setUser` cannot run before `init`, so that view always begins logged-out — resolving it at + * its start time would keep the identity off `t_sessions.usr_id` for the entire session. + */ + it('should stamp a view with the identity in force when it is emitted, not when the view began', () => { + const viewStart = 1000; + const asked: number[] = []; + registerCommonContext(createTestConfiguration(), hooks, ANONYMOUS_ID, (startTime) => { + asked.push(startTime); + return ALICE; + }); + + eventManager.notify({ + kind: EventKind.RAW, + source: EventSource.MAIN, + format: EventFormat.RUM, + data: { type: 'view', view: { id: 'v1' } }, + startTime: viewStart, + } as unknown as RawRumEvent); + + expect(asked).not.toHaveLength(0); + expect(asked).not.toContain(viewStart); + expect(lastRumEvent().usr?.id).toBe(ALICE.id); + }); + + it('should still resolve a point-in-time event at its own timestamp, so a late crash keeps its user', () => { + const crashMoment = 1000; + const asked: number[] = []; + registerCommonContext(createTestConfiguration(), hooks, ANONYMOUS_ID, (startTime) => { + asked.push(startTime); + return undefined; + }); + + eventManager.notify({ + kind: EventKind.RAW, + source: EventSource.MAIN, + format: EventFormat.RUM, + data: RAW_ERROR_DATA, + startTime: crashMoment, + } as unknown as RawRumEvent); + + expect(asked).toContain(crashMoment); + }); + it('should not stamp telemetry events, whose format has no user properties', () => { eventManager.notify({ kind: EventKind.RAW, diff --git a/src/assembly/commonContext.ts b/src/assembly/commonContext.ts index 47cfbfb9..e9a6ab72 100644 --- a/src/assembly/commonContext.ts +++ b/src/assembly/commonContext.ts @@ -1,27 +1,43 @@ +import type { TimeStamp } from '@flashcatcloud/browser-core'; import type { Configuration } from '../config'; +import { resolveEventUser, type User } from '../domain/UserContext'; import type { FormatHooks } from './hooks'; /** * Define the common attributes for the events of each format * * @param anonymousId device-scoped identifier — see `AnonymousId` + * @param getUser identity in force at an event's start time, or `undefined` when nobody is logged + * in — see `UserContext`. Defaults to "never anyone", so a caller that does not wire it up gets + * the anonymous id alone. */ -export function registerCommonContext(configuration: Configuration, hooks: FormatHooks, anonymousId: string) { - hooks.registerRum(() => ({ +export function registerCommonContext( + configuration: Configuration, + hooks: FormatHooks, + anonymousId: string, + getUser: (startTime: TimeStamp) => User | undefined = () => undefined +) { + hooks.registerRum((params) => ({ date: Date.now(), source: 'electron', service: configuration.service, version: configuration.version, application: { id: configuration.applicationId }, session: { type: 'user' }, - // `anonymous_id` only. Deliberately **not** `usr.id`, and not to be "aligned" with the browser - // SDK later: unique users are counted here off - // `COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))`, which reads the anonymous id - // first, and that id is stable across a login. The browser SDK copies it into `usr.id` because - // its count is `COUNT(DISTINCT usr_id)` and that is the only way it can see logged-out users; - // doing the same here would buy nothing and would count one device as two people, since - // `usr.id` flips from the anonymous id to the real one the moment the user logs in. - usr: { anonymous_id: anonymousId }, + // The anonymous id and the real identity coexist, and the anonymous id is written last so no + // identity can displace it — `setUser` cannot even carry the key, but the ordering makes that + // independent of `sanitizeUser`. + // + // `usr.id` is present **only** once the application calls `setUser`. It is deliberately not + // backfilled with the anonymous id, and not to be "aligned" with the browser SDK later: unique + // users are counted here off `COALESCE(NULLIF(usr_anonymous_id, ''), NULLIF(usr_id, ''))`, + // which reads the anonymous id first, and that id is stable across a login. The browser SDK + // copies it into `usr.id` because its count is `COUNT(DISTINCT usr_id)` and that is the only + // way it can see logged-out users; doing the same here would buy nothing and would count one + // device as two people, since `usr.id` would flip from the anonymous id to the real one the + // moment the user logs in. `NULLIF(usr_id, '')` is also why `clearUser` removes the key + // instead of blanking it — an empty string and an absent field are not the same row. + usr: { ...resolveEventUser(getUser, params.eventType, params.startTime), anonymous_id: anonymousId }, ddtags: `sdk_version:${__SDK_VERSION__}`, _dd: { format_version: 2 }, })); diff --git a/src/bridge/BridgeHandler.spec.ts b/src/bridge/BridgeHandler.spec.ts index 43f6a152..6b65681e 100644 --- a/src/bridge/BridgeHandler.spec.ts +++ b/src/bridge/BridgeHandler.spec.ts @@ -7,6 +7,7 @@ import { BRIDGE_CHANNEL, CONFIG_CHANNEL, IDENTITY_CHANNEL } from '../common'; import { RendererRegistry } from '../domain/RendererRegistry'; import { ViewTimingCorrector } from '../domain/ViewTimingCorrector'; import { StackPathNormalizer } from '../domain/StackPathNormalizer'; +import type { User } from '../domain/UserContext'; const { mockIpcMainOn, mockAddError } = vi.hoisted(() => { const mockIpcMainOn = vi.fn(); @@ -63,6 +64,7 @@ describe('BridgeHandler', () => { let eventManager: EventManager; let rendererRegistry: RendererRegistry; let sessionId: string; + let user: User | undefined; /** `senderId: null` simulates an IPC event without a `sender` (e.g. a destroyed webContents). */ let simulateIpcMessage: (msg: string, senderId?: number | null) => void; /** Replays a renderer's synchronous configuration request, and returns what it got back. */ @@ -73,6 +75,7 @@ describe('BridgeHandler', () => { eventManager = new EventManager(); rendererRegistry = new RendererRegistry(); sessionId = 'session-1'; + user = undefined; mockIpcMainOn.mockImplementation((channel: string, callback: IpcCallback) => { if (channel === BRIDGE_CHANNEL) { @@ -94,6 +97,7 @@ describe('BridgeHandler', () => { eventManager, DEFAULT_BRIDGE_OPTIONS, () => sessionId, + () => user, rendererRegistry, new ViewTimingCorrector(rendererRegistry, true), new StackPathNormalizer(true, APP_ROOT) @@ -130,6 +134,16 @@ describe('BridgeHandler', () => { expect(simulateConfigRequest()).toMatchObject({ sessionId: 'session-2' }); }); + it('should answer with the identity of the moment, so a renderer opened after login starts with it', () => { + user = { id: 'alice', name: 'Alice' }; + + expect(simulateConfigRequest()).toMatchObject({ user: { id: 'alice', name: 'Alice' } }); + }); + + it('should answer without a user when nobody is logged in', () => { + expect(simulateConfigRequest()).not.toHaveProperty('user.id'); + }); + it('should answer a renderer that has no sender', () => { const ipcEvent = { returnValue: undefined as unknown }; const configHandler = mockIpcMainOn.mock.calls.find(([channel]) => channel === CONFIG_CHANNEL)![1] as ( @@ -165,6 +179,42 @@ describe('BridgeHandler', () => { expect(sender.send).toHaveBeenCalledWith(IDENTITY_CHANNEL, { sessionId: '' }); }); + it('should push the identity when it changes, reusing the session channel', () => { + const sender = createSender(); + simulateConfigRequest(sender); + + user = { id: 'alice', name: 'Alice' }; + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.USER_CHANGED }); + + expect(sender.send).toHaveBeenCalledWith(IDENTITY_CHANNEL, { + sessionId: 'session-1', + user: { id: 'alice', name: 'Alice' }, + }); + }); + + it('should push an absent user once the identity is cleared', () => { + const sender = createSender(); + simulateConfigRequest(sender); + user = { id: 'alice' }; + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.USER_CHANGED }); + + user = undefined; + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.USER_CHANGED }); + + expect(sender.send).toHaveBeenLastCalledWith(IDENTITY_CHANNEL, { sessionId: 'session-1', user: undefined }); + }); + + it('should carry the identity alongside a session renewal', () => { + const sender = createSender(); + simulateConfigRequest(sender); + user = { id: 'alice' }; + + sessionId = 'session-2'; + eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.SESSION_RENEW }); + + expect(sender.send).toHaveBeenCalledWith(IDENTITY_CHANNEL, { sessionId: 'session-2', user: { id: 'alice' } }); + }); + it('should not push on unrelated lifecycle events', () => { const sender = createSender(); simulateConfigRequest(sender); diff --git a/src/bridge/BridgeHandler.ts b/src/bridge/BridgeHandler.ts index 267d17b5..3dcd5adf 100644 --- a/src/bridge/BridgeHandler.ts +++ b/src/bridge/BridgeHandler.ts @@ -6,6 +6,7 @@ import { monitor, addError as addTelemetryError } from '../domain/telemetry'; import { BRIDGE_CHANNEL, CONFIG_CHANNEL, IDENTITY_CHANNEL } from '../common'; import type { BridgeConfig, IdentityUpdate } from '../common'; import type { RendererRegistry } from '../domain/RendererRegistry'; +import type { User } from '../domain/UserContext'; import type { ViewTimingCorrector } from '../domain/ViewTimingCorrector'; import type { StackPathNormalizer } from '../domain/StackPathNormalizer'; @@ -23,10 +24,10 @@ interface BridgedRumEvent { /** * The part of the bridge configuration that is fixed for the lifetime of the SDK — everything the - * preload reads except the session id, which `buildConfig` adds as of the moment it is asked. - * Derived from `BridgeConfig` so the two cannot drift as fields are added. + * preload reads except the session id and the user, which `buildConfig` adds as of the moment it + * is asked. Derived from `BridgeConfig` so the two cannot drift as fields are added. */ -export type BridgeOptions = Omit; +export type BridgeOptions = Omit; /** * Receives events from renderer processes via IPC and routes them through the @@ -39,9 +40,13 @@ export type BridgeOptions = Omit; * chain. * * It also answers the renderers' identity questions. `anonymousId` never changes, so the - * synchronous config channel carries it once; `sessionId` does, so it is pushed to every renderer - * known to have a bridge whenever it changes — asking for it synchronously per event would be far - * too slow. + * synchronous config channel carries it once; `sessionId` and the `setUser` identity do, so they + * are pushed to every renderer known to have a bridge whenever they change — asking for them + * synchronously per event would be far too slow. + * + * The identity push is what renderers *read*; it is not how bridged renderer events get their + * identity. Those are stamped in `Assembly.assembleRendererRumEvent`, because the Browser SDK's + * bridge contract has no user getter yet and an event must not depend on one existing. */ export class BridgeHandler { /** Renderers that asked for the configuration, and so hold a preload cache to keep up to date. */ @@ -51,6 +56,7 @@ export class BridgeHandler { private readonly eventManager: EventManager, private readonly bridgeOptions: BridgeOptions, private readonly getSessionId: () => string, + private readonly getUser: () => User | undefined, private readonly rendererRegistry: RendererRegistry, private readonly viewTimingCorrector: ViewTimingCorrector, private readonly stackPathNormalizer: StackPathNormalizer @@ -73,7 +79,9 @@ export class BridgeHandler { this.eventManager.registerHandler({ canHandle: (event): event is LifecycleEvent => event.kind === EventKind.LIFECYCLE && - (event.lifecycle === LifecycleKind.SESSION_RENEW || event.lifecycle === LifecycleKind.SESSION_EXPIRED), + (event.lifecycle === LifecycleKind.SESSION_RENEW || + event.lifecycle === LifecycleKind.SESSION_EXPIRED || + event.lifecycle === LifecycleKind.USER_CHANGED), handle: monitor(() => { this.pushIdentity(); }), @@ -139,7 +147,7 @@ export class BridgeHandler { * which carries structured-cloneable values only — a function would throw there. */ private buildConfig(): BridgeConfig { - return { ...this.bridgeOptions, sessionId: this.getSessionId() }; + return { ...this.bridgeOptions, sessionId: this.getSessionId(), user: this.getUser() }; } private trackBridgedRenderer(sender: WebContents | undefined): void { @@ -151,7 +159,7 @@ export class BridgeHandler { } private pushIdentity(): void { - const update: IdentityUpdate = { sessionId: this.getSessionId() }; + const update: IdentityUpdate = { sessionId: this.getSessionId(), user: this.getUser() }; for (const sender of this.bridgedRenderers) { if (sender.isDestroyed()) { diff --git a/src/common/bridge.types.ts b/src/common/bridge.types.ts index be76e37e..b0aad4cf 100644 --- a/src/common/bridge.types.ts +++ b/src/common/bridge.types.ts @@ -1,4 +1,5 @@ import type { DefaultPrivacyLevel } from '@flashcatcloud/browser-core'; +import type { User } from '../domain/UserContext'; /** * Payload the main process returns over the synchronous {@link CONFIG_CHANNEL}. @@ -14,10 +15,18 @@ export interface BridgeConfig { anonymousId: string; /** Id of the session active when the renderer asked, or `''` when no session is active. */ sessionId: string; + /** Identity set through `setUser` in the main process, or `undefined` when nobody is logged in. */ + user?: User; } /** Payload the main process pushes over {@link IDENTITY_CHANNEL} whenever an identifier changes. */ export interface IdentityUpdate { /** Id of the session now active, or `''` when the session expired without a replacement yet. */ sessionId: string; + /** + * Identity now in force, or `undefined` after `clearUser`. Absent and empty are distinct: the + * backend counts users off `NULLIF(usr_id, '')`, so a cleared identity has to remove the field + * rather than blank it. + */ + user?: User; } diff --git a/src/domain/UserContext.spec.ts b/src/domain/UserContext.spec.ts new file mode 100644 index 00000000..0d885701 --- /dev/null +++ b/src/domain/UserContext.spec.ts @@ -0,0 +1,350 @@ +import { mockFs } from '../mocks.specUtil'; + +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 } from 'vitest'; +import type { TimeStamp } from '@flashcatcloud/browser-core'; +import { UserContext, type User } from './UserContext'; +import { DiskValueHistory } from '../tools/DiskValueHistory'; +import { displayWarn } from '../tools/display'; +import { EventKind, EventManager, LifecycleKind, type Event, type LifecycleEvent } from '../event'; + +vi.mock('node:fs/promises'); +const mfs = mockFs(); + +const EXPIRE_DELAY = 10_000; +const ALICE: User = { id: 'alice', name: 'Alice', email: 'alice@example.com' }; + +describe('UserContext', () => { + let eventManager: EventManager; + let lifecycleEvents: LifecycleEvent[]; + + async function createContext(): Promise { + return UserContext.init(eventManager, EXPIRE_DELAY); + } + + /** Advances the clock so history entries get distinct timestamps. */ + function tick(ms = 10): void { + vi.setSystemTime(Date.now() + ms); + } + + function at(): TimeStamp { + return Date.now() as TimeStamp; + } + + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(1_000); + mfs.readFile.mockRejectedValue(new Error('ENOENT')); + mfs.writeFile.mockResolvedValue(undefined); + + eventManager = new EventManager(); + lifecycleEvents = []; + eventManager.registerHandler({ + canHandle: (event: Event): event is LifecycleEvent => event.kind === EventKind.LIFECYCLE, + handle: (event) => lifecycleEvents.push(event), + }); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); + mfs.reset(); + }); + + describe('set', () => { + it('should expose the identity as the current user', async () => { + const context = await createContext(); + + context.set(ALICE); + + expect(context.get()).toEqual(ALICE); + }); + + it('should return a copy, so a caller cannot rewrite the stored identity', async () => { + const context = await createContext(); + context.set(ALICE); + + context.get()!.id = 'mallory'; + + expect(context.get()!.id).toBe('alice'); + }); + + it('should not be affected by later mutation of the object it was given', async () => { + const context = await createContext(); + const user = { ...ALICE }; + + context.set(user); + user.id = 'mallory'; + + expect(context.get()!.id).toBe('alice'); + }); + + /** + * The guard against `setUser` reaching `usr.anonymous_id`. Only `id`/`name`/`email` are copied, + * so no extra property survives — see the note in `UserContext` about upstream's `extraInfo` + * leaving this key unprotected. + */ + it('should drop every property that is not id, name or email', async () => { + const context = await createContext(); + + context.set({ id: 'alice', anonymous_id: 'forged', plan: 'premium' } as unknown as User); + + expect(context.get()).toEqual({ id: 'alice' }); + }); + + it('should keep a partial identity when only an id is given', async () => { + const context = await createContext(); + + context.set({ id: 'alice' }); + + expect(context.get()).toEqual({ id: 'alice' }); + }); + }); + + describe('rejected calls', () => { + it.each([ + ['no id', {}], + ['an empty id', { id: '' }], + ['a non-string id', { id: 42 }], + ['a non-string name', { id: 'alice', name: 42 }], + ['a non-string email', { id: 'alice', email: {} }], + ['null', null], + ])('should ignore a call with %s, and warn', async (_label, input) => { + const context = await createContext(); + + context.set(input as unknown as User); + + expect(context.get()).toBeUndefined(); + expect(displayWarn).toHaveBeenCalled(); + }); + + it('should keep the previous identity rather than half-applying an invalid one', async () => { + const context = await createContext(); + context.set(ALICE); + + context.set({ id: 'bob', name: 42 } as unknown as User); + + expect(context.get()).toEqual(ALICE); + }); + + it('should not notify renderers about a call it rejected', async () => { + const context = await createContext(); + lifecycleEvents.length = 0; + + context.set({ id: '' } as User); + + expect(lifecycleEvents).toHaveLength(0); + }); + }); + + describe('clear', () => { + it('should forget the current identity', async () => { + const context = await createContext(); + context.set(ALICE); + + context.clear(); + + expect(context.get()).toBeUndefined(); + }); + + /** + * `get()` reads a plain field rather than querying the history, precisely so that a clear takes + * effect within the millisecond it happened in. Answering off `history.find(now)` would return + * the user again, because the entry closed at exactly `now` still spans `now`. + */ + it('should take effect immediately, even within the same millisecond', async () => { + const context = await createContext(); + context.set(ALICE); + + context.clear(); + + expect(context.get()).toBeUndefined(); + }); + + it('should be harmless when nobody is logged in', async () => { + const context = await createContext(); + + expect(() => context.clear()).not.toThrow(); + expect(context.get()).toBeUndefined(); + }); + }); + + describe('find — the identity in force at a past moment', () => { + it('should resolve to undefined before any identity was set', async () => { + const context = await createContext(); + const before = at(); + + tick(); + context.set(ALICE); + + expect(context.find(before)).toBeUndefined(); + }); + + it('should resolve an event to the identity in force when it happened', async () => { + const context = await createContext(); + context.set(ALICE); + const whileAlice = at(); + + tick(); + context.set({ id: 'bob' }); + + expect(context.find(whileAlice)).toEqual(ALICE); + }); + + it('should still resolve events from before a logout to the user who was logged in', async () => { + const context = await createContext(); + context.set(ALICE); + const whileAlice = at(); + + tick(); + context.clear(); + tick(); + + expect(context.find(whileAlice)).toEqual(ALICE); + expect(context.find(at())).toBeUndefined(); + }); + + /** + * Guard for the millisecond race `ViewCollection.createNewView` avoids the same way: `set` + * closes the previous entry and opens the next one from a **single** clock read. Read the clock + * twice and anything that happens in between widens into a hole where no identity is in force, + * so an event landing in it is attributed to nobody. + * + * The pause is simulated inside `closeActive` because that is where a real one occurs: it + * stringifies the whole history and schedules a disk write before returning. + */ + it('should leave no gap between two identities when closing the previous one is slow', async () => { + const context = await createContext(); + context.set(ALICE); + tick(); + + const switchMoment = at(); + // Captured unbound on purpose: the mock below re-invokes it with the instance it was called + // on, so the real close still happens after the simulated pause. + // eslint-disable-next-line @typescript-eslint/unbound-method + const closeActive = DiskValueHistory.prototype.closeActive; + const spy = vi.spyOn(DiskValueHistory.prototype, 'closeActive').mockImplementation(function ( + this: DiskValueHistory, + endTime + ) { + vi.setSystemTime(Date.now() + 5); + closeActive.call(this, endTime); + }); + + context.set({ id: 'bob' }); + spy.mockRestore(); + + // Every moment between the two identities has to resolve to one of them. + for (let offset = 0; offset <= 5; offset++) { + expect(context.find((switchMoment + offset) as TimeStamp)).toBeDefined(); + } + }); + + it('should return a copy, so an event cannot rewrite history', async () => { + const context = await createContext(); + context.set(ALICE); + const moment = at(); + + context.find(moment)!.id = 'mallory'; + + expect(context.find(moment)!.id).toBe('alice'); + }); + }); + + describe('persistence', () => { + it('should persist the history so a crash parsed on the next startup can be attributed', async () => { + const context = await createContext(); + + context.set(ALICE); + await vi.advanceTimersByTimeAsync(0); + + expect(mfs.writeFile).toHaveBeenCalledWith('/mock/user/data/_dd_user_history', expect.any(String), 'utf-8'); + }); + + it('should resolve an identity recorded by a previous run', async () => { + // Both timestamps predate this process: the previous run logged in, then crashed, and the + // dump is only parsed now. + const loginMoment = 500; + const crashMoment = 600 as TimeStamp; + mfs.readFile.mockResolvedValue( + JSON.stringify([{ startTime: loginMoment, endTime: null, value: { id: 'alice' } }]) + ); + + const context = await createContext(); + + expect(context.find(crashMoment)).toEqual({ id: 'alice' }); + }); + + /** + * A restored history says who *was* logged in, not who is now. Nothing in a fresh process has + * called `setUser` yet, so the bridge must not tell renderers there is a user. + */ + it('should not report a restored identity as the current one', async () => { + mfs.readFile.mockResolvedValue(JSON.stringify([{ startTime: at(), endTime: null, value: { id: 'alice' } }])); + + const context = await createContext(); + + expect(context.get()).toBeUndefined(); + }); + + /** + * Observed against dev before it was fixed: a second run stamped its first view with the *first* + * run's user. Quitting is not logging out, so the previous run's entry is still open on disk; + * restoring it as active attributes this process's events to whoever used the machine last. + */ + it('should not attribute new events to the identity a previous run left open', async () => { + const previousRun = 500; + mfs.readFile.mockResolvedValue( + JSON.stringify([{ startTime: previousRun, endTime: null, value: { id: 'alice' } }]) + ); + + const context = await createContext(); + + // No `tick()`: the first main-process view is created in the same millisecond as `init`, and + // that is the event whose identity the session inherits. `find` treats `endTime` as + // inclusive, so closing the restored entry at exactly `now` would still match here. + expect(context.find(at())).toBeUndefined(); + }); + + it('should still attribute a crash from the previous run to the user who was logged in then', async () => { + const previousRun = 500; + const crashMoment = 600; + mfs.readFile.mockResolvedValue( + JSON.stringify([{ startTime: previousRun, endTime: null, value: { id: 'alice' } }]) + ); + + const context = await createContext(); + + expect(context.find(crashMoment as TimeStamp)).toEqual({ id: 'alice' }); + }); + }); + + describe('change notifications', () => { + it('should notify on set, so the bridge can push the identity to renderers', async () => { + const context = await createContext(); + lifecycleEvents.length = 0; + + context.set(ALICE); + + expect(lifecycleEvents).toEqual([{ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.USER_CHANGED }]); + }); + + it('should notify on clear', async () => { + const context = await createContext(); + context.set(ALICE); + lifecycleEvents.length = 0; + + context.clear(); + + expect(lifecycleEvents).toEqual([{ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.USER_CHANGED }]); + }); + }); +}); diff --git a/src/domain/UserContext.ts b/src/domain/UserContext.ts new file mode 100644 index 00000000..734707e7 --- /dev/null +++ b/src/domain/UserContext.ts @@ -0,0 +1,191 @@ +import { app } from 'electron'; +import * as path from 'node:path'; +import { timeStampNow, type TimeStamp } from '@flashcatcloud/browser-core'; +import { DiskValueHistory } from '../tools/DiskValueHistory'; +import { SESSION_TIME_OUT_DELAY } from './session'; +import { displayWarn } from '../tools/display'; +import { EventKind, LifecycleKind, type EventManager } from '../event'; + +export const USER_HISTORY_FILE_NAME = '_dd_user_history'; + +/** + * The logged-in user's identity, as the application knows it. + * + * Deliberately only the three standard fields. `anonymous_id` is **not** part of this shape and + * cannot be reached through it: it is device-scoped, owned by `AnonymousId`, and has to survive a + * login, a logout, and a different user logging in on the same machine. + */ +export interface User { + id: string; + name?: string; + email?: string; +} + +/** + * The only keys copied out of a caller-supplied object. Everything else is dropped, which is what + * keeps `setUser({ anonymous_id: 'x' })` from reaching an event. + * + * Upstream's equivalent (`@datadog/electron-sdk` 0.7.0, `ContextManager.filterReservedKeys`) only + * excludes the standard fields from its free-form `extraInfo` bag, so `addUserExtraInfo({ + * anonymous_id })` there overwrites the device id. We do not ship `extraInfo` — if it is ever + * added, `anonymous_id` has to be reserved alongside `id`/`name`/`email`, or unique-user counting + * becomes corruptible from application code. + */ +const STANDARD_FIELDS = ['id', 'name', 'email'] as const; + +/** + * Holds the identity set through `setUser`, and answers two different questions with two different + * stores: + * + * - **"who is logged in right now"** (`get`) — what `getUser()` returns and what the bridge + * broadcasts to renderers. A plain field, so `clearUser()` takes effect immediately. + * - **"who was logged in when this event happened"** (`find`) — what event assembly asks. A + * time-indexed history, because a main-process event is not necessarily assembled in the moment + * it describes: a native crash is parsed on the *next* startup and carries the crash's original + * timestamp, and `addError` accepts a caller-supplied `startTime`. Attributing those to whoever + * happens to be logged in at assembly time would hand one user's crash to another. + * + * The history is disk-backed for the same reason `ViewContext`'s is: it has to outlive the process + * that recorded it, or the crash parsed on the next startup finds nothing. That does mean the + * identity is written to `userData` in plain text, like the anonymous id and the session file + * beside it. + */ +export class UserContext { + /** Who is logged in now, or `undefined` after `clear()` and before any `set()`. */ + private current: User | undefined; + + private constructor( + private readonly history: DiskValueHistory, + private readonly eventManager: EventManager + ) {} + + static async init(eventManager: EventManager, expireDelay = SESSION_TIME_OUT_DELAY): Promise { + const filePath = path.join(app.getPath('userData'), USER_HISTORY_FILE_NAME); + const history = await DiskValueHistory.init({ filePath, expireDelay }); + + // A run that ended without `clearUser` — the normal case, since quitting is not logging out — + // leaves its entry open, and restoring it still-active would attribute this process's events + // to the previous run's user until the application calls `setUser` again. If someone else is + // now using the machine, that is a leak rather than a rounding error. Closing it here draws + // the line at the process boundary: earlier timestamps still resolve to that user, which is + // what a crash from the previous run needs, and nothing new does. + // + // Closed one millisecond *before* now, because `find` treats `endTime` as inclusive and the + // first main-process view is created in the same millisecond as this call — observed against + // dev, where a second run stamped its opening view with the first run's user. The previous run + // ended strictly before this one began, so excluding the boundary is also the truthful bound. + history.closeActive((timeStampNow() - 1) as TimeStamp); + + return new UserContext(history, eventManager); + } + + set(user: User): void { + const sanitized = sanitizeUser(user); + if (!sanitized) { + // Rejected calls must not reach the renderers: pushing here would tell them the identity + // changed when it did not. + return; + } + + // One clock read for both ends. Reading it twice leaves a gap between the entry that closes + // and the one that opens, and an event landing inside that gap finds no identity at all — + // the millisecond race `ViewCollection.createNewView` avoids the same way. + const startTime = timeStampNow(); + this.history.closeActive(startTime); + this.history.add(sanitized, startTime); + this.current = sanitized; + this.notifyChange(); + } + + /** + * The history entry is closed rather than deleted, so an event describing a moment before the + * logout still resolves to the user who was logged in then. `find` treats both ends of an entry + * as inclusive, so an event stamped with the exact millisecond of the logout resolves to that + * user too — it describes a moment no later than the logout, which is the same answer. + */ + clear(): void { + this.history.closeActive(timeStampNow()); + this.current = undefined; + this.notifyChange(); + } + + /** + * The identity in force now. Returns a copy: the stored object is handed to every event that + * looks it up, so a caller mutating it would rewrite history. + */ + get(): User | undefined { + return this.current ? { ...this.current } : undefined; + } + + /** + * The identity in force at `startTime`, or `undefined` if nobody was logged in then. Events from + * before the first `setUser` must resolve to `undefined` rather than to the anonymous id — see + * `registerCommonContext` for why `usr.id` is never backfilled. + */ + find(startTime: TimeStamp): User | undefined { + const user = this.history.find(startTime); + return user ? { ...user } : undefined; + } + + private notifyChange(): void { + this.eventManager.notify({ kind: EventKind.LIFECYCLE, lifecycle: LifecycleKind.USER_CHANGED }); + } +} + +/** + * Which moment an event's identity is resolved at. + * + * **Point-in-time events** (error, resource, action, vital…) resolve at their own timestamp. That + * is what lets a native crash parsed on the next startup be attributed to whoever was logged in + * when it happened, rather than to whoever is logged in when it is finally read off disk. + * + * **View events resolve at the moment they are emitted**, because a view is an interval rather + * than an instant: it is re-reported as it grows (`_dd.document_version`), and the backend derives + * the session's identity from the **last** view row it receives + * (`buildSessionViewUpdates` in fc-rum reads `lastView.UserID`). The main process emits exactly one + * synthetic view per session, spanning the whole session, so resolving it at its start time would + * mean the identity never reaches the session at all — `setUser` cannot run before `init`, so the + * view always starts logged-out. The same applies to a renderer page view that spans a login. + */ +export function resolveEventUser( + getUser: (startTime: TimeStamp) => User | undefined, + eventType: string | undefined, + startTime: TimeStamp +): User | undefined { + return getUser(eventType === 'view' ? timeStampNow() : startTime); +} + +/** + * Copies the standard fields off a caller-supplied object, or rejects the call. + * + * Rejection is all-or-nothing on purpose. Accepting an `id` while dropping a malformed `name` + * would report an identity the application never asked for, and a half-applied identity is harder + * to notice than none at all. + */ +function sanitizeUser(user: User): User | undefined { + if (typeof user !== 'object' || user === null) { + displayWarn('setUser expects an object with a string `id`; the call was ignored.'); + return undefined; + } + + if (typeof user.id !== 'string' || user.id === '') { + displayWarn('The property "id" of the user is required and must be a non-empty string; the call was ignored.'); + return undefined; + } + + const sanitized = { id: user.id } as User; + + for (const field of STANDARD_FIELDS) { + const value = user[field]; + if (value === undefined || field === 'id') { + continue; + } + if (typeof value !== 'string') { + displayWarn(`The property "${field}" of the user must be a string; the call was ignored.`); + return undefined; + } + sanitized[field] = value; + } + + return sanitized; +} diff --git a/src/event/event.constants.ts b/src/event/event.constants.ts index bba18a92..9c0dc29e 100644 --- a/src/event/event.constants.ts +++ b/src/event/event.constants.ts @@ -26,4 +26,5 @@ export const LifecycleKind = { END_USER_ACTIVITY: 'end_user_activity', SESSION_EXPIRED: 'session_expired', SESSION_RENEW: 'session_renew', + USER_CHANGED: 'user_changed', } as const; diff --git a/src/event/event.types.ts b/src/event/event.types.ts index efbe0a1d..811d9f10 100644 --- a/src/event/event.types.ts +++ b/src/event/event.types.ts @@ -64,7 +64,13 @@ export interface SessionRenewEvent { lifecycle: typeof LifecycleKind.SESSION_RENEW; } -export type LifecycleEvent = EndUserActivityEvent | SessionExpiredEvent | SessionRenewEvent; +/** `setUser` or `clearUser` changed the identity, and renderers holding a cache have to be told. */ +export interface UserChangedEvent { + kind: typeof EventKind.LIFECYCLE; + lifecycle: typeof LifecycleKind.USER_CHANGED; +} + +export type LifecycleEvent = EndUserActivityEvent | SessionExpiredEvent | SessionRenewEvent | UserChangedEvent; export type Event = RawEvent | ServerEvent | LifecycleEvent; export interface EventHandler { diff --git a/src/index.ts b/src/index.ts index e4468ae7..f7718df7 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,9 +1,11 @@ +import type { TimeStamp } from '@flashcatcloud/browser-core'; import { Assembly, createFormatHooks, registerCommonContext } from './assembly'; import type { InitConfiguration } from './config'; import { buildConfiguration } from './config'; import { RumCollection } from './domain/rum'; import { SessionManager } from './domain/session'; import { initAnonymousId } from './domain/AnonymousId'; +import { UserContext, type User } from './domain/UserContext'; import { UserActivityTracker } from './domain/UserActivityTracker'; import { RendererRegistry } from './domain/RendererRegistry'; import { ViewTimingCorrector } from './domain/ViewTimingCorrector'; @@ -22,6 +24,7 @@ let eventManager: EventManager | undefined; let transport: Transport | undefined; let rumApi: ReturnType | undefined; let tracing: Tracing | undefined; +let userContext: UserContext | undefined; /** * Initialize the Electron SDK @@ -39,8 +42,14 @@ export async function init(configuration: InitConfiguration): Promise { const hooks = createFormatHooks(); const anonymousId = await initAnonymousId(); + const context = await UserContext.init(eventManager); + userContext = context; + // Looked up per event by start time rather than captured: a native crash is assembled on the + // next startup carrying the crash's own timestamp, so "who is logged in now" is the wrong + // question to ask of it. See `UserContext`. + const getUserAt = (startTime: TimeStamp) => context.find(startTime); - registerCommonContext(config, hooks, anonymousId); + registerCommonContext(config, hooks, anonymousId, getUserAt); startTelemetry(eventManager, config); const manager = await SessionManager.start(eventManager, hooks); sessionManager = manager; @@ -53,7 +62,7 @@ export async function init(configuration: InitConfiguration): Promise { new WindowVisibilityTracker(rendererRegistry); } - new Assembly(eventManager, hooks); + new Assembly(eventManager, hooks, getUserAt); // Only the fields the renderer bridge reads: they are returned over a synchronous IPC channel, // which can carry structured-cloneable data only — a callback would throw there. The session id // is read through a getter rather than captured, because it changes as the session is renewed. @@ -65,6 +74,7 @@ export async function init(configuration: InitConfiguration): Promise { anonymousId, }, () => getActiveSessionId(manager), + () => context.get(), rendererRegistry, new ViewTimingCorrector(rendererRegistry, config.correctPrewarmedViewTimings), stackPathNormalizer @@ -98,6 +108,51 @@ export function stopSession(): void { callMonitored(() => sessionManager?.expire()); } +/** + * Identify the logged-in user. The identity is attached to every subsequent main-process event and + * to the renderer events that reach the main process over the bridge. + * + * An `id` is required; a call without one is ignored with a warning, as is one whose `name` or + * `email` is not a string — a half-applied identity is harder to notice than none at all. Only + * `id`, `name` and `email` are read; any other property is dropped. + * + * This does **not** touch `usr.anonymous_id`. The two identifiers coexist by design: the anonymous + * id is device-scoped and stable across logins, and unique users are counted off it first. + * + * The name matches `flashcatRum.setUser()` in `@flashcatcloud/browser-rum`, so both processes of + * the same application use one vocabulary. + * + * @example + * setUser({ id: 'user-123', name: 'Alice', email: 'alice@example.com' }); + * // Later, when the user logs out: + * clearUser(); + */ +export function setUser(user: User): void { + callMonitored(() => userContext?.set(user)); +} + +/** + * The identity currently set through {@link setUser}, or `undefined` when nobody is logged in. + * Returns a copy — mutating it changes nothing. + */ +export function getUser(): User | undefined { + return callMonitored(() => userContext?.get()); +} + +/** + * Forget the identity set through {@link setUser}, for instance on logout. + * + * Subsequent events carry no `usr.id` **at all**, rather than an empty one: unique users are + * counted off `NULLIF(usr_id, '')`, where an absent field and an empty string are different rows. + * 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. + * + * `usr.anonymous_id` is unaffected — the device is still the same device. + */ +export function clearUser(): void { + callMonitored(() => userContext?.clear()); +} + /** * Report a manually handled error */ @@ -198,6 +253,7 @@ export function _generateTelemetryError() { } export type { InitConfiguration } from './config'; +export type { User } from './domain/UserContext'; export type { FailureReason, FeatureOperationOptions, diff --git a/src/preload/preloadScript.spec.ts b/src/preload/preloadScript.spec.ts index 55e74db6..d56eaaaa 100644 --- a/src/preload/preloadScript.spec.ts +++ b/src/preload/preloadScript.spec.ts @@ -18,6 +18,7 @@ interface EventBridge { getAllowedWebViewHosts: () => string; getSessionId: () => string; getAnonymousId: () => string; + getUser: () => string; send: (msg: string) => void; } @@ -166,4 +167,57 @@ describe('preload script', () => { expect(bridge.getSessionId()).toBe('session-2'); }); }); + + describe('user identity', () => { + it('should answer an empty object when nobody is logged in', async () => { + const bridge = await runPreload(); + + expect(bridge.getUser()).toBe('{}'); + }); + + it('should answer the identity carried by the configuration', async () => { + mockIpcRenderer.sendSync.mockReturnValue({ ...DEFAULT_CONFIG, user: { id: 'alice', name: 'Alice' } }); + + const bridge = await runPreload(); + + expect(JSON.parse(bridge.getUser())).toEqual({ id: 'alice', name: 'Alice' }); + }); + + it('should answer the identity the main process pushes', async () => { + const bridge = await runPreload(); + + pushIdentity({ sessionId: 'session-1', user: { id: 'alice' } }); + + expect(JSON.parse(bridge.getUser())).toEqual({ id: 'alice' }); + }); + + it('should go back to an empty object once the identity is cleared', async () => { + mockIpcRenderer.sendSync.mockReturnValue({ ...DEFAULT_CONFIG, user: { id: 'alice' } }); + const bridge = await runPreload(); + + pushIdentity({ sessionId: 'session-1' }); + + expect(bridge.getUser()).toBe('{}'); + }); + + it('should keep a push that lands before the configuration answers', async () => { + mockIpcRenderer.sendSync.mockImplementation(() => { + pushIdentity({ sessionId: 'session-1', user: { id: 'bob' } }); + return { ...DEFAULT_CONFIG, user: { id: 'alice' } }; + }); + + const bridge = await runPreload(); + + expect(JSON.parse(bridge.getUser())).toEqual({ id: 'bob' }); + }); + + it('should answer without a synchronous IPC call per read', async () => { + const bridge = await runPreload(); + mockIpcRenderer.sendSync.mockClear(); + + bridge.getUser(); + + expect(mockIpcRenderer.sendSync).not.toHaveBeenCalled(); + }); + }); }); diff --git a/src/preload/preloadScript.ts b/src/preload/preloadScript.ts index 7d2c5abc..1483751b 100644 --- a/src/preload/preloadScript.ts +++ b/src/preload/preloadScript.ts @@ -32,19 +32,23 @@ if (!window[BRIDGE_INITIALIZED]) { window[BRIDGE_INITIALIZED] = true; let sessionId = ''; - let sessionIdPushed = false; + let user: BridgeConfig['user']; + let identityPushed = false; - // Subscribe before asking for the configuration: the main process can renew the session at any - // moment, and a pushed value must never be overwritten by the older one the config carries. + // Subscribe before asking for the configuration: the main process can renew the session or + // change the user at any moment, and a pushed value must never be overwritten by the older one + // the config carries. Both travel in the same update, so one flag covers them. ipcRenderer.on(IDENTITY_CHANNEL, (_event, update: IdentityUpdate | undefined) => { - sessionIdPushed = true; + identityPushed = true; sessionId = update?.sessionId ?? ''; + user = update?.user; }); const config = ipcRenderer.sendSync(CONFIG_CHANNEL) as BridgeConfig | undefined; - if (!sessionIdPushed) { + if (!identityPushed) { sessionId = config?.sessionId ?? ''; + user = config?.user; } const defaultPrivacyLevel: string = config?.defaultPrivacyLevel ?? MASK; @@ -77,6 +81,18 @@ if (!window[BRIDGE_INITIALIZED]) { getAnonymousId() { return anonymousId; }, + /** + * Identity the main process set through `setUser`, as JSON, or `'{}'` when nobody is logged + * in. A JSON string rather than an object, to match `getCapabilities` and + * `getAllowedWebViewHosts` — the bridge only ever hands strings across. + * + * Kept up to date by {@link IDENTITY_CHANNEL} pushes, like the session id. Note that renderer + * events do not need this: the main process stamps the identity on them as they pass through. + * It is here so a renderer can attribute anything it uploads itself to the same user. + */ + getUser() { + return user ? JSON.stringify(user) : '{}'; + }, send(msg: string) { ipcRenderer.send(BRIDGE_CHANNEL, msg); }, From 45de2f5b2bcc33ccbeba84adf8c13a0d7a5b2652 Mon Sep 17 00:00:00 2001 From: Fiona Date: Fri, 7 Aug 2026 05:04:40 -0700 Subject: [PATCH 2/2] test: stop two suites needing a real Electron install MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `commonContext` and `Assembly` reach `UserContext` for the identity in force, and it imports `electron` at module load for the path its history file lives at. Nothing in either suite touches that history, but the import alone is enough: without an Electron binary the module throws on load and takes both suites with it. That is what CI has, and why these two were the only ones failing there while the other thirty-five passed — they were the only ones reaching `electron` without mocking it. Reproduced by removing `node_modules/electron/path.txt`, which is what the module checks: the same error, and gone with the mock. --- src/assembly/Assembly.spec.ts | 9 ++++++++- src/assembly/commonContext.spec.ts | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/assembly/Assembly.spec.ts b/src/assembly/Assembly.spec.ts index 2df27074..08cdc31f 100644 --- a/src/assembly/Assembly.spec.ts +++ b/src/assembly/Assembly.spec.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, it, expect } from 'vitest'; +import { beforeEach, describe, it, expect, vi } from 'vitest'; import { DISCARDED, SKIPPED, type TimeStamp } from '@flashcatcloud/browser-core'; import { Assembly } from './Assembly'; import { createFormatHooks, type FormatHooks } from './hooks'; @@ -16,6 +16,13 @@ import type { RumEvent, RawRumData } from '../domain/rum'; import type { User } from '../domain/UserContext'; import { createTestConfiguration } from '../mocks.specUtil'; +// `commonContext` and `Assembly` reach `UserContext` for the identity in force, and it imports +// `electron` at module load for the path its history file lives at. Nothing here touches that +// history, but the import alone needs an Electron install these tests do not have. +vi.mock('electron', () => ({ + app: { getPath: vi.fn(() => '/mock/user-data') }, +})); + const RAW_ERROR_DATA: RawRumData = { type: 'error', error: { id: '1', message: 'test', source: 'custom', handling: 'handled' }, diff --git a/src/assembly/commonContext.spec.ts b/src/assembly/commonContext.spec.ts index 05f30cce..7db0c4cd 100644 --- a/src/assembly/commonContext.spec.ts +++ b/src/assembly/commonContext.spec.ts @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, it } from 'vitest'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; import { Assembly } from './Assembly'; import { createFormatHooks, type FormatHooks } from './hooks'; import { registerCommonContext } from './commonContext'; @@ -15,6 +15,13 @@ import type { RumEvent, RawRumData } from '../domain/rum'; import type { User } from '../domain/UserContext'; import { createTestConfiguration } from '../mocks.specUtil'; +// `commonContext` and `Assembly` reach `UserContext` for the identity in force, and it imports +// `electron` at module load for the path its history file lives at. Nothing here touches that +// history, but the import alone needs an Electron install these tests do not have. +vi.mock('electron', () => ({ + app: { getPath: vi.fn(() => '/mock/user-data') }, +})); + const ANONYMOUS_ID = 'device-anonymous-id'; const ALICE: User = { id: 'alice', name: 'Alice', email: 'alice@example.com' };