Skip to content

feat(runner): Sentry error monitoring for the authoring app + API worker (DEV-2134) - #88

Open
demtario wants to merge 6 commits into
masterfrom
feat/DEV-2134-sentry-api
Open

feat(runner): Sentry error monitoring for the authoring app + API worker (DEV-2134)#88
demtario wants to merge 6 commits into
masterfrom
feat/DEV-2134-sentry-api

Conversation

@demtario

@demtario demtario commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Adds Sentry error reporting to the two runner surfaces: the authoring app and the API worker. Errors only — no tracing, no session replay, no profiling.

DEV-2134: https://app.clickup.com/t/86cax9zfa

Why

The runner had no error reporting at all. DEV-2129 (Babel 6.26 parse failure across 8 examples), DEV-2130 (blank React demo from a jsx/tsx entry mismatch) and the Tier-2 5-slot container exhaustion were all found by a human clicking around or by the link-sweep script, days after they started failing. Workers Logs gives raw lines with short retention, no grouping and no alerting.

Build settings — in place

Where Name Value
repo secret SENTRY_AUTH_TOKEN Sentry Organization Auth Token (sntrys_…), fixed org:ci scope
repo variable SENTRY_ORG handsoncode
repo variable SENTRY_PROJECT demos

All three are set, and deploy-runner-authoring.yml reads them from those exact places. vite.config.ts enables source-map upload only when all three are present, so a partial or broken setup is a clean no-op rather than a broken deploy.

Worth knowing for later: a failed upload (rotated token, wrong slug) does not fail the build — sentry-cli errors and vite still exits 0. Verified with fake credentials. The symptom is unreadable minified traces in Sentry, not a red deploy, so check the deploy log for [sentry-vite-plugin] Error if prod traces ever stop resolving.

apps/authoring

  • @sentry/react initialised from a dedicated src/sentry.ts, imported first in main.tsx so a throw during module evaluation is still caught. Callsites report through a reportError helper instead of importing the SDK, which keeps the production gate (below) authoritative in one place.
  • Sentry.ErrorBoundary at the app root. An editor-shell render crash previously left a blank page and no record of why.
  • Reporting added at the callsites that swallowed a failure whole: saved-demo load, docs-example load and switch (tagged by which step failed — the DEV-2130 shape), versions fetch, broker /broker/userinfo, and demo save / fork / embed / share / revoke. The writeFile/deleteFile "not mounted" catches are deliberately left alone; that is expected control flow, not a fault.
  • @sentry/vite-plugin uploads source maps and tags the release with github.sha. build.sourcemap follows whether upload is enabled, so a build without the credentials neither emits nor publishes .map files (~12 MB otherwise).

workers/api

  • withSentry() on the fetch handler; instrumentDurableObjectWithSentry on both Sandbox and BuilderSandbox. Tier-2 session orchestration and the share-build snapshotter run inside the DOs, so their failures never reach the handler's error path.
  • The export name Sandbox is load-bearing — proxyToSandbox resolves the live-preview namespace by that literal name — so the wrapper is assigned to it rather than aliased. The as unknown as casts dodge TS2589 on the Sandbox SDK's recursive RPC generic, mirroring the existing proxyToSandbox cast a few lines below.
  • The outer try/catch converts every unexpected throw into a 500 body, so withSentry() alone would never observe one. captureException is called there, plus at the two npm-registry probes that silently degrade version pinning. InvalidFilePathError stays unreported — it is input validation returning a 400, not a fault.
  • observability.enabled stays on; this is additive.

Decisions worth a reviewer's attention

The Worker's DSN var is ERROR_REPORTING_DSN, not SENTRY_DSN. This looks like a naming quirk and is not. @sentry/cloudflare's getFinalOptions does dsn: userOptions.dsn ?? getEnvVar(env, "SENTRY_DSN") — it reads that exact key straight off env whenever the options object omits a dsn. Under that name the SDK initialises from raw env and silently bypasses the production gate below. Commented at both the definition and the use site so it does not get "fixed" later.

Nothing is reported outside production, and that needed a gate rather than an absent DSN. .env.production is committed, so it is loaded by ci.yml's authoring build — and playwright.config.ts serves that exact build at localhost:4173. Without a gate, every PR run and every local vite preview would file events against the production project. The browser gates on window.location.hostname; the Worker gates on PREVIEW_HOST, the prod-vs-local switch the Tier-2 preview URLs already rely on and which .dev.vars already overrides.

