From 3602b998e60b38ea1fe07a59a46eceefc3f844c7 Mon Sep 17 00:00:00 2001 From: PhilippTheServer Date: Wed, 26 Aug 2026 17:14:35 +0200 Subject: [PATCH] feat(telemetry): report uncaught errors An error that breaks checkout was visible only in the console of the person it happened to, which means nobody who could fix it ever saw it. Adds a global ErrorHandler reporting uncaught errors to the API, off by default like the analytics port. The failure mode being reported on is a component throwing inside a render loop, which calls the handler as fast as the browser can loop. So an identical error is reported at most three times per session, there is a hard per-session ceiling, reports are batched, and a failing endpoint is swallowed. A reporter that turns a render loop into a request loop takes a broken page and makes it a broken page plus a hammered API. Anything can be thrown in JavaScript and frameworks do, so nothing assumes an Error shape: strings, bare objects and null all produce a usable report rather than an exception inside the error handler. The handler still logs to the console. Swallowing that would remove what a developer looks at first in exchange for a report they cannot see locally. A reporter that throws is caught, so it can never mask the original error. Closes #4 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01YY1ekLLeFLkAU2kvdQ8Ey4 --- src/app/app.config.ts | 21 +- src/app/core/error-reporting.service.spec.ts | 210 +++++++++++++++++++ src/app/core/error-reporting.service.ts | 177 ++++++++++++++++ src/app/storefront.config.ts | 11 + 4 files changed, 418 insertions(+), 1 deletion(-) create mode 100644 src/app/core/error-reporting.service.spec.ts create mode 100644 src/app/core/error-reporting.service.ts diff --git a/src/app/app.config.ts b/src/app/app.config.ts index e790ba0..cde0d51 100644 --- a/src/app/app.config.ts +++ b/src/app/app.config.ts @@ -1,8 +1,17 @@ -import { ApplicationConfig, inject, provideAppInitializer } from '@angular/core'; +import { + ApplicationConfig, + ErrorHandler, + inject, + provideAppInitializer, +} from '@angular/core'; import { provideHttpClient, withInterceptors } from '@angular/common/http'; import { provideRouter, withInMemoryScrolling } from '@angular/router'; import { routes } from './app.routes'; import { AnalyticsService } from './core/analytics.service'; +import { + ErrorReportingService, + TelemetryErrorHandler, +} from './core/error-reporting.service'; import { AuthService } from './core/auth.service'; import { authInterceptor } from './core/auth.interceptor'; @@ -12,5 +21,15 @@ export const appConfig: ApplicationConfig = { provideRouter(routes, withInMemoryScrolling({ scrollPositionRestoration: 'enabled' })), provideAppInitializer(() => inject(AuthService).init()), provideAppInitializer(() => inject(AnalyticsService).init()), + { provide: ErrorHandler, useClass: TelemetryErrorHandler }, + provideAppInitializer(() => { + const reporter = inject(ErrorReportingService); + if (typeof document !== 'undefined') { + // A closing tab is the usual way a partial batch is lost. + document.addEventListener('visibilitychange', () => { + if (document.visibilityState === 'hidden') reporter.flushNow(); + }); + } + }), ], }; diff --git a/src/app/core/error-reporting.service.spec.ts b/src/app/core/error-reporting.service.spec.ts new file mode 100644 index 0000000..630afc4 --- /dev/null +++ b/src/app/core/error-reporting.service.spec.ts @@ -0,0 +1,210 @@ +import { TestBed } from '@angular/core/testing'; +import { Router } from '@angular/router'; + +import { + ErrorReportingService, + TelemetryErrorHandler, +} from './error-reporting.service'; +import { storefrontConfig } from '../storefront.config'; + +/** + * The failure mode being reported on is a component throwing inside a render + * loop. These pin the guards that stop a reporter turning that into a request + * loop — and that stop it making anything worse. + */ +describe('ErrorReportingService', () => { + let sent: Array<{ errors: Array> }>; + let calls: number; + let originalFetch: typeof globalThis.fetch; + + function configure(enabled: boolean) { + (storefrontConfig.errorReporting as { enabled: boolean }).enabled = enabled; + TestBed.configureTestingModule({ + providers: [ + ErrorReportingService, + { provide: Router, useValue: { url: '/checkout?token=secret' } }, + ], + }); + return TestBed.inject(ErrorReportingService); + } + + beforeEach(() => { + sent = []; + calls = 0; + originalFetch = globalThis.fetch; + globalThis.fetch = ((_url: string, init?: RequestInit) => { + calls++; + sent.push(JSON.parse(String(init?.body))); + return Promise.resolve(new Response(null, { status: 202 })); + }) as typeof globalThis.fetch; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + (storefrontConfig.errorReporting as { enabled: boolean }).enabled = false; + }); + + // ------------------------------------------------------------------------- + // Off by default + // ------------------------------------------------------------------------- + + it('is disabled in the shipped configuration', () => { + expect(storefrontConfig.errorReporting.enabled).toBe(false); + }); + + it('sends nothing when disabled', () => { + const service = configure(false); + for (let i = 0; i < 20; i++) { + service.report(new Error(`boom ${i}`)); + } + expect(calls).toBe(0); + }); + + // ------------------------------------------------------------------------- + // A render loop must not become a request loop + // ------------------------------------------------------------------------- + + it('reports an identical error at most three times per session', () => { + const service = configure(true); + + for (let i = 0; i < 500; i++) { + service.report(new TypeError('same failure every frame')); + } + service.flushNow(); + + const reported = sent.flatMap((batch) => batch.errors); + expect(reported.length).toBe(3); + }); + + it('still reports genuinely different errors', () => { + const service = configure(true); + + service.report(new TypeError('first')); + service.report(new RangeError('second')); + service.flushNow(); + + const messages = sent.flatMap((b) => b.errors).map((e) => e['message']); + expect(messages).toContain('first'); + expect(messages).toContain('second'); + }); + + it('stops entirely after a per-session ceiling', () => { + const service = configure(true); + + // 100 distinct signatures, so per-signature capping cannot be what stops it. + for (let i = 0; i < 100; i++) { + service.report(new Error(`distinct ${i}`)); + } + service.flushNow(); + + const reported = sent.flatMap((b) => b.errors); + expect(reported.length).toBeLessThanOrEqual(50); + }); + + // ------------------------------------------------------------------------- + // What gets sent + // ------------------------------------------------------------------------- + + it('strips the query string from the reported path', () => { + const service = configure(true); + service.report(new Error('boom')); + service.flushNow(); + + const report = sent[0].errors[0]; + expect(report['path']).toBe('/checkout'); + expect(JSON.stringify(sent)).not.toContain('secret'); + }); + + it('handles a thrown string without assuming an Error shape', () => { + const service = configure(true); + // JavaScript lets anything be thrown, and frameworks do. + expect(() => service.report('just a string')).not.toThrow(); + service.flushNow(); + + expect(sent[0].errors[0]['name']).toBe('UnknownError'); + expect(sent[0].errors[0]['message']).toBe('just a string'); + }); + + it('handles a thrown object with no message', () => { + const service = configure(true); + expect(() => service.report({ status: 500 })).not.toThrow(); + service.flushNow(); + + expect(sent[0].errors[0]['name']).toBe('UnknownError'); + }); + + it('handles null without throwing', () => { + const service = configure(true); + expect(() => service.report(null)).not.toThrow(); + }); + + // ------------------------------------------------------------------------- + // Never make things worse + // ------------------------------------------------------------------------- + + it('swallows a failing endpoint', () => { + const service = configure(true); + globalThis.fetch = (() => + Promise.reject(new Error('network down'))) as typeof globalThis.fetch; + + expect(() => { + service.report(new Error('boom')); + service.flushNow(); + }).not.toThrow(); + }); +}); + +describe('TelemetryErrorHandler', () => { + it('still logs to the console', () => { + // Swallowing this would remove what a developer looks at first, in exchange + // for a report they cannot see locally. + const logged: unknown[] = []; + const original = console.error; + console.error = (...args: unknown[]) => logged.push(args[0]); + + TestBed.configureTestingModule({ + providers: [ + TelemetryErrorHandler, + { provide: ErrorReportingService, useValue: { report: () => undefined } }, + ], + }); + + try { + const error = new Error('visible'); + TestBed.inject(TelemetryErrorHandler).handleError(error); + expect(logged).toContain(error); + } finally { + console.error = original; + } + }); + + it('does not let a broken reporter mask the original error', () => { + const original = console.error; + const logged: unknown[] = []; + console.error = (...args: unknown[]) => logged.push(args[0]); + + TestBed.configureTestingModule({ + providers: [ + TelemetryErrorHandler, + { + provide: ErrorReportingService, + useValue: { + report: () => { + throw new Error('reporter is broken'); + }, + }, + }, + ], + }); + + try { + const error = new Error('the real problem'); + expect(() => + TestBed.inject(TelemetryErrorHandler).handleError(error), + ).not.toThrow(); + expect(logged).toContain(error); + } finally { + console.error = original; + } + }); +}); diff --git a/src/app/core/error-reporting.service.ts b/src/app/core/error-reporting.service.ts new file mode 100644 index 0000000..317c56a --- /dev/null +++ b/src/app/core/error-reporting.service.ts @@ -0,0 +1,177 @@ +import { ErrorHandler, Injectable, inject } from '@angular/core'; +import { Router } from '@angular/router'; + +import { storefrontConfig } from '../storefront.config'; + +interface ErrorReport { + app: 'storefront'; + name: string; + message: string; + stack?: string; + path?: string; + occurred_at: string; +} + +/** Reports of the same error beyond this are dropped for the rest of the session. */ +const MAX_PER_SIGNATURE = 3; + +/** Hard ceiling per session, whatever the signatures. */ +const MAX_PER_SESSION = 50; + +/** Batched to this size before an eager flush. */ +const BATCH_SIZE = 5; + +/** Idle time before a partial batch is sent anyway, in milliseconds. */ +const FLUSH_DELAY = 2000; + +/** The API truncates too; this stops the browser sending needless kilobytes. */ +const MAX_STACK_CHARS = 4000; + +/** + * Ships uncaught errors to the API. + * + * **It must never make things worse.** The failure mode being reported on is a + * component throwing inside a render loop, which will call this as fast as the + * browser can loop. So identical errors are reported at most three times per + * session, there is a hard per-session ceiling, and a failing endpoint is + * swallowed. A reporter that turns a render loop into a request loop has taken + * a broken page and made it a broken page plus a hammered API. + * + * **Off by default**, like the analytics port. Cloning this repository must not + * start sending anything anywhere. + */ +@Injectable({ providedIn: 'root' }) +export class ErrorReportingService { + private readonly router = inject(Router); + private readonly config = storefrontConfig.errorReporting; + + private readonly seen = new Map(); + private queue: ErrorReport[] = []; + private timer: ReturnType | null = null; + private sessionTotal = 0; + + get enabled(): boolean { + return this.config.enabled; + } + + report(error: unknown): void { + if (!this.enabled || this.sessionTotal >= MAX_PER_SESSION) { + return; + } + + const { name, message, stack } = this.describe(error); + const signature = `${name}::${message}`; + + const count = (this.seen.get(signature) ?? 0) + 1; + this.seen.set(signature, count); + if (count > MAX_PER_SIGNATURE) { + return; + } + + this.sessionTotal++; + this.queue.push({ + app: 'storefront', + name, + message, + stack: stack?.slice(0, MAX_STACK_CHARS), + // Route only. The API strips query strings as well, but a token in a URL + // should not travel to a request log on the way there. + path: this.router.url.split(/[?#]/)[0], + occurred_at: new Date().toISOString(), + }); + + if (this.queue.length >= BATCH_SIZE) { + this.flush(); + return; + } + this.timer ??= setTimeout(() => this.flush(), FLUSH_DELAY); + } + + /** Anything can be thrown in JavaScript, so nothing here may assume a shape. */ + private describe(error: unknown): { name: string; message: string; stack?: string } { + if (error instanceof Error) { + return { + name: error.name || 'Error', + message: (error.message || String(error)).slice(0, 500), + stack: error.stack, + }; + } + if (typeof error === 'object' && error !== null) { + const shaped = error as { name?: unknown; message?: unknown; stack?: unknown }; + return { + name: typeof shaped.name === 'string' ? shaped.name.slice(0, 120) : 'UnknownError', + message: + typeof shaped.message === 'string' + ? shaped.message.slice(0, 500) + : safeStringify(error).slice(0, 500), + stack: typeof shaped.stack === 'string' ? shaped.stack : undefined, + }; + } + return { name: 'UnknownError', message: String(error).slice(0, 500) }; + } + + private flush(beacon = false): void { + if (this.timer) { + clearTimeout(this.timer); + this.timer = null; + } + if (this.queue.length === 0) { + return; + } + + const body = JSON.stringify({ errors: this.queue }); + this.queue = []; + + try { + if (beacon && typeof navigator !== 'undefined' && navigator.sendBeacon) { + navigator.sendBeacon( + this.config.endpoint, + new Blob([body], { type: 'application/json' }), + ); + return; + } + void fetch(this.config.endpoint, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body, + keepalive: true, + }).catch(() => undefined); + } catch { + // Silent by design. If reporting an error throws, saying so would mean + // reporting that too. + } + } + + /** Send whatever is queued before the page goes away. */ + flushNow(): void { + this.flush(true); + } +} + +function safeStringify(value: unknown): string { + try { + return JSON.stringify(value); + } catch { + return String(value); + } +} + +/** + * Angular's global error handler. + * + * Still logs to the console. Swallowing that would take away the thing a + * developer looks at first, in exchange for a report they cannot see locally. + */ +@Injectable() +export class TelemetryErrorHandler implements ErrorHandler { + private readonly reporter = inject(ErrorReportingService); + + handleError(error: unknown): void { + try { + this.reporter.report(error); + } catch { + // Never let reporting mask the original error. + } + console.error(error); + } +} diff --git a/src/app/storefront.config.ts b/src/app/storefront.config.ts index 95e4f92..bb26d2a 100644 --- a/src/app/storefront.config.ts +++ b/src/app/storefront.config.ts @@ -22,4 +22,15 @@ export const storefrontConfig = { enabled: false, endpoint: '/api/v1/analytics/events', }, + + /** + * Uncaught error reporting. + * + * Off by default, like analytics. Requires FRONTEND_ERRORS_ENABLED on the + * API, which otherwise answers the endpoint with a 404. + */ + errorReporting: { + enabled: false, + endpoint: '/api/v1/telemetry/errors', + }, } as const;