From 01112c350ce985bd16bace6058fc5445934273b2 Mon Sep 17 00:00:00 2001 From: Devin Foley Date: Wed, 12 Aug 2026 11:23:18 -0700 Subject: [PATCH] fix(ui): always give respondWith a real Response in the sw fetch fallback (#11292) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The UI registers a service worker (`ui/public/sw.js`) with a network-first fetch handler whose cache is an offline fallback > - The fallback hands `event.respondWith` the result of `caches.match(...)`, which resolves `undefined` on a cache miss — and in the navigation branch, `caches.match("/") || offlineResponse` never uses the fallback because `caches.match` returns a promise, which is always truthy > - When the network fetch rejects (server restart, deploy, brief outage) and the cache misses, the browser fails the request with `Uncaught (in promise) TypeError: Failed to convert value to 'Response'`, so navigation breaks outright instead of degrading to the offline page > - This pull request awaits the cache lookups and guarantees a real `Response` on every path > - The benefit is that brief server unavailability degrades to the offline fallback instead of a dead navigation ## Linked Issues or Issue Description No existing issue. Description follows the bug template: **What happened?** Navigating while the server was briefly unavailable (mid-restart) produced `The FetchEvent for "…" resulted in a network error response: the promise was rejected.` and `sw.js:1 Uncaught (in promise) TypeError: Failed to convert value to 'Response'.` The navigation failed instead of showing the offline fallback. **Expected behavior** A failed navigation serves the cached app shell when present, otherwise the "Offline" 503 response. A failed asset fetch serves its cache entry when present, otherwise a proper network-error response. `respondWith` always receives a real `Response`. **Steps to reproduce** 1. Load the app so `sw.js` is active; ensure `/` is not in the service worker cache (fresh cache version). 2. Restart or stop the backend. 3. Navigate to any page: the fetch rejects, `caches.match` misses, and the browser logs the conversion TypeError with a failed navigation. ## What Changed - `ui/public/sw.js`: the fetch fallback awaits `caches.match(...)` and returns the "Offline" 503 for navigations and `Response.error()` for assets when the cache misses. - `ui/src/lib/sw-offline-fallback.test.ts`: evaluates the real `sw.js` in a sandboxed scope and covers the three fallback paths; the two miss-path tests fail against the previous code. ## Verification - `pnpm vitest run src/lib/sw-offline-fallback.test.ts` in `ui/` — 3 tests pass. - Verified both miss-path tests fail against the unmodified `sw.js`. ## Risks Low risk. The change only affects the fetch-rejection path; successful fetches and cache hits behave exactly as before. `Response.error()` mirrors what the browser would produce for an unhandled failed no-cors fetch. ## Model Used - Claude (Anthropic), Claude Fable 5 (`claude-fable-5`), via Claude Code CLI with tool use (code search, edit, test execution). ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --- ui/public/sw.js | 11 ++- ui/src/lib/sw-offline-fallback.test.ts | 110 +++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 ui/src/lib/sw-offline-fallback.test.ts diff --git a/ui/public/sw.js b/ui/public/sw.js index f90d12156f..e5997304dc 100644 --- a/ui/public/sw.js +++ b/ui/public/sw.js @@ -32,11 +32,16 @@ self.addEventListener("fetch", (event) => { } return response; }) - .catch(() => { + .catch(async () => { + // caches.match() resolves undefined on a miss (and the promise itself + // is always truthy, so `||` can never supply a fallback). respondWith + // must always receive a real Response — resolving undefined breaks + // the navigation with "Failed to convert value to 'Response'" instead + // of showing anything. if (request.mode === "navigate") { - return caches.match("/") || new Response("Offline", { status: 503 }); + return (await caches.match("/")) ?? new Response("Offline", { status: 503 }); } - return caches.match(request); + return (await caches.match(request)) ?? Response.error(); }) ); }); diff --git a/ui/src/lib/sw-offline-fallback.test.ts b/ui/src/lib/sw-offline-fallback.test.ts new file mode 100644 index 0000000000..ab2b86ca09 --- /dev/null +++ b/ui/src/lib/sw-offline-fallback.test.ts @@ -0,0 +1,110 @@ +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it, vi } from "vitest"; + +const uiRoot = resolve(fileURLToPath(new URL("../..", import.meta.url))); + +type FetchListener = (event: { + request: { method: string; url: string; mode: string }; + respondWith: (response: Promise | Response) => void; +}) => void; + +function loadServiceWorkerFetchListener(overrides: { + fetch: () => Promise; + cachesMatch: (key: unknown) => Promise; +}): FetchListener { + const listeners = new Map void>(); + const swSelf = { + addEventListener: (type: string, listener: (event: unknown) => void) => { + listeners.set(type, listener); + }, + skipWaiting: vi.fn(), + clients: { claim: vi.fn() }, + location: { origin: "https://app.example.com" }, + }; + const caches = { + match: overrides.cachesMatch, + open: vi.fn(async () => ({ put: vi.fn() })), + keys: vi.fn(async () => []), + delete: vi.fn(async () => true), + }; + const code = readFileSync(resolve(uiRoot, "public/sw.js"), "utf8"); + new Function("self", "caches", "fetch", "Response", "URL", code)( + swSelf, + caches, + overrides.fetch, + Response, + URL, + ); + const listener = listeners.get("fetch"); + if (!listener) throw new Error("sw.js registered no fetch listener"); + return listener as FetchListener; +} + +async function respondTo( + listener: FetchListener, + request: { method: string; url: string; mode: string }, +): Promise { + let captured: Promise | Response | undefined; + listener({ + request, + respondWith: (response) => { + captured = response; + }, + }); + return await captured; +} + +describe("sw.js offline fallback", () => { + it("serves the Offline response for a failed navigation with an empty cache", async () => { + const listener = loadServiceWorkerFetchListener({ + fetch: () => Promise.reject(new TypeError("network down")), + cachesMatch: async () => undefined, + }); + + const response = await respondTo(listener, { + method: "GET", + url: "https://app.example.com/settings/instance", + mode: "navigate", + }); + + // respondWith must always receive a real Response; resolving undefined + // fails the navigation with "Failed to convert value to 'Response'". + expect(response).toBeInstanceOf(Response); + expect(response!.status).toBe(503); + expect(await response!.text()).toBe("Offline"); + }); + + it("serves the cached shell for a failed navigation when one exists", async () => { + const shell = new Response("app shell", { status: 200 }); + const listener = loadServiceWorkerFetchListener({ + fetch: () => Promise.reject(new TypeError("network down")), + cachesMatch: async (key) => (key === "/" ? shell : undefined), + }); + + const response = await respondTo(listener, { + method: "GET", + url: "https://app.example.com/settings/instance", + mode: "navigate", + }); + + expect(response).toBe(shell); + }); + + it("returns a network-error Response for a failed asset with no cache entry", async () => { + const listener = loadServiceWorkerFetchListener({ + fetch: () => Promise.reject(new TypeError("network down")), + cachesMatch: async () => undefined, + }); + + const response = await respondTo(listener, { + method: "GET", + url: "https://app.example.com/assets/index-abc.js", + mode: "no-cors", + }); + + expect(response).toBeInstanceOf(Response); + expect(response!.type).toBe("error"); + }); +});