DEV Community

Cover image for The bug report that never left the browser
Juan Torchia
Juan Torchia Subscriber

Posted on

The bug report that never left the browser

Summer Bug Smash: Clear the Lineup πŸ›πŸ›Ή

This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.

There's a shape of bug I've learned to distrust: the one where the safety net is bolted to the thing it's supposed to catch.

I was reading Element Web's reporting code looking for something worth fixing when I hit a function that builds the whole Sentry payload as a single object literal β€” with two await calls sitting inside it. One of them asks the crypto layer for diagnostics. Optional diagnostics. Nice-to-have detail on a report that is already complete without them.

I stopped there, because I could already see how that sentence ends. If the optional thing rejects, the object never exists. If the object never exists, there is no capture call. And the same pattern was waiting one directory over, in the rageshake path.

The subsystem being diagnosed could prevent the diagnostic report from leaving the browser.

Somebody decides to tell you what broke, and the broken part gets a veto. One deliberate press of a button, both explicit channels gone: the rageshake bundle and the manual Sentry event.

I measured it at the boundary that actually counts β€” a real Sentry Browser SDK with a local, network-free transport. Under the same synthetic failure: zero serialized events before the fix, exactly one after.

Before and after: under the same synthetic crypto rejection, the original flow aborts the rageshake and serializes zero Sentry events, while the hardened flow preserves the rageshake and serializes exactly one Sentry event

Same synthetic crypto rejection

Before                          After
  collectBugReport(): rejected    report completed with available diagnostics
  Sentry envelopes:   0           Sentry events: 1
                                  unrelated context families: retained
                                  auxiliary error message or stack: absent
Enter fullscreen mode Exit fullscreen mode

Project Overview

Element Web is the web client behind Element, a Matrix-based communication app. Its bug-report dialog can send two independent things: a rageshake bundle β€” logs and diagnostics packed into multipart form data and posted to a configured endpoint β€” and, when Sentry is configured, a single manually captured Sentry event.

Both are explicit. Nothing leaves the browser unless a person opens that dialog and submits it. That framing shaped every decision below: this isn't background telemetry, it's someone choosing to hand over evidence, usually while something is already broken.

The invariant I set out to restore is deliberately narrow:

A diagnostic collector that throws or rejects must not abort the explicitly submitted report.

It promises nothing about a collector that hangs forever, and nothing about failures in base report construction, compression, transport or the SDK itself. A contract you can't test is a slogan.

Bug Fix or Performance Improvement

A reproduction I could run on demand

I needed a failure that was deterministic and boring, not a screenshot of something weird. On develop at commit 7ffb0ffced8b93b7f4a697ff53b4344eb1c32fa6 I made getOwnDeviceKeys() return a rejected promise β€” a synthetic error, a synthetic identity, a synthetic device.

The result on both paths:

  • collectBugReport() rejected before producing its FormData;
  • sendSentryReport() rejected before reaching captureException();
  • the real Sentry Browser SDK's local transport received zero envelopes.

That establishes a mechanism, not a frequency. I don't know how often this rejects for real users, and I'm not going to dress a code-level reproduction up as an incident. What I can say precisely is that when it does happen, one user action loses both channels.

The root cause is an ordering problem, not an error-handling problem

In the rageshake path, optional enrichment sat directly in the critical path:

const cryptoApi = client.getCrypto();

if (cryptoApi) {
    await collectCryptoInfo(cryptoApi, body);
    await collectRecoveryInfo(client, cryptoApi, body);
}
Enter fullscreen mode Exit fullscreen mode

None of that is the report. It's decoration on a report that already contains the user's text, their IDs and the logs. But an unhandled rejection propagates, and the caller never gets its FormData.

The Sentry path had the same shape in a more brittle container:

return {
    user: getUserContext(client),
    crypto: await getCryptoContext(client),
    device: getDeviceContext(client),
    storage: await getStorageContext(),
};
Enter fullscreen mode Exit fullscreen mode

An object literal is an unforgiving place to fail. One rejected property and there is no object, no capture call, no event.

Why I didn't wrap it in one big try/catch

The one-line fix is obvious and I rejected it on purpose.

A single boundary around the whole collection block does keep the report alive, but it throws away every context that had already been gathered successfully β€” the first failure erases the work of the collectors that were fine. It also flattens several distinct failures into one anonymous "something went wrong". You'd get a report, just a poorer one, at exactly the moment you need a richer one.

The other candidate was a shared concurrent collector built on Promise.allSettled. It deletes the duplication between the two pipelines, and it also changes call ordering, concurrency against crypto and the homeserver, and two payload models that don't want to be one. That's a refactor looking for a bug, and this bug is narrow. Possible future work; not this patch.

What I shipped is the middle option: one failure boundary per diagnostic family, sequential, preserving the existing order.

