mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 04:32:32 +00:00
Push receiver: show() never rejects; diagnostics describe each displayed notification
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: <error>" 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 <noreply@anthropic.com>
This commit is contained in:
+1
-1
Submodule internal updated: be4774bec4...d6ca78f628
@@ -268,9 +268,14 @@ export default function PushStatusPage() {
|
||||
{diag && diag.displayed && diag.displayed.length > 0 && (
|
||||
<ul className="text-xs text-gray-700 space-y-0.5">
|
||||
{diag.displayed.map((n, i) => (
|
||||
<li key={i} className="truncate">
|
||||
<li key={i} className="break-words">
|
||||
{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"}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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" } } };
|
||||
|
||||
@@ -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<number> {
|
||||
}
|
||||
|
||||
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<PushDiagnostics> {
|
||||
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;
|
||||
}
|
||||
|
||||
@@ -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<string, unknown>);
|
||||
return `data object{${keys.slice(0, 6).join(",")}${keys.length > 6 ? ",…" : ""}}`;
|
||||
}
|
||||
return `data ${typeof value}`;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<string>} "shown", "fallback" or "failed: <error>" 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<string, unknown>} fields the record's fields besides id, type, result and shown
|
||||
* @param {Promise<string>} [shown] the display outcome — already in flight, awaited only for the record
|
||||
* @param {Promise<string>} [shown] the display outcome — already in flight and never rejecting, awaited only for the record
|
||||
* @returns {Promise<void>}
|
||||
*/
|
||||
function persist(entry, fields, shown) {
|
||||
|
||||
Reference in New Issue
Block a user