Skip to content

T8: logException domain and skipFrames #44

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
krassx wants to merge 2 commits into from
Closed

T8: logException domain and skipFrames #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e4763b2
feat(core): logException domain and skipFrames options
krassx Oct 12, 2026
752bd01
ci: retrigger checks and automated review
krassx Oct 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion docs/design/mobile-api-alignment.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Each task is test-first with the mutator loop, keeps the coverage gate, and land
| T5 | Launch state: `getStatus()` with the Stopped/Launching/Launched/Stopping enum, `relaunch(options?)`, `getLaunchOptions()`, `getHostLaunchOptions()`; verify launch → stop → launch works | `getStatus`, `relaunch`, `getLaunchOptions` | |
| T6 | Missing options: `captureLogsLevel`, `captureBreadcrumbs`, `detectHttpErrors`, `captureNetworkDefaultSanitizer`, `reportDefaultCrashPriority`, `reportDefaultErrorPriority`; `getDefaultNetworkSanitizer()` so a custom filter can reuse the default redaction | `OptionsDescriptors` keys under `com.bugsee.option.*` | canonical keys already exist for some |
| T7 | `notify(title, body?, severity?, fields?, urgent?)` | `Bugsee.notify` | needs a backend check first: which endpoint carries it, and does the collector accept it from a browser |
| T8 | `logException` options: `domain`; check what `mergingRules` does on iOS | Android `ExceptionOptions.Domain`, iOS `BugseeExceptionLoggingOptions` | |
| T8 | `logException` options: `domain`; check what `mergingRules` does on iOS | Android `ExceptionOptions.Domain`, iOS `BugseeExceptionLoggingOptions` | Done: `domain` + `skipFrames` (see T8 findings) |
| T9 | Browser: persist the user identifier across page loads, as iOS does | iOS `setUserIdentifier` | storage-failure tolerant |

## Options added by T6
Expand All @@ -43,6 +43,31 @@ Invalid values warn through `onError` and fall back to the default.
`{ sanitize, sanitizeUrl, sanitizeErrorMessage }`, the same object the built-in sanitizer uses. Call it from inside a
`setNetworkEventFilter` filter to keep the default redaction.

## T8 findings

Investigated 2026-10 against Android `library/.../reporting/exceptions/`, iOS `BugseeExceptionLoggingOptions.h`,
`BGSCrashReport.m`, and worker `crash/{android,apple,javascript}.py`, `crash/managed/common.py`.

- **`domain`** (Android `ExceptionOptions.Domain = "domain"`, iOS `exceptionDomain`): a short issue-organising
category. Android writes it as `exception.domain` in crash.json (`ExceptionSerializer`); iOS as the error `domain`.
The worker folds it into the grouping signature (`android.py:175-179`, `apple.py:370`: "reports with the same
domain and stack trace group together") and prefixes the issue summary with `[domain]` (`android.py:251-268`, capped
at 150 chars). **Gap:** JavaScript crashes go through `crash/managed/common.py:get_crash_signatures`, which does not
read `domain`, and `javascript.py:530` builds its own summary. The SDK now sends `exception.domain` (the Android wire
shape); it has no grouping or summary effect until the worker's managed path reads it. Not verified against a
running backend.
- **`skipFrames`** (Android `ExceptionOptions.SkipFrames`, iOS `mergingRules[BugseeMergingRulesSkipFramesKey]`):
skips N top frames when computing the crash location/signature; frames stay in the report. Android applies it
client-side in `ExceptionSignature`; the worker also reads `exceptionLoggingOptions.mergingRules.skipFrames`
(`android.py:111,248`, `apple.py:301,439`), but not on the managed/JS path. JS equivalent: the top N frames of the
outermost exception get `hidden: true`, which `managed/common.py:_get_crash_location` already skips. Same
semantics, no worker change, trace still shows them. Implemented.
- **`mergingRules`**: a dictionary whose only key is `skipFrames`. Not added as its own option; `skipFrames` is the
flat equivalent (Android's map already uses the flat key).
- **`includeVideo`**: forces a video-less report. Not applicable: JS has no video; replay is governed by the
`replay` launch option. Kept difference.
- Invalid values (empty/non-string `domain`, non-integer or negative `skipFrames`) are ignored and routed to `onError`.

## Kept differences (documented, not changed)

- `captureBreadcrumbs` defaults to `true`, not mobile's `false`: UI-interaction breadcrumbs (click / change / submit /
Expand Down
70 changes: 70 additions & 0 deletions packages/core/src/client.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,76 @@ describe('createClient — crash.json (SC3)', () => {
});
});

