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;