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.
Short version: both explicit reporting paths awaited optional crypto diagnostics before completing, so one rejected collector aborted the rageshake bundle and the manual Sentry event. Under the same synthetic failure, a real Sentry Browser SDK serialized zero events before the fix and exactly one after. Four files, no new data, no new triggers.
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
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 itsFormData; -
sendSentryReport()rejected before reachingcaptureException(); - 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.
I can't tell you how often that rejection fires. I can tell you something about which reports it takes, though: not a random sample. The ones lost are exactly the reports filed from sessions where the crypto layer is already unhealthy — the population you most need diagnostics from. A failure mode that drops reports uniformly costs you volume. One that drops them selectively costs you the signal.
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);
}
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(),
};
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;
}
}
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");
}
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
- Issue: https://github.com/element-hq/element-web/issues/34526
- Pull request (my fork): https://github.com/JuanTorchia/element-web/pull/1
- Immutable commit: https://github.com/JuanTorchia/element-web/commit/51382cacce103a28a0888a90c57c38040a069dc5
- Public A/B evidence package: https://gist.github.com/JuanTorchia/80f95db2ee70f8d465aea0b891aaf414/3314d1249661fccadab38376e37216106065f6ff
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
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
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
extraretained; - no auxiliary error message or stack anywhere in the envelope;
-
flush()returningtrue, 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]);
}
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
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.
Update — 11 August 2026. @gnomeman4201 asked in the comments where a timeout would belong if this invariant ever covered collectors that hang, and then went and measured it. With the real Sentry Browser SDK and a network-free transport, a Promise.race deadline lets the report serialize exactly one event without the slow family — while the collector is still running, completing only after the envelope has already gone out. With a collector that honours an AbortSignal, the lifetime assertion inverts and it aborts instead. The narrow conclusion: a race proves the report stopped waiting, not that the collector stopped. It does not establish that Element's crypto API supports cancellation, and he was careful to keep that separate. His harness, CI run and evidence record: github.com/GnomeMan4201
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.
If you've drawn that boundary in your own reporting path — especially if you've handled the hang case and not just the rejection — I'd like to hear where you put it.