Preview failures are classified by error class, not dropped wholesale. The preview iframe runs arbitrary authored and imported example code, so a compile error or a mid-keystroke typo is product output, not an application fault — reporting those would bury the signal under one issue per syntax error. But DemoRuntime.onError is a mixed channel:

Emitter Error Action
sandpack.ts compile error in the code being edited drop — product output
container.ts "The session was closed." drop — normal teardown
container.ts ContainerBootFailure report
container.ts dev-server / WS error report
mount() rejection SessionStartError (has .status) report unless 410

SessionStartError is the Tier-2 pool refusing a session — the exhaustion class behind #87, and one of the things this PR exists to surface. reportRuntimeError in App.tsx does the classification, next to the existing describeRuntimeError. Both container cases are fingerprinted by class so per-example boot-log text doesn't shard one problem into hundreds of issues.

Note that the beforeSend foreign-frame filter is a weak backstop, not the guarantee. A genuinely cross-origin iframe error arrives as "Script error." with no stack frames, so it would pass the filter. What actually protects the quota is the engine !== "container" early return.

The Worker release comes from the version_metadata binding. Cloudflare's own per-deploy version id, so deploy-runner-api.yml needs no change at all — no SHA plumbing, and nothing that has to interact with the four --routes flags already in the deploy script.

The DSN is committed, in two places, on purpose. A DSN is a write-only ingest endpoint and ships inside the JS bundle by construction, so hiding it buys nothing — and keeping it out of wrangler secret means changing it needs no Cloudflare access. Abuse is bounded Sentry-side (allowed domains, inbound filters, spike protection). It sits beside VITE_API_BASE in .env.production and beside LOGIN_BROKER_URL in the wrangler.jsonc vars block, both already labelled as public non-secret config.

packages/runtime and packages/editor-shell stay uninstrumented. They are libraries; they keep surfacing errors through their existing interfaces and the classifier lives in the app.

Verification

Local, pre-deploy:

  • pnpm typecheck clean · 67/67 unit tests · pnpm e2e 10 passed / 85 skipped.
  • Gate closed: real production bundle served at localhost:4173 — no Sentry client is created, and an unhandled error produces zero ingest requests (checked in a real browser, network panel).
  • Send path open: with the gate temporarily pointed at localhost, the envelope POST returns 200 — DSN and project confirmed valid.
  • Worker: gate closed → SDK logs "No DSN provided, client will not send events"; gate open → client initialises and captures.
  • DO wrapping is safe at runtime. This was the real risk in the Worker half: Sentry's Proxy hands the SDK an Object.create(ctx) stand-in for DurableObjectState. Booted a real Tier-2 session and proxied its preview URL through the wrapped Sandbox DO — neither the Sandbox SDK nor proxyToSandbox name resolution breaks.
  • Preview errors are not reported — both paths checked. Full stack running locally (API worker + authoring dev server), gate temporarily open. A deliberate app error produced exactly one envelope (HTTP 200), proving the client was live and sending. Then, in src/index.tsx of the React starter: (a) a syntax error produced no event and no UI error, because pushUpdate() catches the transpile failure at sandpack.ts:248 and keeps the last good sandbox — it never reaches the bundler; (b) an unresolvable import — valid syntax, fails at bundle time — did surface Error: Setup failed in the UI through show-erroremitErroronError, and still produced no event. Envelope count stayed at 1 across both.
  • Mount churn does not flood issues. reportRuntimeError fires even when the mount effect was cancelled, on the grounds that a refused session still failed — which makes rapid example/version switching the obvious volume risk. Raced POST /api/session against an immediate DELETE three times: 410 every time, which the classifier drops.
  • A partial SENTRY_* setup builds clean, and a failed upload still runs the post-upload cleanup, so .map files are never published.

Gaps — need prod to close

  • Source-map-resolved stack trace in prod — the first deploy will tell us whether the upload wiring is right.
  • Real events on both surfaces tagged with the Cloudflare release id.
  • Sentry project settings still to configure: spike protection, allowed domains = demos.handsontable.com, default browser inbound filters.
  • One real event ("DEV-2134 authoring smoke error (local verify)") is already in the project from the send-path test above.

