Skip to content

fix(reo): stop a blocked analytics script from reporting as a Studio error - #1658

Merged
dawsontoth merged 5 commits into
stagefrom
fix/reo-script-load-unhandled-rejection
Aug 28, 2026
Merged

fix(reo): stop a blocked analytics script from reporting as a Studio error#1658
dawsontoth merged 5 commits into
stagefrom
fix/reo-script-load-unhandled-rejection

Conversation

@dawsontoth

@dawsontoth dawsontoth commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Studio loads Reo.dev analytics through the bundled reodotdev package. When an ad blocker blocks its CDN, loadReoScript rejects from the injected script's own onerror — and the chain in useReo had no rejection handler, so it became an unhandled rejection. Because the reject happens inside our bundle, the rejection's only located frame is index-*.js, which means shouldKeepEvent's reo.dev stack filter cannot attribute it away: it reached Datadog Error Tracking as an unhandled Studio error.

Found in the daily RUM review: 14 events across 5 sessions (4.3% of all sessions) in the 24h to 2026-08-27, against 0–1/day for the six days before, on the currently-served build (index-DFE8mV3G.js). Together with a second third-party family this accounted for a third of the day's error-sessions while total error volume fell — errors got broader, not louder.

This catches and drops the rejection (Reo is optional; nothing degrades without it), and starts the chain from a resolved promise so a synchronous throw from the vendor lands in the same handler — useReo() is the first hook in App.tsx with no error boundary above it, so an escaping throw would blank Studio rather than degrade analytics.

