Skip to content

Commit 03efc8f

Browse files
committed
feat(e2e): run the browser tier in a real Chromium and the OTel bridge end to end
Two harnesses that did not exist, and three defects only they could find. browser.e2e.ts boots @bugsee/browser inside a real headless Chromium via Playwright, serves the page from its OWN origin so the upload is genuinely cross-origin, and asserts the bundle that arrives: console, a real fetch, a real window.onerror crash with a structured crash.json, a real rrweb replay, and real IndexedDB persistence. Every browser-tier suite until now ran under jsdom — replay.e2e.ts names the gap in its own header. opentelemetry.e2e.ts runs a real @opentelemetry/sdk-trace-base provider in a real node process with Bugsee's SpanProcessor registered on it, and asserts both directions on the wire: the consumed trace as a Bugsee transaction (root + parented child) and the same trace teed to an OTLP endpoint. What they found: - The mock collector sent no CORS headers and answered no preflight, so no harness could ever drive the SDK from a real browser. The real collector must send them; the browser SDK is always cross-origin with the page. - assertNoSecrets scanned raw bytes and was therefore BLIND to replay.bin, which is gzipped. Measured, not theorised: with masking off, a typed password sat in plaintext inside replay.bin and the shared secret scan passed it. It now inflates gzip entries. Its one binary test used an uncompressed replay.bin — a property the shipped file never has. - The performance uploader delivered ONLY on its 30s interval. client.flush() drained reports but not transactions, and stop() dropped them, so any process ending between ticks — a serverless invocation, a CLI, a test, a closing tab — uploaded no APM data at all. The umbrella now composes the extension drain into both. The mock collector also had no /v2/performance/transactions route, so every APM batch in every previous e2e 404'd into onError unseen.
1 parent 0247ba9 commit 03efc8f

15 files changed

Lines changed: 1058 additions & 16 deletions

‎packages/bugsee/src/launch.test.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,38 @@ describe('bugsee umbrella launch', () => {
282282
expect(JSON.parse(perfPosts[0]?.body ?? '{}').transactions).toHaveLength(1);
283283
});
284284