describe('logException domain and skipFrames', () => {
const stacked = () => {
const err = new Error('boom');
err.stack =
'Error: boom\n at wrapper (a.js:1:1)\n at inner (b.js:2:2)\n at app (c.js:3:3)';
return err;
};
const run = async (opts: Record<string, unknown>) => {
const { uploadPipeline, enqueue } = fakeUpload();
const onError = vi.fn();
const client = createClient({ uploadPipeline, appToken: 'tok', getEnvironment, onError });
await client.logException(stacked(), opts);
return { crash: crashOf(enqueue.mock.calls[0]?.[0] as Bundle), onError };
};

it('writes exception.domain, the field the Android worker folds into the signature', async () => {
const { crash, onError } = await run({ domain: 'checkout ' });
expect(crash.exception.domain).toBe('checkout '); // verbatim, not trimmed
expect(onError).not.toHaveBeenCalled();
});

it('omits domain when not given', async () => {
const { crash } = await run({});
expect('domain' in crash.exception).toBe(false);
expect(crash.exception.frames.some((f: { hidden?: boolean }) => f.hidden)).toBe(false);
});

it('ignores a non-string or empty domain and warns through onError', async () => {
for (const bad of [42, '', {}]) {
const { crash, onError } = await run({ domain: bad });
expect('domain' in crash.exception).toBe(false);
expect(onError).toHaveBeenCalledTimes(1);
expect(String(onError.mock.calls[0]?.[0])).toContain('domain');
}
});

it('marks the top skipFrames frames hidden, so grouping skips them but they stay visible', async () => {
const { crash } = await run({ skipFrames: 2 });
const f = crash.exception.frames as { trace: string; hidden?: boolean }[];
expect(f).toHaveLength(3);
expect(f.map((x) => x.hidden)).toEqual([true, true, undefined]);
});

it('skipFrames beyond the frame count hides all of them; 0 hides none', async () => {
const all = await run({ skipFrames: 99 });
expect(all.crash.exception.frames.every((x: { hidden?: boolean }) => x.hidden)).toBe(true);
const none = await run({ skipFrames: 0 });
expect(none.crash.exception.frames.some((x: { hidden?: boolean }) => x.hidden)).toBe(false);
expect(none.onError).not.toHaveBeenCalled();
});

it('ignores an invalid skipFrames (negative, fractional, non-number) and warns', async () => {
for (const bad of [-1, 1.5, '2', Number.NaN]) {
const { crash, onError } = await run({ skipFrames: bad });
expect(crash.exception.frames.some((x: { hidden?: boolean }) => x.hidden)).toBe(false);
expect(onError).toHaveBeenCalledTimes(1);
expect(String(onError.mock.calls[0]?.[0])).toContain('skipFrames');
}
});

it('applies to a non-Error throwable too', async () => {
const { uploadPipeline, enqueue } = fakeUpload();
const client = createClient({ uploadPipeline, appToken: 'tok', getEnvironment });
await client.logException('str', { domain: 'd', skipFrames: 1 });
const crash = crashOf(enqueue.mock.calls[0]?.[0] as Bundle);
expect(crash.exception.domain).toBe('d');
expect(crash.exception.frames[0].hidden).toBe(true);
});
});

