From 9f2c4cbe4f7ed6765016176058ea1973962a096e Mon Sep 17 00:00:00 2001 From: Sakariyah Abdulhazeem Date: Wed, 30 Sep 2026 12:29:21 +0100 Subject: [PATCH] fix(frontend): surface service worker registration failures (#285) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The registration path hid failures from developers and monitoring. Report them properly while keeping the app working without a service worker: - failures are logged with `console.warn` and sent through a new `reportError` seam (`app/lib/monitoring.ts`) that forwards to a monitoring service when one is attached to `globalThis` (Sentry-compatible or `window.__AIRFLEX_MONITOR__`), and is a safe no-op otherwise; - a development-mode warning explains *why* registration was skipped (production-only; next-pwa emits `/sw.js` only in production builds); - an environment without `navigator.serviceWorker` warns and degrades gracefully instead of throwing. Note: the catch was not literally empty on `main` (it logged via `console.error`); this implements the issue's asks — `console.warn`, monitoring forwarding, and a dev-mode explanation. Closes #285 --- frontend/app/lib/monitoring.ts | 67 ++++++++++++++ .../components/ServiceWorkerRegister.test.tsx | 91 +++++++++++++++++++ frontend/components/ServiceWorkerRegister.tsx | 29 ++++-- 3 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 frontend/app/lib/monitoring.ts create mode 100644 frontend/components/ServiceWorkerRegister.test.tsx diff --git a/frontend/app/lib/monitoring.ts b/frontend/app/lib/monitoring.ts new file mode 100644 index 0000000..7d82ace --- /dev/null +++ b/frontend/app/lib/monitoring.ts @@ -0,0 +1,67 @@ +/** + * Minimal monitoring seam (issue #285). + * + * There is no first-party monitoring module in the frontend yet, so this file + * provides a single, no-throw entry point for reporting errors. It always logs + * locally, and additionally forwards to whatever monitoring service happens to + * be attached to the global scope (e.g. a Sentry-compatible `Sentry` object or + * a custom `window.__AIRFLEX_MONITOR__`). + * + * Everything here is SSR-safe: it must never touch `window`/`navigator` + * directly and must never throw, even when no monitoring service is present. + */ + +export type MonitoringContext = Record; + +interface MonitoringService { + captureException?: (error: unknown, context?: MonitoringContext) => void; +} + +type MonitoringGlobal = typeof globalThis & { + __AIRFLEX_MONITOR__?: MonitoringService; + Sentry?: MonitoringService; +}; + +/** + * Resolve the monitoring service, if one has been installed on the global + * scope. Returns `undefined` when running on the server or when nothing is + * available. + */ +function getMonitoringService(): MonitoringService | undefined { + if (typeof globalThis === "undefined") { + return undefined; + } + + const root = globalThis as MonitoringGlobal; + + if (typeof root.__AIRFLEX_MONITOR__?.captureException === "function") { + return root.__AIRFLEX_MONITOR__; + } + + if (typeof root.Sentry?.captureException === "function") { + return root.Sentry; + } + + return undefined; +} + +/** + * Report an error to the console and, when available, to the monitoring + * service. Guaranteed not to throw. + * + * @param error - The error (or thrown value) to report. + * @param context - Optional structured context (e.g. `{ source: "..." }`). + */ +export function reportError(error: unknown, context?: MonitoringContext): void { + console.warn("[monitoring] captured error:", error, context ?? ""); + + try { + const service = getMonitoringService(); + service?.captureException?.(error, context); + } catch (monitoringError) { + console.error( + "[monitoring] failed to forward error to monitoring service:", + monitoringError + ); + } +} diff --git a/frontend/components/ServiceWorkerRegister.test.tsx b/frontend/components/ServiceWorkerRegister.test.tsx new file mode 100644 index 0000000..13f1417 --- /dev/null +++ b/frontend/components/ServiceWorkerRegister.test.tsx @@ -0,0 +1,91 @@ +import React from "react"; +import { render, waitFor } from "@testing-library/react"; +import ServiceWorkerRegister from "./ServiceWorkerRegister"; +import { reportError } from "@/app/lib/monitoring"; + +jest.mock("@/app/lib/monitoring", () => ({ + reportError: jest.fn(), +})); + +const mockedReportError = reportError as jest.MockedFunction; + +const originalNodeEnv = process.env.NODE_ENV; + +function setServiceWorker(value: unknown) { + Object.defineProperty(navigator, "serviceWorker", { + value, + configurable: true, + }); +} + +function removeServiceWorker() { + delete (navigator as { serviceWorker?: unknown }).serviceWorker; +} + +describe("ServiceWorkerRegister", () => { + let warnSpy: jest.SpyInstance; + + beforeEach(() => { + jest.clearAllMocks(); + warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {}); + }); + + afterEach(() => { + warnSpy.mockRestore(); + process.env.NODE_ENV = originalNodeEnv; + removeServiceWorker(); + }); + + it("does not register and does not throw when navigator.serviceWorker is absent", () => { + removeServiceWorker(); + + expect(() => render()).not.toThrow(); + expect(mockedReportError).not.toHaveBeenCalled(); + }); + + it("warns and reports to monitoring when registration rejects", async () => { + process.env.NODE_ENV = "production"; + const error = new Error("boom"); + const register = jest.fn().mockRejectedValue(error); + setServiceWorker({ register }); + + expect(() => render()).not.toThrow(); + + await waitFor(() => + expect(register).toHaveBeenCalledWith("/sw.js", { scope: "/" }) + ); + await waitFor(() => + expect(mockedReportError).toHaveBeenCalledWith(error, { + source: "service-worker", + }) + ); + expect(warnSpy).toHaveBeenCalledWith( + "[pwa] Service worker registration failed:", + error + ); + }); + + it("does not warn when registration resolves", async () => { + process.env.NODE_ENV = "production"; + const register = jest.fn().mockResolvedValue({}); + setServiceWorker({ register }); + + render(); + + await waitFor(() => expect(register).toHaveBeenCalled()); + expect(mockedReportError).not.toHaveBeenCalled(); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it("logs a development explanation when registration is skipped", () => { + process.env.NODE_ENV = "development"; + setServiceWorker({ register: jest.fn() }); + + render(); + + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining("production-only feature") + ); + expect(mockedReportError).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/components/ServiceWorkerRegister.tsx b/frontend/components/ServiceWorkerRegister.tsx index 06e4f4a..e93af49 100644 --- a/frontend/components/ServiceWorkerRegister.tsx +++ b/frontend/components/ServiceWorkerRegister.tsx @@ -1,6 +1,7 @@ "use client"; import { useEffect } from "react"; +import { reportError } from "@/app/lib/monitoring"; /** * Registers the PWA service worker (issue #107). @@ -14,20 +15,36 @@ import { useEffect } from "react"; */ export default function ServiceWorkerRegister() { useEffect(() => { - if ( - typeof window === "undefined" || - !("serviceWorker" in navigator) || - process.env.NODE_ENV !== "production" - ) { + if (typeof window === "undefined") { return; } // next-pwa disables itself in development (`disable` flag in // next.config.js), so /sw.js only exists in production builds. + if (process.env.NODE_ENV !== "production") { + console.warn( + "[pwa] Service worker registration skipped: service worker " + + "registration is a production-only feature; next-pwa disables it " + + "in development, so /sw.js does not exist." + ); + return; + } + + // Graceful degradation: without this API the app still works, just + // without offline/PWA support. + if (!("serviceWorker" in navigator)) { + console.warn( + "[pwa] Service worker registration skipped: navigator.serviceWorker " + + "is unavailable in this environment." + ); + return; + } + navigator.serviceWorker .register("/sw.js", { scope: "/" }) .catch((err) => { - console.error("[pwa] Service worker registration failed:", err); + console.warn("[pwa] Service worker registration failed:", err); + reportError(err, { source: "service-worker" }); }); }, []);