Out of scope

  • Public /d/:id viewer and /embed/:id — instrumenting the embed means a Sentry script running on handsontable.com docs pages (consent/PII surface, third-party script in docs, client-environment noise we can't act on).
  • containers/live + containers/builder Node-side instrumentation — needs the Node SDK baked into both images and container-level DSN plumbing. Follow-up; see DEV-2030.
  • Worker source-map upload, tracing, replay, and any alert-routing policy beyond Sentry's default "new issue" mail.

🤖 Generated with Claude Code


Note

Medium Risk
Touches production observability and worker/DO wrappers on session and sharing paths, but does not change auth or business logic; main risks are mis-gated events or DO instrumentation regressions, which the PR documents and tests locally.

Overview
Adds Sentry error reporting (errors only, no tracing/replay) to the demo runner’s authoring SPA and API Worker, with strict production-only gates so CI, vite preview, and wrangler dev do not pollute the prod project.

Authoring: New sentry.ts initializes @sentry/react only on demos.handsontable.com; reportError centralizes tagged captures at previously silent failures (demo load/save/fork/embed/share, docs loads, versions fetch, broker userinfo). A root ErrorBoundary replaces blank-page crashes. Container preview faults (SessionStartError, ContainerBootFailure) are reported with fingerprints; Sandpack user-code errors are not. Deploy builds pass SENTRY_* + GITHUB_SHA so @sentry/vite-plugin uploads source maps and strips .map from dist/.

API Worker: withSentry wraps the fetch handler; Sandbox and BuilderSandbox DOs are instrumented. DSN is ERROR_REPORTING_DSN (avoids SDK auto-init via SENTRY_DSN); reporting is on when PREVIEW_HOST matches prod. Unexpected handler errors and npm registry probe failures are captured. Release uses CF_VERSION_METADATA.

Docs cover tokens, variables, and operational notes (failed map upload does not fail the build).

Reviewed by Cursor Bugbot for commit 797cb9b. Bugbot is set up for automated code reviews on this repo. Configure here.

demtario and others added 3 commits July 27, 2026 15:36
The runner had no error reporting: DEV-2129, DEV-2130 and the Tier-2 slot
exhaustion were all found by hand or by the link-sweep script, days after they
started failing. Workers Logs gives raw lines with no grouping or alerting.

Wraps the fetch handler with withSentry() and both Durable Objects with
instrumentDurableObjectWithSentry() — Tier-2 session orchestration and the
share-build snapshotter run inside the DOs, so their failures never reach the
handler's error path. Release comes from the Cloudflare version_metadata
binding, so no deploy-workflow change is needed.

The outer catch converts every unexpected throw into a 500 body, so withSentry()
would never observe it; captureException is called there and at the two npm
registry probes, which silently degrade version pinning. InvalidFilePathError
stays unreported — it is a 400, not a fault.

Errors only: tracesSampleRate 0, no replay, no profiling.

Two things worth knowing for the follow-up frontend change:

- The DSN var is named ERROR_REPORTING_DSN, not SENTRY_DSN. The SDK's
  getFinalOptions falls back to env.SENTRY_DSN whenever the options object omits
  a dsn, which initialises the client straight from env and defeats any gate.
- Reporting is gated on PREVIEW_HOST matching the production host, the
  prod-vs-local switch Tier-2 previews already use. Verified locally: with the
  gate closed the SDK logs "No DSN provided, client will not send events"; with
  it open the client initialises and captures. A Tier-2 session was booted and
  its preview URL proxied through the wrapped Sandbox DO to confirm the Proxy
  does not break the Sandbox SDK or proxyToSandbox() name resolution.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Second half of DEV-2134, after the API worker. Adds @sentry/react behind a
single init module (src/sentry.ts) plus an error boundary — an editor-shell
render crash used to leave a blank page and no record of why.

Reports at the callsites that previously swallowed a failure whole: saved-demo
load, docs-example load and switch (the DEV-2130 class, tagged by which step
failed), versions fetch, broker userinfo, and the demo save/fork/embed/share/
revoke calls. The writeFile/deleteFile "not mounted" catches are left alone —
that is expected control flow.

reportRuntimeError classifies preview failures instead of reporting all or
nothing. DemoRuntime.onError is a mixed channel: on the Sandpack engine it
carries compile errors from the code being edited, which are product output and
would bury the signal under one issue per keystroke. On the container engine it
carries SessionStartError (the Tier-2 pool refusing a session, the failure class
behind PR #87) and ContainerBootFailure, which are exactly what we want. 410 is
excluded as normal teardown, and both are fingerprinted by class so per-example
boot logs don't shard one problem into hundreds of issues.

Nothing is reported outside production. .env.production is committed and is
therefore loaded by ci.yml's authoring build, whose output Playwright serves at
localhost:4173 — so without a gate every PR run would file events. Init is
conditional on window.location.hostname, mirroring the Worker's PREVIEW_HOST
gate.

Source maps upload via @sentry/vite-plugin on the deploy workflow's build step
only; build.sourcemap follows the token's presence, so a build without one
neither emits nor publishes .map files. Needs repo secret SENTRY_AUTH_TOKEN and
repo variables SENTRY_ORG / SENTRY_PROJECT (slugs, not the numeric DSN ids).

Verified in a real browser against the production bundle: on localhost no client
is created and an unhandled error produces zero ingest requests; with the gate
temporarily pointed at localhost the envelope POST returns 200, confirming the
DSN and project. Typing broken syntax into the editor produced no event, but the
hosted Sandpack bundler was unreachable in that environment, so the preview never
raised a compile error — repeat that negative check on prod.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ap upload

Follow-up to the authoring instrumentation, from review.

A token with no SENTRY_ORG/SENTRY_PROJECT left the plugin enabled with no upload
target. Verified with fake credentials: sentry-cli fails with 401, and vite still
exits 0 — so a partial setup degrades silently to unreadable minified traces
rather than failing the deploy. The enable condition now requires all three, so a
partial setup is a clean no-op; documented the failure mode and the fact that
post-upload cleanup still runs (a failed upload never publishes .map files).

Also normalises the release: the `define` substitutes "" when GITHUB_SHA is
absent, and a release of "" would not match the SHA-named artifact bundle the
plugin uploads, silently breaking map resolution.

Separately confirmed the cancelled-mount reporting decision. reportRuntimeError
fires even when the mount effect was cancelled, on the grounds that a refused
session still failed; the risk was that rapid example/version switching would
make this the highest-volume issue source. Raced POST /api/session against an
immediate DELETE three times against the local worker: all three returned 410,
which is the status the classifier drops.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario demtario self-assigned this Jul 27, 2026
The token scope was documented as `project:releases` + `org:read`, which is the
older user-auth-token wording. An Organization Auth Token — what the bundler
plugin wants — has a fixed `org:ci` scope covering source-map upload and release
creation, with nothing to select.

Also names the org/project slugs (handsoncode / demos) instead of telling the
reader to go find them, and calls out the Deploy Token on the release-tracking
page as the wrong credential: it drives the release webhook and cannot upload
source maps.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario

Copy link
Copy Markdown
Contributor Author

@qunabu In https://handsoncode.sentry.io/settings/projects/demos/ -> Project security, you need to set allowed domain to demos.handsontable.com. I dont have permissions to do that

@demtario
demtario marked this pull request as ready for review July 28, 2026 08:24
Comment thread runner/apps/authoring/src/sentry.ts Outdated
Reported by Bugbot on #88, and it reproduces. `ignoreErrors` is applied by the
event-filters integration, which processes every event — including explicit
captureException calls — so the `/Failed to fetch/i` and `/Load failed/i`
patterns were discarding exactly the failures reportError was added to surface.
An offline broker /broker/userinfo call or a dead /api/versions raises
`TypeError: Failed to fetch`; verified in a browser that with those patterns in
ignoreErrors the reported event never reaches the transport.

The patterns are only meant to suppress global onerror/onunhandledrejection
noise, so they now live in beforeSend behind a `mechanism.handled === false`
check. captureException sets `handled: true`, and mechanism is populated before
beforeSend runs, so intentional reports are never matched.

Verified in a browser: reportError with `TypeError: Failed to fetch` and
`TypeError: Load failed` both reach the transport, while an unhandled
ResizeObserver error and an unhandled `Failed to fetch` rejection are both
dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 94e54f8. Configure here.

Comment thread runner/apps/authoring/src/MyDemos.tsx
Reported by Bugbot on #88. demo-revoke only reported from the fetch .catch, so a
403/404/500 DELETE left `res` non-null, skipped the success branch, and reported
nothing — while the list stayed exactly as it was with no error shown. That is
the silent-failure case the reporting was added for, and the one path where the
comment claiming to cover it was wrong.

save/fork/embed/share already report after !res.ok; revoke now matches.

Verified in a browser with the DELETE stubbed to 403: the row stays unrevoked,
no error surfaces in the UI, and `revoke failed (403)` is reported with context
demo-revoke. Before the change that produced nothing at all.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@demtario
demtario requested a review from qunabu July 28, 2026 08:59
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.

1 participant