Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 9 additions & 4 deletions packages/runtime-browser/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
release?: string;
/** Send a session_start ping on init (default true). */
sessionTracking?: boolean;
/** Last-chance hook: mutate or drop (return null) an event before send. */
/** Last-chance hook: mutate or drop (return null) an event before send. Throwing drops the event. */
beforeSend?: (event: BrowserEvent) => BrowserEvent | null;
/** Observe failed fetch and XHR requests and 5xx responses (default true). */
captureNetworkFailures?: boolean;
Expand Down Expand Up @@ -311,9 +311,14 @@
// Scrub before beforeSend so the last-chance hook sees the final form.
if (event.context) event.context = redactContext(event.context);
if (opts.beforeSend) {
const mapped = opts.beforeSend(event);
if (!mapped) return;
event = mapped;
try {
const mapped = opts.beforeSend(event);
if (!mapped) return;
event = mapped;

Check notice on line 317 in packages/runtime-browser/src/index.ts

View check run for this annotation

Autter.dev / autter/review-gate

💡 Suggestion · Hook-produced non-serializable data can throw from telemetry capture

A beforeSend hook that returns an event with context containing a bigint completes successfully, so the new catch does not handle it. When the queue reaches MAX_QUEUE, enqueue calls flush synchronously, where JSON.stringify throws outside an error boundary. The batch has already been removed from the queue and counted as sent. Guard serialization or validate hook output before queuing so malformed hook output cannot throw into the host application. Suggested fix: Handle the boundary input case at packages/runtime-browser/src/index.ts:317. Initialize with sessionTracking:false and beforeSend(event) { event.context = { value: 1n }; return event; }, then call captureMessage("test") ten times. currently The tenth captureMessage triggers flush, whose JSON.stringify throws a TypeError on the bigint outside the new try/catch. The capture API throws into the host application, and all ten events have already been removed from the queue and counted as sent. Bigint is allowed by the declared Record<string, unknown> context type. — add the guard / idempotency / transaction / bound that makes this case safe, then confirm the assumption "An event returned by beforeSend can be queued without allowing hook-produced data to throw into the host application." holds. Blast radius — this counterexample can fail the downstream usage that depends on this file: functions `AutterBrowserOptions`, `initAutterBrowser`, `stripQuery`, `getSessionId`, `flush`, `enqueue`; scopes `@autter/runtime-browser`.

Check notice on line 317 in packages/runtime-browser/src/index.ts

View check run for this annotation

Autter.dev / autter/review-gate

💡 Suggestion · Biome: lint/style/noParameterAssign

Reassigning a function parameter is confusing. Suggested fix: Fix the Biome `lint/style/noParameterAssign` issue at packages/runtime-browser/src/index.ts:317: Reassigning a function parameter is confusing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟠 [deterministic] Biome: lint/style/noParameterAssign — Risk: 55/100

Reassigning a function parameter is confusing.

🛠 AI fix prompt (copy & paste into your coding agent)
Fix the Biome `lint/style/noParameterAssign` issue at packages/runtime-browser/src/index.ts:317: Reassigning a function parameter is confusing.

Flagged by Autter security & observability checks.

} catch {
// A telemetry hook must not change application request outcomes.
return;
}
}
queue.push(event);
if (queue.length >= MAX_QUEUE) {
Expand Down
59 changes: 59 additions & 0 deletions packages/runtime-browser/test/before-send.test.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { test } from "node:test";
import assert from "node:assert/strict";
import { captureMessage, flush, initAutterBrowser } from "../dist/index.js";

test("beforeSend failures preserve application requests and later delivery", async () => {
const response = new Response("unavailable", { status: 503 });
const networkError = new Error("original network failure");
const hookError = new Error("beforeSend failure");
const sent = [];
let rejectRequest = false;
let hookMode = "throw";
globalThis.window = {
fetch: async () => {
if (rejectRequest) throw networkError;
return response;
},
addEventListener() {},
};
globalThis.document = { addEventListener() {}, visibilityState: "visible" };
globalThis.location = { href: "https://app.example.test/", pathname: "/" };
Object.defineProperty(globalThis, "navigator", {
configurable: true,
value: {
sendBeacon(_url, body) { sent.push(body); return true; },
},
});
initAutterBrowser({
endpoint: "/api/autter-runtime",
service: "web",
sessionTracking: false,
captureActions: false,
captureTimings: false,
beforeSend(event) {
if (hookMode === "throw") throw hookError;
if (hookMode === "drop") return null;
return { ...event, message: "mapped message" };
},
});

assert.equal(await window.fetch("/checkout"), response);
rejectRequest = true;
await assert.rejects(window.fetch("/offline"), (error) => error === networkError);
assert.doesNotThrow(() => captureMessage("failed hook"));
flush();
assert.equal(sent.length, 0);

hookMode = "drop";
captureMessage("dropped message");
flush();
assert.equal(sent.length, 0);

hookMode = "map";
captureMessage("healthy message");
flush();
assert.equal(sent.length, 1);
const payload = JSON.parse(await sent[0].text());
assert.equal(payload.events.length, 1);
assert.equal(payload.events[0].message, "mapped message");
});
Loading