Uh oh!
There was an error while loading. Please reload this page.
Console error reporting: consume the DSN from runtime config - #6603
Merged
Conversation
ObjectStack's users consume a prebuilt Console and never run a build of it, so the build-time VITE_SENTRY_DSN half of the two-key gate was unreachable for them -- they could neither turn reporting on nor turn PII collection off. app-shell now reads the sink and its knobs from telemetry.errorReporting on /api/v1/runtime/config, and the build-time DSN path is retired rather than kept as a second source. The gate collapses to one input, which is the fix rather than a side effect: resolveSentryGate() takes the runtime payload alone, and its `reason` drops from four values to two because "the runtime declined" and "no DSN arrived" are now the same state, described from the one place an operator looks. VITE_SENTRY_RELEASE survives as the only build-time knob: a release identifies which bundle produced a stack trace and must match that build's uploaded source maps, which no server can know. initSentry() stays sequenced after initRuntimeConfig() and the ordering is now more load-bearing, not less -- the DSN itself arrives from the server. The committed-telemetry-endpoint ratchet keeps its rules unchanged; they key on the variable suffix and the value, never on the VITE_ prefix, so they already cover the runtime-side spelling. That is now pinned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DKWDdUJ2XNRESVVWUvcpnh
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-zhuang
left a comment
Contributor
There was a problem hiding this comment.
PM contract-tier verification record (dispatching seat for objectstack#12681; this seat shares the PR's author identity, so this is a COMMENT review — not a governed surface).
Read the full 1885-line diff. Findings, all positive:
- The gate collapses to one input and the collapse is complete:
resolveSentryGate(runtimeErrorReporting)takes noenvargument any more — nothing a build was compiled with can influence whether reporting happens;reasonhonestly shrinks from four values to two, withruntime-deniedcollapsing INTOno-dsnbecause they are now the same state. The withheld verdict carries no PII and zero sample rates. - Every
VITE_SENTRY_*retires exceptVITE_SENTRY_RELEASE, which is a label read at the call site, never a gate — with the source-maps reasoning recorded in three places a future editor would look. - The ratchet did not narrow:
committed-telemetry-endpoint.test.tsrules are keyed on key-suffix and value, and the new counter-probes pin that a committedOS_TELEMETRY_CLIENT_ERROR_REPORTING_DSNin a.envfile is caught exactly like the retired spelling — a later "tidy-up" cannot reopen the hole under the new name. - The mirror discipline is preserved:
readClientErrorReportingis a declared line-for-line hand copy of the producer's export (this repo deliberately takes no dependency on@objectstack/cloud-connection), the secret-DSN guard is kept on this side as the last line against a third-party host, knobs are re-derived defensively (=== truefor PII, finite 0..1 for rates,0kept as a real answer), andtelemetrystays replace-not-merge so a withdrawn DSN withdraws. - The landing-order pin exists on this side too: a payload carrying only the replaced #10805 boolean reads as null.
- Docs and comments (error-tracking.md,
.env.production, main.tsx ordering note) describe only the final shape; the error-tracking CSP note gains the right new caveat — the DSN can now be repointed without a rebuild, so a hosting-layer CSP can silently go stale.
Cross-repo pair: objectstack#12697 (verified in its own review). Landing via the normal queue; the card closes when both halves are merged.
Generated by Claude Code
os-zhuang
marked this pull request as ready for review
August 27, 2026 10:37
os-zhuang
enabled auto-merge
August 27, 2026 10:37
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of objectstack-ai/objectstack#12681 — the client half. The server half is objectstack-ai/objectstack#12697; neither PR closes the card on its own.
What this changes
The Console's error-reporting DSN — and every knob that travels with it — now arrives from the runtime, on
telemetry.errorReportingofGET /api/v1/runtime/config. The build-timeVITE_SENTRY_DSNpath is retired rather than kept as a second source.The maintainer's ruling on the card, verbatim and untranslated:
ObjectStack's users consume a prebuilt Console. Under the two-key gate a build-time key was unreachable for them, so a self-hosting operator could not enable client error reporting at all — the runtime permission was reachable and the source was not. They also could not turn
sendDefaultPiioff, which is the knob deciding whether IP and User-Agent leave their network.The gate collapsed to one input, and that is the fix
resolveSentryGate()now takes the runtime payload alone — noenvargument. That absence is the point: nothing a build was compiled with can influence whether reporting happens.reasondrops from four values to two.forced-offretired withVITE_SENTRY_ENABLED;runtime-deniedcollapsed intono-dsn, because once the DSN is the grant, "the runtime declined" and "no DSN arrived" are the same state — described from the one place an operator has to look. Turning reporting off is unsetting the server DSN; there is deliberately no build-time force-off left, because nobody consuming a prebuilt console could reach one.The fail-closed posture is unchanged and structurally stronger: absence of a source is not a value that can be misread, where the boolean needed a strict
=== trueplus a written argument about why a negativedisabledflag would have been vacuous on exactly the runtimes that were leaking.What moved to the runtime, and the one knob that did not
Moved into the payload:
sendDefaultPii,environment,tracesSampleRate,replaysOnErrorSampleRate. Not new surface — the same surface relocated to the side that can operate it.VITE_SENTRY_RELEASEstays build-time, and is now the onlyVITE_SENTRY_*variable that exists. A release identifies which bundle produced a stack trace and must match the source maps that bundle's pipeline uploaded; a server cannot know which Console build it is serving. It is a label on the events, never a gate, so it is read at theSentry.initcall site rather than inside the gate.VITE_SENTRY_ENABLED,VITE_SENTRY_ENVIRONMENT,VITE_SENTRY_TRACES_SAMPLE_RATEandVITE_SENTRY_REPLAYretire alongsideVITE_SENTRY_DSN.replaysSessionSampleRatestays hard-coded to0and is deliberately not authorable: whole-session replay is a strictly larger surface than error-session replay and nothing pulls it.Ordering, and the ratchet
initSentry()stays sequenced afterinitRuntimeConfig()inapps/console/src/main.tsx, and the ordering is now more load-bearing, not less: the DSN itself is server-pushed, so a call at module-eval time freezes "no sink" for the session and turns the operator's only switch into a permanent removal. The comment there says so.committed-telemetry-endpoint.test.tskeeps its rules unchanged — its job ("nothing endpoint-shaped is committed to this repo") is unchanged, and a committed DSN is still inlined into the published bundle. Its rules key on the variable's suffix and on the value, never on theVITE_prefix, so they already covered the runtime-side spelling; two counter-probe assertions now pin that, so a later tidy-up cannot narrow the rules to the retired names and reopen the hole under a new one. Prose updated to describe the new recipe.Docs
apps/console/docs/error-tracking.md(rewritten last by #6601) is updated to describe only the final shape.Its CSP section is kept accurate and nothing CSP-related was added to code, per the PM's premise correction on the card: the Console ships no CSP —
apps/console/index.htmlsets none and the repo defines no default policy — so there is no in-repo CSP obstacle. The section remains the hosting-layer note, with one addition earned by this change: a runtime-served DSN can be repointed at a different ingest origin with no Console rebuild, so a hosting CSP that was correct before can start dropping events with nothing else having moved.Breaking
VITE_SENTRY_DSNin the Console build environmentOS_TELEMETRY_CLIENT_ERROR_REPORTING_DSNon the ObjectStack runtimeVITE_SENTRY_SEND_DEFAULT_PII=trueOS_TELEMETRY_CLIENT_ERROR_REPORTING_SEND_DEFAULT_PII=trueVITE_SENTRY_ENVIRONMENT/VITE_SENTRY_TRACES_SAMPLE_RATEOS_TELEMETRY_CLIENT_ERROR_REPORTING_*variablesVITE_SENTRY_REPLAY=trueOS_TELEMETRY_CLIENT_ERROR_REPORTING_REPLAY_SAMPLE_RATE=0.1VITE_SENTRY_ENABLED=falseisClientErrorReportingAllowed(): booleangetClientErrorReporting(): RuntimeClientErrorReporting or nullresolveSentryGate(env, runtimeAllows)resolveSentryGate(runtimeErrorReporting)RuntimeTelemetry.allowClientErrorReportingRuntimeTelemetry.errorReporting?Changeset declares
minoron@object-ui/app-shelland@object-ui/console— per this repo's rule that breaking changes ship asminorwith the semantics spelled out in the body, never asmajor.Compatibility — any landing order is safe
telemetry.allowClientErrorReporting, its=== truetest denies ⇒ off.{ telemetry: { allowClientErrorReporting: true } }, and assertsnull.Neither half can turn reporting on by itself, so the two PRs can land in either order with no window in which anything sends unexpectedly.
Validation
All of the following ran on
0657139(this branch's head, working tree clean), exit codes captured before any pipe.pnpm exec vitest run packages/app-shell/src/runtime-config.test.ts packages/app-shell/src/observability/sentry.test.ts packages/app-shell/src/observability/committed-telemetry-endpoint.test.ts— 3 files, 55 tests passed. Run from the repo root: the repo's own guard rejected thepnpm --filterform, which would have reported22 passedfrom a different package entirely.pnpm exec vitest run packages/app-shell/— 558 files, 5400 passed, 1 skipped.pnpm --filter @object-ui/app-shell run type-check(tsc --noEmit && tsc -p tsconfig.test.json) — clean. Verified with--listFilesthat all 3 edited test files are inside that program, so "type-check clean" actually covers them.pnpm --filter @object-ui/console run type-check— clean (after building the console dependency closure; the first run'sCannot find module '@object-ui/plugin-*'errors were unbuilt packages, not verdicts).package.jsonand.github/workflows/:check-control-bytes,check-changeset-presence,check-changeset-no-major,check:doc-fences,check:doc-snippets,check:doc-types,check:readme-exports,check:vi-mock-specifiers,check:self-import,check:phantom-deps,check:shell-escape-residue,check:spec-symbols— all green.no-explicit-anyspellings — the per-file count is flat or lower than the merge base (sentry.test.tswent 2 to 0), andeslint .carries no--max-warnings. The narrowing is a measurement, not a skip: this repo's ESLint config sets noparserOptions.projectand no typed rules, so no untouched file's verdict can move because of this diff.Reverse-verified — both new pins were shown able to fail, each mutation confirmed on disk by before/after
grep -con the anchored text plus agit hash-objectdifference, and each restore proved by an emptygit diff HEADand a blob hash matchingHEAD:=== trueto!!) ⇒ 1 failed / 17 passed.Declared NOT MEASURED
pnpm testandpnpm lintwere not run; CI runs both. The affected package's full suite and the derived gate families above are the local half.Generated by Claude Code
Generated by Claude Code