From 7068c67ec7c968b50ea9b13e59bf01c8876e6382 Mon Sep 17 00:00:00 2001
From: Shuki Vaknin <shukivaknin@gmail.com>
Date: Fri, 25 Sep 2026 20:15:45 +0300
Subject: [PATCH] fix(push): don't abort enabling push when the browser can't
 read its subscription

Firefox rejects pushManager.getSubscription() with
`AbortError: Error retrieving push subscription` when its local push
record is stale or the push service is unreachable. enableWebPush let
that reject the whole call, so the onboarding banner showed the browser's
bare error and push could never be turned on again - even though
subscribing fresh is exactly the recovery.

Treat a failed lookup as "no subscription to reuse" and continue to
subscribe(), whose error still surfaces when the push service is really
unavailable. Same reasoning for unsubscribe() of a mismatched
subscription: getting the new subscription is what matters.

Claude-Session: https://claude.ai/code/session_019KwXZEwxijHRbfobfXmcK2
---
 lib/__tests__/web-push.test.ts | 26 ++++++++++++++++++++++++++
 lib/web-push.ts                | 11 +++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts
index ab7da8933..8358ad888 100644
--- a/lib/__tests__/web-push.test.ts
+++ b/lib/__tests__/web-push.test.ts
@@ -231,6 +231,32 @@ describe('enableWebPush', () => {
     expect(calls.some((c) => c.url.endsWith('/api/push/register/web'))).toBe(true);
   });
 
+  it('subscribes anyway when the browser cannot retrieve the existing subscription', async () => {
+    // Firefox rejects the lookup with `AbortError: Error retrieving push
+    // subscription` when its local record is stale; that must not abort enable.
+    const { registration } = installPushBrowser();
+    registration.pushManager.getSubscription.mockRejectedValue(
+      new DOMException('Error retrieving push subscription', 'AbortError'),
+    );
+    installFetch({});
+
+    const result = await enableWebPush({ client: makeClient([]), relayBaseUrl: RELAY });
+
+    expect(registration.pushManager.subscribe).toHaveBeenCalledTimes(1);
+    expect(result.subscriptionId).toBe('push-new');
+  });
+
+  it('still subscribes when unsubscribing the mismatched subscription fails', async () => {
+    const { browserSub, registration } = installPushBrowser();
+    browserSub.options.applicationServerKey = new Uint8Array([9, 9, 9]).buffer;
+    browserSub.unsubscribe.mockRejectedValue(new Error('unsubscribe failed'));
+    installFetch({});
+
+    await enableWebPush({ client: makeClient([]), relayBaseUrl: RELAY });
+
+    expect(registration.pushManager.subscribe).toHaveBeenCalledTimes(1);
+  });
+
   it('replaces a browser subscription whose key the browser does not expose', async () => {
     const { browserSub, registration } = installPushBrowser();
     browserSub.options.applicationServerKey = null;
diff --git a/lib/web-push.ts b/lib/web-push.ts
index b2e5a7399..363e7a1c2 100644
--- a/lib/web-push.ts
+++ b/lib/web-push.ts
@@ -424,12 +424,19 @@ export async function enableWebPush(
   // service on every send - Mozilla answers 401 "VAPID public key mismatch" -
   // while isWebPushEnabled keeps reporting push as on.
   const applicationServerKey = urlBase64ToUint8Array(vapidPublicKey);
-  let pushSubscription = await registration.pushManager.getSubscription();
+  // A stale or broken local record makes Firefox reject the lookup itself with
+  // `AbortError: Error retrieving push subscription`, which would abort the
+  // whole enable. There is nothing to reuse in that case, so carry on to
+  // subscribe() and let its own error surface if the push service is really
+  // unreachable.
+  let pushSubscription = await registration.pushManager.getSubscription().catch(() => null);
   if (
     pushSubscription
     && !sameKey(pushSubscription.options?.applicationServerKey, applicationServerKey)
   ) {
-    await pushSubscription.unsubscribe();
+    // An unsubscribe that fails leaves the old record in place; subscribing
+    // with the right key is what matters, so don't abort the enable for it.
+    await pushSubscription.unsubscribe().catch(() => undefined);
     pushSubscription = null;
   }
   if (!pushSubscription) {