feat(error-reporting): CrashReport panel + Sentry-compatible client#247
Conversation
Drops into the fileshare error boundary: on any uncaught render-time throw inside the encrypt flow, render a "Tell PostGuard" panel that captures the exception (with rich client context: URL, viewport, UA, locale, app version) and POSTs it to the configured GlitchTip / Sentry endpoint via @sentry/browser. Reset path wipes encryptState and resets the boundary so the user can keep going. - `src/lib/errorReporting.ts` — lazy SDK init, manual-capture only (no global error / unhandledrejection hooks, no breadcrumbs), 3 s flush budget. - `src/lib/components/filesharing/CrashReport.svelte` — two-button panel: primary "Tell PostGuard" (flips to "Sending…" → "Thank you!" in-place, width pinned), secondary "Try again". - `src/lib/env.ts` — `GLITCHTIP_DSN` runtime config flag; empty value disables reporting at module load. - `src/hooks.client.ts` — initialises the SDK on app boot. - `src/routes/(app)/fileshare/+page.svelte` — wraps the encrypt flow in `<svelte:boundary>` with CrashReport as the `failed` snippet. - Locale strings (`crash` section, en + nl). - `static/config.js` — local dev DSN matching the GlitchTip stack in docker-compose. Deployed environments override via the Terraform ConfigMap. - `docker-compose.yml` — local GlitchTip stack (postgres + redis + web + worker + idempotent init Job that seeds the admin user, the PostGuard org, the postguard-website project, and a pinned ProjectKey UUID so the DSN survives `docker compose down -v`). - `package.json` / `package-lock.json` — adds `@sentry/browser ^10` and bumps `svelte-preprocess` to `^6.0.5` (the older 6.0.3 declared `typescript@^5.0.0` only, which broke `npm ci` after the Sentry install pulled in a wider tsc range). Pairs with the postguard-ops GlitchTip deployment.
|
/dobby review |
|
Dobby has received the request! Routing to the right specialist now... |
![dobby-coder[bot]](https://avatars.githubusercontent.com/in/3365457?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code review
Adds GlitchTip-backed crash reporting behind a Svelte boundary on the fileshare page. Implementation is clean and svelte-check is green. Main concern: the GlitchTip init script's DSN-pinning branch is unreachable, so on a fresh volume the hardcoded DSN in static/config.js silently goes stale — the exact case the comment claims to prevent.
Rule compliance
WCAG 2.2 AA: the new CrashReport panel is missing a :focus / :focus-visible style (SC 2.4.7 Focus Visible) and the failed-status <p> lacks role="status" / aria-live (SC 4.1.3 Status Messages). Also flagging that the new GLITCHTIP_* env vars and dev-stack steps aren't documented in the README — informational, not a blocker.
| project=project, | ||
| defaults={'public_key': DEV_KEY}, | ||
| ) | ||
| if key_created and key.public_key != DEV_KEY: |
There was a problem hiding this comment.
[Code review] if key_created and key.public_key != DEV_KEY is unreachable: when key_created=True, get_or_create already populated public_key from defaults to DEV_KEY, so the inequality is false; when key_created=False, the first clause is false. GlitchTip auto-creates a default ProjectKey when a Project is created, so on a fresh volume get_or_create returns the auto-created key (key_created=False) and DEV_KEY is never applied — the hardcoded DSN in static/config.js silently goes stale. Replace with update_or_create, or invert to if not key_created and key.public_key != DEV_KEY.
| } | ||
| if (typeof navigator !== 'undefined') { | ||
| ctx.userAgent = navigator.userAgent | ||
| ctx.platform = navigator.platform |
There was a problem hiding this comment.
[Code review] navigator.platform is deprecated and returns an empty string in current Firefox; drop it or switch to navigator.userAgentData?.platform with a UA fallback.
| appVersion: APP_VERSION, | ||
| uiLocale: $locale, | ||
| }) | ||
| sendState = ok ? 'sent' : 'failed' |
There was a problem hiding this comment.
[Code review] If reportError throws synchronously (Sentry SDK internal error, malformed DSN), the await rejects and sendState is stuck at 'sending' forever — the primary button stays disabled with no failed-state UI. Wrap in try/catch and set sendState = 'failed' in the catch.
| attributes={ATTRIBUTES} | ||
| readonly={encryptState.encryptionState === | ||
| EncryptionState.Encrypting} | ||
| <svelte:boundary> |
There was a problem hiding this comment.
[Code review] <FileInput> sits outside the boundary, so a render-time error in FileInput escapes the boundary and crashes the page instead of showing the CrashReport panel. Move the <svelte:boundary> opening above the FileInput so the safety net covers the whole flow.
| transform: none; | ||
| } | ||
|
|
||
| .crash-btn:not(:disabled):hover { |
There was a problem hiding this comment.
[Rule: wcag-aa-compliance] No :focus / :focus-visible rule on .crash-btn — keyboard users get no visible focus indicator (WCAG 2.4.7 Focus Visible, Level AA). Add a :focus-visible style with a contrasting outline or border.
| <p class="crash-message">{$_('filesharing.crash.message')}</p> | ||
|
|
||
| {#if sendState === 'failed'} | ||
| <p class="crash-status error"> |
There was a problem hiding this comment.
[Rule: wcag-aa-compliance] The failed-state <p> appears dynamically but has no role="status" / aria-live="polite", so screen readers won't announce the failure (WCAG 4.1.3 Status Messages, Level AA). Add role="status" to the <p> (or move the live region to a stable wrapper element so it's announced when the text changes).
- docker-compose.yml glitchtip-init: invert the DEV_KEY rewrite
condition. GlitchTip auto-creates a default ProjectKey on Project
save, so get_or_create returns that auto-created key
(key_created=False) and the `defaults={'public_key': DEV_KEY}`
clause is ignored. The previous `if key_created and …` branch was
unreachable, leaving the DSN pinned UUID never applied on fresh
volumes.
- errorReporting.ts: drop `navigator.platform` (deprecated; Firefox
returns "" already).
- CrashReport.svelte:
- Wrap reportError() in try/catch so a synchronous SDK throw can't
leave the button stuck on "Sending…".
- Add `:focus-visible` outline on .crash-btn (WCAG 2.4.7).
- Move the failed-state <p> into a stable role=status / aria-live
live region so screen readers announce it (WCAG 4.1.3).
- fileshare/+page.svelte: move <svelte:boundary> above <FileInput>
so a render-time error in FileInput is caught by CrashReport
instead of crashing the whole page.
|
/dobby please re-review — pushed 4d52834 addressing all six prior comments. |
|
Dobby has received the request! Routing to the right specialist now... |
There was a problem hiding this comment.
Code review
All six prior findings (DEV_KEY rewrite branch, try/catch around reportError, :focus-visible, live region, boundary above FileInput, deprecated navigator.platform) verified resolved in 4d52834. svelte-check clean, no new issues introduced.
Rule compliance
No violations. WCAG 2.2 AA (focus indicator, live region, tap target, contrast via tokens) ✓; conventional-commit title ✓; no design-slop tells (uses repo tokens); svelte-preprocess bump 6.0.3 → 6.0.5 matches the dep-bump-gotchas note's "unpin once 6.0.5 ships" guidance; uses defined --pg-font-weight-{bold,medium} (not the undefined -semibold); feat PR so the tests-required-on-fixes rule doesn't apply.
* docs: document pg-js 2.0 DecryptFileResult + onDownloadProgress + cause-preserving IdentityMismatchError Sources: - encryption4all/postguard-js#86 (DecryptFileResult shape change, onDownloadProgress) - encryption4all/postguard-js#84 (IdentityMismatchError cause preservation) * docs: clarify createEnvelope tier 2 vs tier 3 upload-failure semantics Source: encryption4all/postguard-js#82 (re-throw on tier 3, console.warn on tier 2) * docs: add Runtime config section for postguard-website APP_CONFIG keys Sources: - encryption4all/postguard-website#244 (STAGING) - encryption4all/postguard-website#247 (GLITCHTIP_DSN) - encryption4all/postguard-website#255 (SITE_URL) * docs: surface private signing attributes on FriendlySender (from encryption4all/postguard-js#89) * docs: document /download trust-confirmation gate (from encryption4all/postguard-website#258) --------- Co-authored-by: dobby-yivi-agent[bot] <275734547+dobby-yivi-agent[bot]@users.noreply.github.com>
1 participant
Summary
Wraps the fileshare encrypt flow in a
<svelte:boundary>. On any uncaught render-time throw, a "Tell PostGuard" panel appears with two buttons — primary captures the exception (with rich client context: URL, viewport, user agent, locale, app version) and POSTs it to GlitchTip via@sentry/browser; secondary resets the state and re-renders the form.src/lib/errorReporting.ts(lazy SDK init, manual-capture only — no global handlers, no breadcrumbs, 3 s flush budget).src/lib/components/filesharing/CrashReport.svelte(two-button panel; primary label flips to "Sending…" → "Thank you!" in place with a pinned width so the row doesn't reflow).src/lib/env.ts:GLITCHTIP_DSNruntime flag — empty value disables reporting at module load.src/hooks.client.ts: init on app boot.src/routes/(app)/fileshare/+page.svelte: boundary + CrashReport as thefailedsnippet.crashsection in en + nl.static/config.js: local dev DSN matching the docker-compose stack. Deployed environments override via the Terraform ConfigMap (see the matching postguard-ops PR).docker-compose.yml: local GlitchTip stack — postgres + redis + web + worker + idempotent init Job (seeds admin user, PostGuard org, postguard-website project, pinned ProjectKey UUID so the DSN survivesdocker compose down -v).package.json+ lockfile: adds@sentry/browser@^10, bumpssvelte-preprocessto^6.0.5(older^5.0.0-only TS peer dep was rejecting the wider Sentry-pulled range).Test plan
npx svelte-checkclean.docker compose up, init Job seeds GlitchTip with the dev admin + DSN; the website's CrashReport button delivers events to http://localhost:8001 (the dev DSN is wired instatic/config.js).*/api/*/envelope/; works in a private window.Pairs with privacybydesign/postguard-ops#54 (GlitchTip K8s stack).