Top comments (8)
Really liked the distinction you make between proving the failure mechanism and claiming anything about its production frequency.
What got me thinking was the boundary you deliberately leave open around collectors that hang rather than reject. If you eventually extended this invariant to cover hangs, how would you decide where the timeout belongs?
Would you treat the timeout as a property of each diagnostic family, a global reporting deadline, or something stemming from the amount of evidence already collected?
It seems like that choice changes the semantics quite a bit especially once “optional enrichment must not block the report” becomes “optional enrichment only gets N milliseconds to participate.”
@gnomeman4201 Thanks — and your last line states it better than my post does. "Optional enrichment only gets N milliseconds to participate" is the whole thing in one sentence. I'd been filing it under "timeouts are out of scope", which is mostly a way of not thinking about it.
Short version of why it stayed out: a rejection is something the code observes. A hang isn't. Nothing ever tells you a promise isn't coming back — you pick a moment and declare it dead. That's a policy call, and I didn't want to smuggle one in under a resilience fix.
If I had to pick one and defend it in review: global deadline. The person is waiting on a report, not on six diagnostics, and per-family budgets add up — six families at two seconds each is a twelve-second button.
But there's a wrinkle I keep bumping into, and it's why I'm not sure any of the three options is really "the" answer. The obvious implementation is a race — Promise.race([collector, timeout]) — and that doesn't cancel anything. The hung collector is still running, still holding whatever it was holding. A deadline buys back control flow, not resources. So "optional enrichment gets N milliseconds" really means "the report stops waiting after N milliseconds", which is honest enough to ship but a weaker promise than it sounds. Doing it properly needs real cancellation that the underlying crypto API may not offer.
The other thing nagging me: a global deadline is order-dependent. Crypto is collected before storage today, so a slow crypto layer means storage silently never gets gathered. No error, just a hole that always appears in the same place. Fixing that means collecting families concurrently under one deadline — which is exactly the shared collector I argued against in the post, because it changes ordering and load. So the honest version of the timeout costs me the refactor I said wasn't worth it. I don't love that.
Your third option is the one I'd defend least: "enough evidence" needs someone to know which family mattered, and nobody knows that until triage. Though scaling by trigger might survive — a report filed after a crash can probably afford less patience than one someone sat down and typed.
Two things I'd genuinely like your read on. Would you accept the ordering bias as a known cost, or is that a dealbreaker? And if you feel like poking at it, the SDK harness in the evidence gist runs on a local transport with no network — a hang case is maybe twenty lines on top. I'm honestly unsure a deadline reproduces cleanly there; I never wrote that test.
I think I’d accept the ordering bias as a known cost only if the ordering itself became part of the contract rather than an implementation accident. If crypto always gets first claim on a global budget, then “global deadline” quietly contains a priority policy too.
That actually makes me wonder whether the cleaner model is a global deadline plus very small per family ceilings not because those ceilings guarantee cancellation, but because they prevent one family from consuming the report’s entire waiting budget. You’d still have the resource-lifetime problem underneath, which I agree Promise.race() doesn’t solve.
I also like your trigger distinction more than my “enough evidence” option. Evidence completeness is unknowable before triage; trigger semantics are known at collection time.
And yes, I’d poke at the harness. The interesting test to me isn’t just whether the deadline reproduces, but whether we can make the distinction observable between “report stopped waiting” and “collector actually stopped.” Those are two very different guarantees.
That reframing is better than what I had. If crypto always gets first claim on the budget, the deadline isn't only a deadline — it's an implicit priority policy, and right now that priority is an accident of the order someone wrote the collectors in. Making it a contract forces the question I'd been avoiding: why should crypto go first?
Once you ask it out loud, today's order looks backwards. The cheap, local, near-certain collectors — settings, device, storage — are the ones most likely to complete inside any budget. Crypto is the expensive one, and for this particular bug it's also the subsystem most likely to be the reason the person is filing a report at all. Collecting it first means spending the report's waiting budget on the thing least likely to return. "Cheapest and most reliable first" would mean a slow crypto layer costs you crypto context, and not everything queued behind it.
So I think your hybrid is the right shape, and better than what I proposed: a global deadline for what the person actually experiences, plus small per-family ceilings so no single collector can eat the whole budget. Two numbers to calibrate instead of one is a real cost, but it turns the failure from "storage is never collected" into "storage gets whatever is left", which is a different category of wrong. And it makes the priority explicit, which is what you were asking for.
Your test framing is the part I want to steal. "Report stopped waiting" versus "collector actually stopped" is exactly the distinction a race hides, and it's observable in the harness: have the synthetic hanging collector flip a flag when it eventually resolves, then assert both that the envelope went out without that family's context and that the flag flips afterwards. That pair is the proof that the collector outlived the report. With real cancellation the second assertion inverts — the collector should reject instead of completing — so the same test tells you which of the two guarantees you actually built.
I'd be glad if you took a run at it. The harness is in the evidence gist, it needs no network and no DSN beyond the synthetic one. If you write that test I'll link it from the post — and if it turns out the deadline doesn't reproduce cleanly, that's worth knowing too. I'd rather publish that than a version of the invariant I can't demonstrate.
Took you up on it. The distinction reproduces cleanly with the real Sentry Browser SDK/local transport.
With a synthetic collector held unresolved, the deadline wins and exactly one event serializes without that context while the collector is still alive. Releasing it afterward makes it complete after the envelope has already gone out. With an AbortSignal aware collector, the lifetime assertion flips: it aborts/rejects instead of completing.
So the narrow result is: Promise.race() proves “report stopped waiting,” not “collector stopped.”
I also ran a happy path control and the whole suite twice in separate Node processes. I kept the cancellation result separate because it obviously doesn’t establish that Element’s actual crypto API supports cancellation.
I put the harness, CI, and evidence record in an immutable commit if you want to link it or pick it apart:
github.com/GnomeMan4201/GnomeMan42...
That's a better result than I expected, and the assertion that matters is the one I'd have gotten wrong on my own: collector completed false at the moment the envelope had already serialized. That's the whole distinction, and it's now a fact instead of an argument.
You were right to keep the cancellation case separate, and I'd keep it separate in any writeup. A synthetic collector that honours an AbortSignal proves the pattern works; it says nothing about whether Element's crypto API accepts one. Those are different claims and collapsing them is exactly the kind of shortcut this whole thread has been avoiding.
One consequence I hadn't thought about until I read your result. If a deadline stops the report waiting but leaves the collector running, then the failure mode isn't only a missing context family — it's accumulation. The report is retryable: someone who gets a bad report can press the button again. Each attempt would spawn another collector against a subsystem that is, by hypothesis, already wedged. One orphan is harmless; a user retrying three times against a hung crypto layer is a different conversation. That's an argument that a deadline without real cancellation isn't half a solution, it's a trade with a second cost.
Which also means the question I fudged in the post has a shape now: it isn't "should we add a timeout", it's "does the crypto API support cooperative cancellation, and if not, is stopping the wait worth the orphans". That's something a maintainer can answer with domain knowledge, and it's a much better thing to bring them than an opinion.
I'm going to add an update to the post pointing at your commit and CI run, credited to you. Thanks for actually running it — this is the part I couldn't do alone, and you turned the one boundary I left open into something measured.
The debugging journey is a good reminder that small browser issues can sometimes hide surprisingly complex problems. Thanks for sharing!
Some comments may only be visible to logged-in visitors. Sign in to view all comments.