From 8a733be369416e9f3595f3e66e702bf1ffb82aea Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Thu, 8 Oct 2026 20:16:41 +0200 Subject: [PATCH] Push receiver: show() never rejects; diagnostics describe each displayed notification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first worker trace from the iPhone read "push · notification · session_watch · added · display failed": the worker ran and wrote the entry, but both showNotification calls threw — on iOS the platform shows the proposed notification itself inside a declarative push event. In the old code show()'s rejection made the event's waitUntil reject at once, with the history write still in flight; that is the plausible cause of the entries lost today. show() now resolves to "shown", "fallback" or "failed: " and never rejects (regression test with a throwing showNotification). The same trace showed all six displayed notifications as "not ours": getNotifications() on iOS hands them back without a data object the entry rule recognises, so the on-open import cannot identify them. The Diagnostics list now shows tag, data shape and timestamp per notification so an import without data can be designed from evidence; a data member that arrives as a JSON string is read as JSON. Co-Authored-By: Claude Fable 5.1 --- internal | 2 +- .../app/push/status/page.tsx | 9 ++++++-- .../lib/__tests__/pushCore.test.ts | 13 ++++++++++++ .../lib/__tests__/pushFormat.test.ts | 13 +++++++++++- .../lib/__tests__/pushSw.test.ts | 17 +++++++++++++++ .../lib/push/pushClient.ts | 14 +++++++++++-- .../lib/push/pushFormat.ts | 13 ++++++++++++ .../public/push/sw-core.js | 11 +++++++++- .../autopilot-monitor-web/public/push/sw.js | 21 +++++++++++++------ 9 files changed, 100 insertions(+), 13 deletions(-) diff --git a/internal b/internal index be4774be..d6ca78f6 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit be4774bec43bbd3d07bba22c0d46177fde8fedf3 +Subproject commit d6ca78f628e4cd61b0912199ebfb1dd1e920ff6c diff --git a/src/Web/autopilot-monitor-web/app/push/status/page.tsx b/src/Web/autopilot-monitor-web/app/push/status/page.tsx index dba8e3ec..8f2840d8 100644 --- a/src/Web/autopilot-monitor-web/app/push/status/page.tsx +++ b/src/Web/autopilot-monitor-web/app/push/status/page.tsx @@ -268,9 +268,14 @@ export default function PushStatusPage() { {diag && diag.displayed && diag.displayed.length > 0 && (
    {diag.displayed.map((n, i) => ( -
  • +
  • {n.title} - {n.id ? "" : " (not ours)"} + {" · "} + {n.tag ? `tag ${n.tag}` : "no tag"} + {" · "} + {n.data} + {n.timestamp ? ` · ${formatDateTime(n.timestamp)}` : ""} + {n.id ? " · ours" : " · not recognised"}
    • ))}
diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/pushCore.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/pushCore.test.ts index d5b4aadd..684ddaae 100644 --- a/src/Web/autopilot-monitor-web/lib/__tests__/pushCore.test.ts +++ b/src/Web/autopilot-monitor-web/lib/__tests__/pushCore.test.ts @@ -477,3 +477,16 @@ describe("worker trace (appendTraceRecord / parseTrace)", () => { expect(parseTrace("\"a string\"")).toEqual([]); }); }); + +describe("entryFromNotification with a JSON-string data member", () => { + it("reads the string as JSON and treats unreadable or non-object text as foreign", () => { + const AT = Date.UTC(2026, 9, 8, 18, 9, 27); + const entry = entryFromNotification( + { title: "Enrollment finished", body: "b", tag: "session_watch", data: JSON.stringify({ id: "srv-7", type: "session_watch", ts: "2026-10-08T18:09:24Z" }) }, + AT, + )!; + expect(entry).toMatchObject({ id: "srv-7", type: "session_watch", title: "Enrollment finished", tag: "session_watch", ts: "2026-10-08T18:09:24.000Z" }); + expect(entryFromNotification({ title: "x", body: "", data: "{not json" }, AT)).toBeNull(); + expect(entryFromNotification({ title: "x", body: "", data: "\"plain\"" }, AT)).toBeNull(); + }); +}); diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/pushFormat.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/pushFormat.test.ts index 9cb3bce3..6f600298 100644 --- a/src/Web/autopilot-monitor-web/lib/__tests__/pushFormat.test.ts +++ b/src/Web/autopilot-monitor-web/lib/__tests__/pushFormat.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "vitest"; -import { formatDateTime, formatRelativeTime, formatTraceRecord } from "../push/pushFormat"; +import { describeNotificationData, formatDateTime, formatRelativeTime, formatTraceRecord } from "../push/pushFormat"; const NOW = Date.parse("2026-10-07T12:00:00Z"); @@ -32,3 +32,14 @@ describe("formatTraceRecord", () => { expect(formatTraceRecord({ at: "not a date", event: "activate" })).toBe("— · activate"); }); }); + +describe("describeNotificationData", () => { + it("names the shape the import can or cannot use", () => { + expect(describeNotificationData(undefined)).toBe("no data"); + expect(describeNotificationData(null)).toBe("data null"); + expect(describeNotificationData('{"id":"x"}')).toBe('data string(10) {"id":"x"}'); + expect(describeNotificationData({ id: "x", entry: {} })).toBe("data object{id,entry}"); + expect(describeNotificationData([1])).toBe("data array(1)"); + expect(describeNotificationData(7)).toBe("data number"); + }); +}); diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/pushSw.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/pushSw.test.ts index 5a822e35..d6fd3340 100644 --- a/src/Web/autopilot-monitor-web/lib/__tests__/pushSw.test.ts +++ b/src/Web/autopilot-monitor-web/lib/__tests__/pushSw.test.ts @@ -175,6 +175,23 @@ describe("push service worker glue (K25)", () => { expect(env.sequence).toEqual(["showNotification", "indexedDB.open", "indexedDB.open"]); }); + it("settles waitUntil and still writes the history when showNotification throws (iOS inside a declarative push event)", async () => { + const env = await loadWorker(erroringIndexedDb); + env.showNotification.mockImplementation(async () => { + env.sequence.push("showNotification"); + throw new Error("NotAllowedError"); + }); + const event = fakeEvent({ data: null, notification: { title: "Enrollment finished", body: "b", data: { id: "srv-9", type: "session_watch" } } }); + + env.handlers.get("push")!(event); + + // Resolves, never rejects: a rejected waitUntil would end the event with the write in flight. + await expect(event.waited).resolves.toBeDefined(); + // Both attempts (the entry, then the generic fallback) threw; the history write started regardless. + expect(env.showNotification).toHaveBeenCalledTimes(2); + expect(env.sequence.slice(0, 2)).toEqual(["showNotification", "indexedDB.open"]); + }); + it("shows a readable payload with its own title and still never waits for IndexedDB", async () => { const env = await loadWorker(hangingIndexedDb); const payload = { notification: { title: "Enrollment failed", body: "DESKTOP-1234", data: { id: "abc", type: "enrollment_failed" } } }; diff --git a/src/Web/autopilot-monitor-web/lib/push/pushClient.ts b/src/Web/autopilot-monitor-web/lib/push/pushClient.ts index 142aa66a..6914d229 100644 --- a/src/Web/autopilot-monitor-web/lib/push/pushClient.ts +++ b/src/Web/autopilot-monitor-web/lib/push/pushClient.ts @@ -27,6 +27,7 @@ import { updateDeviceSubscription, type PushDeviceResponse, } from "./pushApi"; +import { describeNotificationData } from "./pushFormat"; import { decideReconcileAction } from "./pushReconcile"; export const SW_URL = "/push/sw.js"; @@ -162,9 +163,14 @@ export async function importDisplayedNotifications(): Promise { } export interface DisplayedNotification { - /** The history id behind the notification, or null when it is not one of ours. */ + /** The history id behind the notification, or null when the entry rule does not recognise it. */ id: string | null; title: string; + tag: string | null; + /** The shape of the Notification's data member (describeNotificationData) — what an import could work with. */ + data: string; + /** The Notification's own timestamp as ISO, when the browser exposes one. */ + timestamp: string | null; } /** What the status page shows under Diagnostics: the local facts behind a push that did or did not reach the history. */ @@ -197,7 +203,11 @@ export async function collectPushDiagnostics(): Promise { if (registration && typeof registration.getNotifications === "function") { try { const now = Date.now(); - displayed = (await registration.getNotifications()).map((n) => ({ id: entryFromNotification(n, now)?.id ?? null, title: n.title })); + displayed = (await registration.getNotifications()).map((n) => { + const stamped = n as Notification & { timestamp?: unknown }; + const timestamp = typeof stamped.timestamp === "number" && Number.isFinite(stamped.timestamp) ? new Date(stamped.timestamp).toISOString() : null; + return { id: entryFromNotification(n, now)?.id ?? null, title: n.title, tag: n.tag || null, data: describeNotificationData(n.data), timestamp }; + }); } catch { displayed = null; } diff --git a/src/Web/autopilot-monitor-web/lib/push/pushFormat.ts b/src/Web/autopilot-monitor-web/lib/push/pushFormat.ts index c6403efb..c6e96eaa 100644 --- a/src/Web/autopilot-monitor-web/lib/push/pushFormat.ts +++ b/src/Web/autopilot-monitor-web/lib/push/pushFormat.ts @@ -45,3 +45,16 @@ export function formatTraceRecord(record: TraceRecord): string { if (record.id) parts.push(`#${record.id.slice(0, 8)}`); return parts.join(" · "); } + +/** A Notification's data member in one short phrase: its type and, for an object, its keys — the import needs an object with an id. */ +export function describeNotificationData(value: unknown): string { + if (value === undefined) return "no data"; + if (value === null) return "data null"; + if (typeof value === "string") return `data string(${value.length}) ${value.slice(0, 40)}`.trimEnd(); + if (Array.isArray(value)) return `data array(${value.length})`; + if (typeof value === "object") { + const keys = Object.keys(value as Record); + return `data object{${keys.slice(0, 6).join(",")}${keys.length > 6 ? ",…" : ""}}`; + } + return `data ${typeof value}`; +} diff --git a/src/Web/autopilot-monitor-web/public/push/sw-core.js b/src/Web/autopilot-monitor-web/public/push/sw-core.js index ac1223ec..7aa9891f 100644 --- a/src/Web/autopilot-monitor-web/public/push/sw-core.js +++ b/src/Web/autopilot-monitor-web/public/push/sw-core.js @@ -295,7 +295,16 @@ export function normalizePayload(input, nowMs) { * @returns {HistoryEntry | null} null when nothing identifies the notification as ours */ export function entryFromNotification(notification, nowMs) { - const data = isRecord(notification.data) ? notification.data : null; + let raw = notification.data; + if (typeof raw === "string") { + // A data member handed back as its JSON text is read as JSON; unreadable text is foreign. + try { + raw = JSON.parse(raw); + } catch { + raw = null; + } + } + const data = isRecord(raw) ? raw : null; if (data && isRecord(data.entry) && typeof data.entry.id === "string") return /** @type {HistoryEntry} */ (data.entry); if (!data || typeof data.id !== "string") return null; return normalizePayload({ title: notification.title, body: notification.body, tag: notification.tag, data }, nowMs); diff --git a/src/Web/autopilot-monitor-web/public/push/sw.js b/src/Web/autopilot-monitor-web/public/push/sw.js index 6340afbb..ec0eba93 100644 --- a/src/Web/autopilot-monitor-web/public/push/sw.js +++ b/src/Web/autopilot-monitor-web/public/push/sw.js @@ -7,7 +7,10 @@ * Contract (K25): display never waits for IndexedDB — `waitUntil(Promise.all([show, persist]))` * with persist failures swallowed and persist bounded in time; a payload that cannot be read * still shows the generic title, because a silent push costs the subscription on iOS after - * three occurrences. lib/__tests__/pushSw.test.ts pins the order and the settling. + * three occurrences. On iOS the worker's showNotification throws inside a declarative push + * event and the platform shows the proposed notification itself (worker trace, 2026-10-08), so + * show() never rejects: a rejected waitUntil ended the event with the history write still in + * flight, and entries were lost. lib/__tests__/pushSw.test.ts pins the order and the settling. */ import { base64UrlToUint8Array, @@ -75,15 +78,21 @@ async function notifyWindows(type, detail = {}) { /** * @param {import("./sw-core.js").HistoryEntry} entry - * @returns {Promise<"shown" | "fallback">} how the notification went up, for the trace + * @returns {Promise} "shown", "fallback" or "failed: " for the trace — never rejects */ async function show(entry) { try { await self.registration.showNotification(entry.title, toNotificationOptions(entry)); return "shown"; - } catch { - await self.registration.showNotification(GENERIC_TITLE, { body: GENERIC_BODY }); - return "fallback"; + } catch (error) { + try { + await self.registration.showNotification(GENERIC_TITLE, { body: GENERIC_BODY }); + return "fallback"; + } catch (second) { + const first = describeError(error); + const again = describeError(second); + return "failed: " + (again === first ? first : first + " / " + again); + } } } @@ -122,7 +131,7 @@ function describeError(error) { * never answers, the missing record is the evidence the status page shows. * @param {import("./sw-core.js").HistoryEntry} entry * @param {Record} fields the record's fields besides id, type, result and shown - * @param {Promise} [shown] the display outcome — already in flight, awaited only for the record + * @param {Promise} [shown] the display outcome — already in flight and never rejecting, awaited only for the record * @returns {Promise} */ function persist(entry, fields, shown) {