async function collectContext<T>(family: ContextFamily, collector: () => T | Promise<T>): Promise<T | undefined> {
    try {
        return await collector();
    } catch {
        logger.warn(`Failed to collect ${family} context for Sentry report`);
        return undefined;
    }
}
Enter fullscreen mode Exit fullscreen mode

The two pipelines then degrade according to how they already build data, and I left that asymmetry alone. Sentry assembles its context object at the end, so a failed family is simply absent from the event. Rageshake appends to FormData as it goes, so fields written before a late rejection survive into the submitted bundle. Those models differ for reasons that predate me; the fix respects them instead of inventing a third one.

This is the position I'll defend: resilience isn't a pile of catch blocks, it's a decision about what a failure is allowed to mean. The interesting work wasn't catching the exception β€” it was deciding that a diagnostic family may disappear and the report may not.

And the trade-off I'll say out loud instead of hiding in a design doc: the two pipelines now duplicate a little boundary logic. I'll take duplication a reviewer can hold in their head over a shared abstraction that quietly changes call ordering in code I don't maintain. Cheap to delete later if a maintainer disagrees; expensive to unwind if I'm wrong.

The boundary I thought I had built

My first implementation passed every test I had written, which is a much weaker statement than it feels like when the runner turns green. A green suite proves the cases you thought of; that's it. So I treated it as a hypothesis and went looking for the case I hadn't written down.

client.getCrypto() is a synchronous call, and it was still sitting outside the boundary. A synchronous throw there skipped both try blocks and aborted the report exactly as before β€” same bug, one line earlier.

Passing tests had proved the cases I wrote down. They had not yet proved the boundary I thought I had built.

let cryptoApi: CryptoApi | undefined;
try {
    cryptoApi = client.getCrypto();
} catch {
    logger.warn("Failed to collect crypto information for bug report");
}
Enter fullscreen mode Exit fullscreen mode

The accompanying regression forces that synchronous throw and asserts that user_id, device_id and the user's text still make it into the FormData.

Fixed warnings, and a privacy claim I refuse to inflate

Every catch block logs a constant string. No interpolated error, no message, no stack, no key names, no identifiers.

That isn't a style preference. Element enables Sentry's console breadcrumb integration, and the rageshake collector attaches captured logs to the bundle. Anything I log can travel with the report. Writing logger.warn(`crypto failed: ${err}`) would have quietly turned a log line into a data path, and the object I'd be interpolating comes from the crypto layer.

The claim I deliberately do not make is "the payload is unchanged and contains no PII". Both halves would be wrong. These reports already carry Matrix IDs, device IDs, device public keys, local settings, the user's own text and an issue URL β€” by design, with consent, before I touched anything.

The accurate version is narrower and more useful: the fix introduces no new fields and no new categories of user data, and it restores delivery of the report the user explicitly submitted. In the failure scenarios the effective change is from nothing being delivered to the pre-existing, consented payload being delivered. That is a real behavioral change, and burying it under "no PII" would have been the easy, dishonest option. Getting the sentence right was part of the fix, not a footnote.

Code

The production change touches four files and nothing else:

apps/web/src/rageshake/submit-rageshake.ts
apps/web/src/sentry.ts
apps/web/test/unit-tests/sentry-test.ts
apps/web/test/unit-tests/submit-rageshake-test.ts
Enter fullscreen mode Exit fullscreen mode

The real-SDK harness is not in that set. Element asks for unit tests in Jest under /test, so the Vitest transport harness stays outside the patch and lives in the public evidence package instead. It's my proof, not their maintenance burden.

I also found an unrelated typo in the storage diagnostics while reading that file. It doesn't share a root cause with this bug, so it isn't in this commit.

My Improvements

The Jest regressions run in Element's official runner and cover the happy path unchanged, a missing Matrix client, a missing crypto API, a synchronous throw while obtaining it, early and late crypto rejection, rejected browser storage APIs, all four Sentry families failing at once, exception and message capture exactly once, missing Sentry configuration, rageshake crypto and recovery failing independently and together, partial FormData retention after a late rejection, and warning messages asserted as fixed strings with no auxiliary error object attached.

Test Suites: 2 passed, 2 total
Tests:       63 passed, 63 total
Enter fullscreen mode Exit fullscreen mode

The criterion I hold myself to: a regression that would also pass before the fix is documentation, not a test. The two I cared about most were written against the unpatched tree first and watched to fail there β€” the characterization test that asserts collectBugReport() rejects, and the real-SDK baseline that asserts zero envelopes. Everything else is a guardrail hanging off those two.

One behavior change deserves to be stated out loud rather than discovered in review. The Sentry path used MatrixClientPeg.safeGet(), which throws when there is no client β€” a normal situation that produced no event at all. It now uses get() and captures the event with whatever context is available, typically storage. In that scenario the outcome genuinely moves from zero events to one, and that is the point of the patch, not a side effect of it.