285+
// A 30s tick is the ONLY delivery path the uploader had, so anything buffered since the last one was
286+
// lost whenever the process ended first — which for a serverless invocation, a CLI, or a test is every
287+
// time. `flush()` is documented as "get everything out"; it drained reports but not transactions.
288+
it('flush() delivers buffered transactions instead of waiting for the interval', async () => {
289+
const { scheduler } = fakeScheduler();
290+
const { fn: transport, perfPosts } = recordingTransport();
291+
const client = track(
292+
launch('tok', base({ carrier: {}, scheduler, transport, performanceFlushIntervalMs: 7777 })),
293+
);
294+
(client.ext('performance').getActiveSpan() as Transaction).finish(); // buffered, no tick fired
295+
expect(perfPosts).toHaveLength(0);
296+
297+
await client.flush();
298+
299+
expect(perfPosts).toHaveLength(1);
300+
expect(JSON.parse(perfPosts[0]?.body ?? '{}').transactions).toHaveLength(1);
301+
});
302+
303+
it('stop() delivers what was buffered rather than dropping it on teardown', async () => {
304+
const { scheduler } = fakeScheduler();
305+
const { fn: transport, perfPosts } = recordingTransport();
306+
const client = launch(
307+
'tok',
308+
base({ carrier: {}, scheduler, transport, performanceFlushIntervalMs: 7777 }),
309+
);
310+
(client.ext('performance').getActiveSpan() as Transaction).finish();
311+
312+
await client.stop();
313+
314+
expect(perfPosts).toHaveLength(1);
315+
});
316+
285317
it('starts the uploader at performanceFlushIntervalMs', () => {
286318
const { scheduler, intervals } = fakeScheduler();
287319
track(launch('tok', base({ carrier: {}, scheduler, performanceFlushIntervalMs: 7777 })));

‎packages/bugsee/src/wire.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,10 +279,25 @@ export function wireUmbrella(
279279
// already registered as the process singleton, so mutating its stop() here means the carrier singleton,
280280
// a repeat launch, and this return value are all one consistent client whose stop() tears the extensions
281281
// (performance, OTel propagation, consume) down before the core teardown.
282+
//
283+
// `flush()` is composed the same way and for the same reason: the performance uploader delivers on an
284+
// interval, so without this everything buffered since the last tick is lost whenever the process ends
285+
// first — the normal case for a serverless invocation, a CLI run, or a closing tab. The extension's
286+
// drain runs BEFORE the core drain so a transaction batch and the reports leave in one flush.
282287
const stopClient = client.stop;
283-
client.stop = (timeout?: number): Promise<boolean> => {
288+
const flushClient = client.flush;
289+
// No guard around it: `uploader.flush()` already catches a failed batch into `onError` and drops it, so
290+
// a broken APM upload cannot turn a caller's flush()/stop() into a rejection and cost them the crash
291+
// report they were flushing. Wrapping it again here would be untestable through this seam.
292+
const flushExtensions = (): Promise<void> => wired.flush();
293+
client.flush = async (timeout?: number): Promise<boolean> => {
294+
await flushExtensions();
295+
return flushClient(timeout);
296+
};
297+
client.stop = async (timeout?: number): Promise<boolean> => {
284298
void spanProcessor?.shutdown();
285299
offPropagation?.();
300+
await flushExtensions();
286301
wired.stop();
287302
return stopClient(timeout);
288303
};

‎packages/e2e-kit/src/bundle.ts‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// unit tests (bundle.test.ts), run in CI by the `test:unit` task — an assertion library that cannot fail is
1818
// precisely the theater this work removes, so every assertion has a negative test.
1919
import type { ManifestJson } from '@bugsee/protocol';
20-
import { strFromU8, unzipSync } from '@bugsee/util';
20+
import { gunzipSync, strFromU8, unzipSync } from '@bugsee/util';
2121

2222
/** The subset of the mock collector this library needs — keeps it usable from every harness. */
2323
export interface UploadsSource {
@@ -147,6 +147,28 @@ export function assertNoContractViolations(source: ViolationSource): void {
147147
*
148148
* An empty secret is rejected — it would match every file and turn this into a no-op that always passes.
149149
*/
150+
/**
151+
* The searchable bytes of a bundle entry: gzip-compressed ones are INFLATED first.
152+
*
153+
* Without this the scan is blind to exactly the file most likely to carry typed characters. `replay.bin`
154+
* is written gzipped, so a secret recorded verbatim inside it is not a substring of the stored bytes and
155+
* a raw scan sweeps clean over a real leak — measured, not theorised: a real-Chromium run with masking
156+
* disabled put a typed password in plaintext inside `replay.bin` and this function passed it.
157+
*
158+
* Detection is by gzip magic (`1f 8b`) rather than by filename, so it covers any compressed entry.
159+
*
160+
* A truncated or corrupt stream throws, deliberately uncaught here: the caller already wraps this in the
161+
* lenient decode whose fallback searches the RAW bytes, which is exactly the right answer for a file
162+
* whose claimed compression is a lie. Catching it here as well was dead code — a mutation that removed
163+
* the inner handler changed no test outcome, which is what exposed it.
164+
*/
165+
function decompressed(bytes: Uint8Array): Uint8Array {
166+
if (bytes.length < 2 || bytes[0] !== 0x1f || bytes[1] !== 0x8b) {
167+
return bytes;
168+
}
169+
return gunzipSync(bytes);
170+
}
171+
150172
export function assertNoSecrets(bundle: ParsedBundle, secrets: readonly string[]): void {
151173
// The apptoken exemption keys on PROVENANCE, not the name: only the assembler's structural apptoken (a
152174
// root entry the manifest does not declare) is skipped. An attachment that merely happens to be called
@@ -167,7 +189,7 @@ export function assertNoSecrets(bundle: ParsedBundle, secrets: readonly string[]
167189
// Decode leniently: binary payloads still surface ASCII substrings, which is what a leak looks like.
168190
let text: string;
169191
try {
170-
text = strFromU8(bytes);
192+
text = strFromU8(decompressed(bytes));
171193
} catch {
172194
text = Buffer.from(bytes).toString('latin1');
173195
}

‎packages/e2e-kit/src/collector.ts‎

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,16 @@ export interface MockCollector {
5353
echoHits: number;
5454
/** The request headers of each /echo hit (so a test can assert injected traceparent/tracestate). */
5555
echoHeaders: Array<Record<string, string | string[] | undefined>>;
56+
/**
57+
* Performance transactions posted to `/v2/performance/transactions`, flattened across batches.
58+
*
59+
* The mock had no such route, so every batch 404'd into `onError` and no harness could see APM data at
60+
* all — the SDK's transactions were the one payload nothing verified. The OTel consume bridge feeds
61+
* this same endpoint, so it is also how a consumed OTel span becomes visible.
62+
*/
63+
transactions: Array<Record<string, unknown>>;
64+
/** OTLP/HTTP-JSON export requests posted to `/v1/traces` (the produce-direction tee). */
65+
otlpTraces: Array<Record<string, unknown>>;
5666
/**
5767
* Every envelope that failed schema validation, in arrival order. The collector RECORDS rather than
5868
* rejects, so a contract break shows up as a readable test failure instead of an opaque upload error
@@ -77,9 +87,26 @@ const readBody = (req: IncomingMessage): Promise<Uint8Array> =>
7787
// `app.utils.js` success()/error().
7888
const envelope = (result: unknown): unknown => ({ ok: true, result });
7989

90+
/**
91+
* The CORS headers the REAL collector must send, because the browser SDK is always cross-origin with the
92+
* page that loaded it — a customer's app runs on their domain and uploads to Bugsee's.
93+
*
94+
* The mock did not send them, which meant no harness could ever drive the SDK from a real browser: the
95+
* very first upload fails at the preflight with an opaque "Failed to fetch". Every browser-tier suite ran
96+
* under jsdom, where fetch enforces no same-origin policy, so the gap was invisible until a real Chromium
97+
* was pointed at this server.
98+
*/
99+
const corsHeaders = (): Record<string, string> => ({
100+
'access-control-allow-origin': '*',
101+
'access-control-allow-methods': 'GET, POST, PUT, OPTIONS',
102+
'access-control-allow-headers': '*',
103+
'access-control-expose-headers': '*',
104+
'access-control-max-age': '600',
105+
});
106+
80107
const sendJson = (res: ServerResponse, status: number, payload: unknown): void => {
81108
const body = Buffer.from(JSON.stringify(payload));
82-
res.writeHead(status, { 'content-type': 'application/json' });
109+
res.writeHead(status, { 'content-type': 'application/json', ...corsHeaders() });
83110
res.end(body);
84111
};
85112

@@ -96,6 +123,8 @@ export async function startMockCollector(): Promise<MockCollector> {
96123
const issues: Array<Record<string, unknown>> = [];
97124
const uploads: CapturedUpload[] = [];
98125
const echoHeaders: Array<Record<string, string | string[] | undefined>> = [];
126+
const transactions: Array<Record<string, unknown>> = [];
127+
const otlpTraces: Array<Record<string, unknown>> = [];
99128
let echoHits = 0;
100129
let issueSeq = 0;
101130
// Maps an upload path (/upload/<n>) to the issueId we minted for it, so a captured PUT can be
@@ -108,6 +137,15 @@ export async function startMockCollector(): Promise<MockCollector> {
108137
const url = req.url ?? '/';
109138
const method = req.method ?? 'GET';
110139

140+
// Preflight. A cross-origin PUT, and a POST carrying `content-type: application/json`, are both
141+
// non-simple requests, so a real browser sends OPTIONS first and never issues the upload at all
142+
// unless it is answered.
143+
if (method === 'OPTIONS') {
144+
res.writeHead(204, corsHeaders());
145+
res.end();
146+
return;
147+
}
148+
111149
void (async () => {
112150
try {
113151
if (method === 'POST' && url.endsWith('/v2/sessions')) {
@@ -174,20 +212,38 @@ export async function startMockCollector(): Promise<MockCollector> {
174212
} catch (err) {
175213
violations.push({ where: 'manifest', errors: `unreadable bundle: ${String(err)}` });
176214
}
177-
res.writeHead(200);
215+
res.writeHead(200, corsHeaders());
178216
res.end();
179217
return;
180218
}
219+
if (method === 'POST' && url.endsWith('/v2/performance/transactions')) {
220+
const body = await readBody(req);
221+
const batch = JSON.parse(Buffer.from(body).toString('utf8')) as {
222+
transactions?: Array<Record<string, unknown>>;
223+
};
224+
transactions.push(...(batch.transactions ?? []));
225+
sendJson(res, 200, envelope({ accepted: batch.transactions?.length ?? 0 }));
226+
return;
227+
}
228+
if (method === 'POST' && url.endsWith('/v1/traces')) {
229+
// OTLP/HTTP-JSON. A real collector answers with an (empty) ExportTraceServiceResponse.
230+
const body = await readBody(req);
231+
otlpTraces.push(
232+
JSON.parse(Buffer.from(body).toString('utf8')) as Record<string, unknown>,
233+
);
234+
sendJson(res, 200, {});
235+
return;
236+
}
181237
if (url.startsWith('/echo')) {
182238
echoHits += 1;
183239
echoHeaders.push({ ...req.headers }); // captures any injected traceparent/tracestate
184240
sendJson(res, 200, { ok: true, ts: 'e2e' });
185241
return;
186242
}
187-
res.writeHead(404);
243+
res.writeHead(404, corsHeaders());
188244
res.end();
189245
} catch (err) {
190-
res.writeHead(500);
246+
res.writeHead(500, corsHeaders());
191247
res.end(String(err));
192248
}
193249
})();
@@ -203,6 +259,8 @@ export async function startMockCollector(): Promise<MockCollector> {
203259
issues,
204260
uploads,
205261
echoHeaders,
262+
transactions,
263+
otlpTraces,
206264
violations,
207265
get echoHits() {
208266
return echoHits;
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// The literals the browser scenario types into the page and the browser e2e asserts on.
2+
//
3+
// They live in their own module because `browser-scenario.ts` runs on import — it touches `window` at
4+
// module scope, which is correct in a page and a `ReferenceError` in the node test process. Importing the
5+
// values from here keeps ONE definition without dragging the page's side effects into the runner.
6+
7+
/** The value typed into the password field. It must not reach ANY byte of ANY uploaded file. */
8+
export const BROWSER_SECRET = 'pw-e2e-MUST-NOT-SHIP-9f3a';
9+
/** The value typed into an ordinary (non-sensitive) field. */
10+
export const BROWSER_VISIBLE_TEXT = 'ordinary-visible-input';
11+
/** Text rendered into the page body, masked by rrweb's `maskAllText`. */
12+
export const BROWSER_BODY_TEXT = 'body-text-e2e-marker';

0 commit comments

Comments
 (0)