it('still writes a crash.json for a non-Error logException, with a synthetic exception', async () => {
// A bundle with no crash.json produced an issue the backend could only answer with "Crash data
// for the issue was not found" — present, counted, unusable. Non-Error throwables are ordinary
Expand Down
39 changes: 38 additions & 1 deletion packages/core/src/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import {
type Client,
type OptionsContainer,
} from './contracts';
import { buildCrashJson, type FrameEnricher } from './crash';
import { applyExceptionOptions, buildCrashJson, type FrameEnricher } from './crash';
import { checkOrSetAlreadyCaught } from './dedup';
import { createDetectionCoordinator } from './detection-coordinator';
import type { IdentifiedBundle } from './durable-upload-pipeline';
Expand Down Expand Up @@ -163,6 +163,19 @@ export interface LogExceptionOptions {
severity?: SeverityName;
/** Extra labels for the issue. */
labels?: string[];
/**
* Exception domain (Android `ExceptionOptions.Domain`, iOS `exceptionDomain`): a short category such as
* `checkout` that organises issues. Sent as `crash.json` `exception.domain`. A non-string or empty
* value is ignored and reported through `onError`.
*/
domain?: string;
/**
* Number of top stack frames to leave out of issue grouping (Android `ExceptionOptions.SkipFrames`, iOS
* `mergingRules[skipFrames]`). Use it when a wrapper or adapter logs on the app's behalf, so every error
* it reports does not group at the wrapper. The frames stay visible in the trace. Must be a non-negative
* integer; anything else is ignored and reported through `onError`.
*/
skipFrames?: number;
Comment on lines +172 to +178

@cursor cursor Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Medium — JSDoc describes the logException caller; the code skips the exception stack

Problem: This comment says to use skipFrames when a wrapper/adapter logs on the app's behalf, so errors do not group at the wrapper. applyExceptionOptions hides crash.exception.frames[0..N) — the thrown value's frames, V8-order (throw / new Error() site first). A wrapper that does logException(caught) does not appear on caught.stack. This repo's own adapters (express, fastify, reportError, …) correctly pass the original error with no skipFrames.

Impact: Following this JSDoc on the common catch path hides the fault site. Remaining frames are often a shared caller (Express Layer.handle, React render). Unrelated bugs collapse into one issue — the opposite of the stated goal.

Scenario:

app.use((err, _req, _res, next) => {
  void bugsee.logException(err, { skipFrames: 1 }); // “skip this middleware”
  next(err);
});

err.stack is the route that threw. Frame 0 (the actual bug) is marked hidden. Grouping moves to the next frame, frequently a framework frame shared by every request.

skipFrames does skip the wrapper when the wrapper constructs the error (logException(new Error('x'), { skipFrames: 1 })) or logs a non-Error (synthetic frames from callSiteFrames). That is the Android ExceptionSignature meaning; it is not “skip the function that called logException.”

Fix: Document exception-stack semantics, e.g. “Hides the top N frames of the thrown value when picking the grouping location. Use when this call constructs the Error (or logs a non-Error) so grouping does not sit on this helper. Logging a caught error does not skip this call site — it is not on that stack.”

}

/**
Expand Down Expand Up @@ -1121,6 +1134,30 @@ export function createClient(options: CreateClientOptions = {}): InternalBugseeC
...(options.enrichFrames !== undefined ? { enrichFrames: options.enrichFrames } : {}),
...(syntheticFrames !== undefined ? { syntheticFrames } : {}),
});
const crashOptions: { domain?: string; skipFrames?: number } = {};
const domain = exceptionOptions?.domain;
if (domain !== undefined) {
if (typeof domain === 'string' && domain !== '') {
crashOptions.domain = domain;
} else {
onError(
new Error('logException: ignoring invalid `domain` (expected a non-empty string)'),
);
}
}
const skipFrames = exceptionOptions?.skipFrames;
if (skipFrames !== undefined) {
if (Number.isInteger(skipFrames) && skipFrames >= 0) {
crashOptions.skipFrames = skipFrames;
} else {
onError(
new Error(
'logException: ignoring invalid `skipFrames` (expected a non-negative integer)',
),
);
}
}
applyExceptionOptions(crash, crashOptions);
const request = createReportingRequest({
source: { type: 'error', mechanism: exceptionOptions?.mechanism ?? 'programmatic' },
summary: message,
Expand Down
28 changes: 28 additions & 0 deletions packages/core/src/crash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ export interface CrashFrame {
user: boolean;
data?: { source?: string; member?: string; line?: number; column?: number };
debug_id?: string;
/**
* Excluded from the grouping location (the worker skips `hidden` frames when it picks the crash
* location) while still shown in the trace. Set by `logException({ skipFrames })`.
*/
hidden?: boolean;
/**
* Local variables in scope at this frame — `{ name: stringified-value }`, already scrubbed and capped
* by the capturing tier. Absent unless the platform captured them.
Expand Down Expand Up @@ -51,6 +56,8 @@ export interface CrashProvenance {
export interface CrashException {
name: string;
reason?: string;
/** Application-supplied exception domain (`logException({ domain })`; Android `ExceptionOptions.Domain`). */
domain?: string;
frames: CrashFrame[];
cause?: CrashException;
}
Expand Down Expand Up @@ -293,6 +300,27 @@ function renderThrowable(value: unknown): string {
return String(value);
}

/**
* Apply the already-validated `logException` options that live in the crash document: stamp `domain` on
* the outermost exception and mark its top `skipFrames` frames `hidden` (Android/iOS `skipFrames`: the
* frames stay in the trace but stop deciding the grouping location, so a wrapper or adapter does not
* collapse every error it reports into one issue). Mutates and returns `crash`.
*/
export function applyExceptionOptions(
crash: CrashJson,
options: { domain?: string; skipFrames?: number },
): CrashJson {
if (options.domain !== undefined) {
crash.exception.domain = options.domain;
}
if (options.skipFrames !== undefined) {
for (const frame of crash.exception.frames.slice(0, options.skipFrames)) {
frame.hidden = true;
}
}
Comment on lines +316 to +320

@cursor cursor Bot Oct 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 High — skipFrames can hide every grouping-eligible frame

Problem: This loop hides frames[0 .. skipFrames) with no floor. skipFrames >= frames.length (and the skipFrames: 99 test) marks every frame hidden: true. _get_crash_location is documented to skip hidden frames; stack.ts / crash.ts already record that the worker emits no grouping signature without a top frame (managed/common.py:88). This is the same hole as empty frames: [] (SBROWSER-32/35/38/41: one new issue per event).

It is not only the 99 case. isUserFrame already marks node: / @bugsee/ as user: false because those frames were feeding grouping. After a documented wrapper skip, the leftover is often exactly those frames.

Impact: Wrapper authors who pass skipFrames lose grouping. Unrelated events become separate issues (no location) or collapse onto a shared node:internal frame.

Scenario:

function report(err: unknown) {
  void client.logException(err, { skipFrames: 1 }); // JSDoc: skip the wrapper
}
await report('payment failed'); // non-Error → callSiteFrames already stripped logException

Synthetic / async stacks are commonly two frames: report (user) + processTicksAndRejections (node:internal, user: false). skipFrames: 1 hides report. Nothing grouping-eligible remains. The existing test logException('str', { skipFrames: 1 }) only asserts frames[0].hidden and never that a user frame is still unhidden. A 1-frame new Error() constructed inside the wrapper fails the same way.

Fix: Clamp so at least one user !== false frame stays unhidden:

const frames = crash.exception.frames;
const lastUser = frames.findLastIndex((f) => f.user !== false);
const n = lastUser < 0 ? 0 : Math.min(options.skipFrames, lastUser);
for (const frame of frames.slice(0, n)) frame.hidden = true;

Replace the skipFrames: 99 assertion that currently requires every frame hidden. Keep skipFrames: 0 as a no-op.

return crash;
}

export function buildCrashJson(error: unknown, options: BuildCrashOptions = {}): CrashJson {
if (!(error instanceof Error)) {
return {
Expand Down
Loading