Best Use of Sentry

I'm entering this for Best Use of Sentry because Sentry Error Monitoring is part of the failure, part of the reproduction, and part of what convinced me the fix works.

To be exact about credit: Sentry did not find this bug. I found it reading code. What Sentry did was decide what counted as proof β€” and then invalidate one of my testing assumptions.

The bug happened before Element could call captureException() or captureMessage(). A mock can tell you the call happened after the patch. It cannot tell you the SDK accepted the payload, serialized a valid event, or kept the auxiliary failure out of the envelope. So instead of asserting against a spy, I initialized the real @sentry/browser 10.67.0 with a synthetic DSN and a custom transport built with Sentry.createTransport() that performs no network request and simply keeps every serialized request in memory. The harness then parses each envelope item β€” headers, item type, JSON body β€” and asserts on what Sentry actually produced:

  • before: zero envelopes;
  • after: exactly one item of type=event;
  • the primary exception or message intact;
  • unrelated context families and extra retained;
  • no auxiliary error message or stack anywhere in the envelope;
  • flush() returning true, so "no envelope" never means "still buffered".

Then Sentry broke my test, which was the most valuable thing that happened all week.

My first harness ran with defaultIntegrations: false. Clean, isolated β€” and not what production does. Element runs with console breadcrumbs enabled, which means those fixed logger.warn() calls can be captured and serialized inside the event. My harness was structurally incapable of seeing the one path where my own logging could leak. So I enabled Sentry.breadcrumbsIntegration() and asserted on the breadcrumbs directly:

expect(getContextFailureBreadcrumbs(event).map(({ message }) => message)).toEqual([
    "Failed to collect user context for Sentry report",
    "Failed to collect crypto context for Sentry report",
    "Failed to collect device context for Sentry report",
    "Failed to collect storage context for Sentry report",
]);
for (const breadcrumb of getContextFailureBreadcrumbs(event)) {
    expect(breadcrumb.data?.arguments ?? []).toEqual([breadcrumb.message]);
}
Enter fullscreen mode Exit fullscreen mode

Turning that integration on immediately surfaced a second problem: breadcrumbs bled between test cases, because the SDK keeps them on more than one scope. The fix was to clear both the current scope and the isolation scope before each scenario, and to run the whole file twice in separate Vitest processes so I could be sure nothing was passing thanks to residual global SDK state.

Test Files: 1 passed
Tests:      6 passed
Enter fullscreen mode Exit fullscreen mode

The breadcrumb work wasn't bolted on to qualify for a category. It changed how the fix is validated: the evidence now covers not just "an event was captured" but the diagnostic metadata Sentry would really serialize, including the assertion that a fixed family name travels and the underlying error text does not.

What I proved

A deterministic crypto rejection could abort both explicit reporting paths. The same rejection produced zero Sentry envelopes before the fix and exactly one serialized event after it. Unrelated context families survive, rageshake keeps whatever it had already appended, warnings are fixed and low-cardinality, and the auxiliary failure's message and stack stay out of the envelope. No automatic trigger was added, consent didn't change, and the payload schema didn't change.

What I don't know

How often this rejection occurs in production. How many users or reports have been affected. Whether Element's maintainers will prefer this scope, a narrower one, or a different design entirely. Whether the invariant should eventually cover collectors that hang instead of rejecting β€” that changes timing policy and possibly partial data, so it's a question for maintainers rather than a silent expansion of the patch.

Where this actually stands

As of 10 August 2026: the upstream issue is open and was triaged with the T-Defect and A-Feedback-Reporting labels. It has no assignee and no maintainer response. I left one respectful follow-up asking whether a focused patch would be welcome, and then I stopped β€” an open issue isn't mine, and a queue isn't a snub.

So the verifiable pull request lives in my fork. It has not been merged upstream, no upstream CI has run on it, and the fork PR reported no GitHub checks. The counts above come from Element's own Jest configuration and my Vitest harness on my machine, against the linked commit. Locally I also verified scoped formatting and linting, a clean reverse-apply of the patch. That's evidence, not approval, and I'd rather say so than let a green checkmark be implied.

If a maintainer prefers a smaller diff, a different failure boundary, or nothing at all, the reproduction still stands on its own.

Closing

The most valuable bug report is usually written while something is already broken. That's its entire reason to exist β€” and it's exactly the moment when the code gathering extra detail is most likely to fail.

Here's the portable version, and it costs you about ten minutes. Open whatever collects context before your app ships an error, a crash or a support bundle, and read it as a plain list of awaits. For each one ask a single question: if this rejects, does the report still leave the machine? Anything that answers "no" isn't enrichment. It's a dependency nobody agreed to take on, hiding behind the word optional.

Optional diagnostics should make a report more useful. They should never get a vote on whether it exists.

Top comments (0)