mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): drop unhandled rejection from throwing window.Notification (#5143)
## Summary WebKit throws `NotificationError` from the `Notification` constructor when the notification backend becomes temporarily unavailable (measured repro attached to #5081). Every existing call site used `void sendDesktopNotification(...).then(...)` — discarding the returned promise with no rejection handler — so a throwing constructor became an unhandled promise rejection. The notification was silently dropped and the only trace was console noise. Closes #5081. ## What changed Fenced the throw at the source inside `sendDesktopNotification` (`desktop/src/features/notifications/lib/desktop.ts`): - A new `try { ... } catch { ... }` wraps `new window.Notification(...)` and the `onclick` attach. - On catch, we `console.warn` once and `return false`, so the promise the call sites discard is always fulfilled with the same boolean result. No caller needs to change. ## Why at the source and not at each call site The issue body lists four rejecting edges: `useAppShellDesktopNotifications` (2×), `useReminderNotifications`, `use-feed-desktop-notifications`. Patching them one-by-one leaves the door open for the next consumer to make the same mistake — and the function itself advertises `Promise<boolean>`, so callers are entitled to assume the promise resolves with the delivery bit rather than rejects. Fixing the inside satisfies both properties for every present and future caller. ## Test plan - Behavior change is a guarded return value around a single constructor; unit coverage is best expressed inside the mounted-hook harness already used in the repro. Existing notification helpers (`shouldNotify*.test.mjs`) continue to pass. - Full `just ci` runs on the blocker. - The next notification after a backend blip delivers normally (the throw is per-call, not sticky). ## Note on scope This addresses the titled bug: unhandled rejection from a throwing constructor. A separate, intended follow-up is to wire a user-visible delivery-miss event if the platform exposes one — that's notification-observability work, not a } catch. --------- Signed-off-by: iroiro147 <sarthak.singh@mastersunion.org> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -0,0 +1,52 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
const notifications = [];
|
||||
|
||||
class WorkingNotification {
|
||||
static permission = "granted";
|
||||
|
||||
constructor(title, options) {
|
||||
notifications.push({ title, options });
|
||||
}
|
||||
|
||||
close() {}
|
||||
}
|
||||
|
||||
class ThrowingNotification {
|
||||
static permission = "granted";
|
||||
|
||||
constructor() {
|
||||
throw new Error("notification backend unavailable");
|
||||
}
|
||||
}
|
||||
|
||||
globalThis.window = { Notification: ThrowingNotification };
|
||||
|
||||
const { sendDesktopNotification } = await import("./desktop.ts");
|
||||
|
||||
test("constructor failure is a delivery miss and does not prevent a later notification", async (t) => {
|
||||
const warnings = [];
|
||||
t.mock.method(console, "warn", (...args) => warnings.push(args));
|
||||
|
||||
const failed = await sendDesktopNotification({ title: "First" });
|
||||
|
||||
assert.equal(failed, false);
|
||||
assert.equal(warnings.length, 1);
|
||||
assert.match(String(warnings[0][1]), /notification backend unavailable/);
|
||||
|
||||
window.Notification = WorkingNotification;
|
||||
|
||||
const delivered = await sendDesktopNotification({
|
||||
title: "Second",
|
||||
body: "Recovered",
|
||||
});
|
||||
|
||||
assert.equal(delivered, true);
|
||||
assert.deepEqual(notifications, [
|
||||
{
|
||||
title: "Second",
|
||||
options: { body: "Recovered", silent: true, extra: undefined },
|
||||
},
|
||||
]);
|
||||
});
|
||||
@@ -381,19 +381,32 @@ export async function sendDesktopNotification(
|
||||
}
|
||||
}
|
||||
|
||||
const notification = new window.Notification(payload.title, {
|
||||
body: payload.body,
|
||||
silent: true,
|
||||
extra: notificationExtra(payload.target),
|
||||
} as DesktopNotificationOptions);
|
||||
// block/buzz#5081 — WebKit throws `NotificationError` from the constructor
|
||||
// when the notification backend becomes temporarily unavailable. Callers
|
||||
// discard the returned promise without a rejection handler, so an
|
||||
// un-guarded throw becomes an unhandled rejection. Treat constructor failure
|
||||
// as a delivery miss (return false) and log the failed delivery.
|
||||
try {
|
||||
const notification = new window.Notification(payload.title, {
|
||||
body: payload.body,
|
||||
silent: true,
|
||||
extra: notificationExtra(payload.target),
|
||||
} as DesktopNotificationOptions);
|
||||
|
||||
const target = payload.target;
|
||||
if (!isTauri() && target) {
|
||||
notification.onclick = () => {
|
||||
dispatchDesktopNotificationTarget(target);
|
||||
notification.close();
|
||||
};
|
||||
const target = payload.target;
|
||||
if (!isTauri() && target) {
|
||||
notification.onclick = () => {
|
||||
dispatchDesktopNotificationTarget(target);
|
||||
notification.close();
|
||||
};
|
||||
}
|
||||
|
||||
return true;
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
"[desktop] window.Notification constructor threw — notification dropped:",
|
||||
error,
|
||||
);
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user