For the human reviewer

  1. Which channel logs the swallowed failure. Chosen: silent. Settled during review: gemini flagged the empty catch and suggested console.error. That specific channel is the one this PR cannot use — the RUM SDK instruments console.error and reports it as a RUM error (error.source: "console", the same route shouldKeepEvent.ts:93-97 documents for React Query's handled AxiosErrors), so it would send the failure straight back into the Error Tracking issue this catch exists to remove. Now console.debug, which RUM does not collect (datadogRum only, no datadogLogs), so a genuine Reo breakage is visible in devtools at zero RUM cost. Nothing left open here unless you disagree that debug is discoverable enough — the alternative is a tagged datadogRum.addAction, deliberately not taken because it re-couples analytics failure to the error pipeline.
  2. Fix at the integration vs. in the RUM filter. Chosen: the Reo call site. A reviewer who wants all third-party noise policy to live in shouldKeepEvent.ts could rule the other way. I ruled for the call site because the filter provably cannot do this job — it attributes on stack frames, and this stack's only frame is ours — so the filter alternative would have to match the message text, which that file's design deliberately avoids. The call site also covers the blank-screen case a filter cannot. The two fixes overlap and should not both land.
  3. Promise.resolve().then(...) vs. try { … } catch {}. Chosen: the resolved-promise funnel, so sync and async failure share one handler instead of two. Purely a legibility call; fully reversible.
  4. Mocked loader vs. a real failing <script>. Chosen: mocked reodotdev plus a stack fixture in shouldKeepEvent.test.ts pinning the premise. The real onerror seam is inside the vendor package and no unit test reaches it; if reodotdev ever rejects from somewhere the wrapper does not cover, the suite stays green. Reversible by adding a jsdom test later.

Verification

End-to-end route: not observable end-to-end from the repo — reproducing it needs a real ad blocker blocking static.reo.dev in a browser, and the resulting assertion is the absence of a Datadog event. Covered by unit tests at the seam instead, plus the production evidence above.

  • npx vitest run — 327 files, 2709 passed, 11 skipped, exit 0.
  • npx tsc -b exit 0; npx oxlint --format stylish . exit 0; npx dprint check exit 0.
  • Mutation-checked, both guards (a test written for a containment bug is exactly the kind that silently tests nothing): deleting .catch(() => {}) fails 3 of 6 cases; removing the Promise.resolve() funnel fails the sync-throw case. Restored, 6/6 green. The sync-throw case initially passed under the .catch mutation — round 2 of the review caught that, and it now routes through collectUnhandledRejections so it fails for both.
  • Symmetry check: src/integrations/google/gtm.ts and src/integrations/stripe/* were audited for the same unguarded-loader shape; Reo was the only remote-script loader with an uncaught chain.

Complexity: easy

Review-Coverage: authored=claude; ran=codex; blocked=gemini(quota); declined=cursor-grok,cursor-composer,domain; rounds=4 @ 76614fb

Human-Review-Need: 3 @ 76614fb

…error

`loadReoScript` rejects from the injected script's own `onerror` whenever an ad
blocker blocks static.reo.dev. The chain in `useReo` had no rejection handler, so
that became an unhandled rejection — and because the reject happens inside our
bundle, the only stack frame is ours, which `shouldKeepEvent`'s reo.dev frame
filter cannot attribute away. It reached Error Tracking as an unhandled Studio
error: 14 events across 5 sessions (4.3% of sessions) in the 24h to 2026-08-27,
against 0-1/day for the six days before.

Catch and drop it. Reo is optional analytics and nothing degrades in its absence.
`useReo` is the first hook in `App.tsx` with no error boundary above it, so a
synchronous throw from `loadReoScript` — argument validation in a future vendor
release, DOM access in a hostile embedding — would escape the effect and blank
Studio rather than degrade analytics. Start the chain from a resolved promise so
both failure modes land in the one handler.

Also pins the fix's premise in `shouldKeepEvent`: the loader rejection's only
frame is the Studio bundle, so the reo.dev stack filter provably cannot suppress
it and broadening that rule is not an alternative fix.
The case asserted only that the effect resolved. With the loader called inside
`.then`, a synchronous throw becomes a rejection either way, so it stayed green
with `.catch` deleted — it pinned the funnel but not the containment. Route it
through `collectUnhandledRejections` so it fails for both mutations, and trim
two comment sentences the code already states.
@dawsontoth
dawsontoth requested a review from cb1kenobi August 27, 2026 15:24

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the useReo hook to safely load and initialize the Reo script within a promise chain, preventing unhandled promise rejections and synchronous throws from crashing the application. It also adds comprehensive unit tests for useReo and updates the Datadog integration tests. The review feedback suggests logging the caught error in the .catch block of useReo instead of silently swallowing it, ensuring better visibility for debugging.

Comment thread src/integrations/reo/reo.ts Outdated
@github-actions

github-actions Bot commented Aug 27, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 59.56% 8004 / 13437
🔵 Statements 60.02% 8581 / 14295
🔵 Functions 52.38% 2023 / 3862
🔵 Branches 53.44% 5740 / 10741
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
src/integrations/reo/reo.ts 100% 100% 100% 100%
Generated in workflow #1822 for commit 2f0da6d by the Vitest Coverage Report Action

Addresses gemini's review note that a fire-and-forget chain should not swallow
errors silently. Uses `console.debug` rather than the suggested `console.error`:
the RUM SDK collects `console.error` as an error event, so that would send the
failure straight back to the Error Tracking issue this catch exists to remove.
Debug keeps a genuine Reo breakage visible to anyone with devtools open.
@dawsontoth
dawsontoth marked this pull request as ready for review August 27, 2026 15:35
@dawsontoth
dawsontoth requested a review from a team as a code owner August 27, 2026 15:35

@kriszyp kriszyp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🤖 Reviewed with Codex

Comment thread src/integrations/reo/reo.test.tsx
The catch-path tests asserted only that no unhandled rejection escaped, so
changing the catch to console.error kept them green while the RUM SDK
collected that call and restored the Error Tracking issue the fix removes.

Spy on console.error and console.debug: assert the failure reaches debug and
that error is never called. Verified by mutation — swapping the catch to
console.error turns all three catch-path tests red.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@dawsontoth
dawsontoth added this pull request to the merge queue Aug 28, 2026
Merged via the queue into stage with commit a3c9a5b Aug 28, 2026
2 checks passed
@dawsontoth
dawsontoth deleted the fix/reo-script-load-unhandled-rejection branch August 28, 2026 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants