Renew the push subscription, so it does not lapse in a week
Background notifications were built, verified against a live server, and then went quiet a few days later on every device that had them. A JMAP push subscription expires -- seven days is the ceiling -- and re-registering before it lapses is the client's job. Nothing did: enableWebPush() was reachable only from the switch in Settings, so the subscription was registered once, expired, and stayed expired. Nobody reports that as a bug. They report that push does not really work. It is renewed on every app start now, which is the only place it can be: the registration is a JMAP call and the service worker has no session cookie to make one with. So the guarantee is that push keeps working as long as ihasmail is opened now and again, and a two-day renewal window against a seven-day ceiling means once a week is enough. Registering is the same call as turning it on -- deviceClientId makes a repeat replace rather than accumulate -- so there is no second path to get wrong. Two more things in the same area, both of which produce the same silence: - webPushActive() asked whether the *account* had any subscription, so the moment one device had one, every other device showed the switch already on. A phone that had never successfully registered, or whose registration had since expired, read as on and delivered nothing. It matches on the device now. - Turning push on reused an existing browser subscription and gave up if there was none. A browser drops or rotates one on its own, and there is no tab open to hear the pushsubscriptionchange when it does, so that state was permanent. Renewal re-subscribes rather than bailing. Whether this browser has push on is now remembered locally, which is what renewal keys off. It is per browser rather than per account on purpose: a subscription is an endpoint and a device, and a phone having push says nothing about the desktop. It is not kept across sign-out, matching sign-out already destroying the subscription itself. The mock is the reason this was invisible in development: it handed back expires: null, so a client that never renewed worked perfectly against it forever. It expires a subscription in seven days now, which is what makes "does this client renew?" a question the mock can answer. Checked against the mock: a create returns an expiry seven days out that survives PushSubscription/get and parses, renewing the same deviceClientId replaces rather than accumulates, and a device with no registration of its own finds nothing where the old code saw two subscriptions and said yes. What the live Stalwart sets for expires is not confirmed -- if it sets none, renewal correctly does nothing and the other two fixes still stand.
This commit is contained in:
@@ -147,6 +147,73 @@ export function subscriptionPayload(sub: PushSubscription, accountId: Id | null,
|
||||
return body;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether push was switched on *in this browser*.
|
||||
*
|
||||
* Device-local on purpose. A subscription is a browser and an endpoint, not an
|
||||
* account: turning it on for a phone says nothing about the desktop, and the
|
||||
* account-wide settings file is the wrong place to record it. It is also not in
|
||||
* `KEEP_ON_SIGN_OUT`, so signing out forgets it, which matches sign-out already
|
||||
* destroying the subscription itself.
|
||||
*/
|
||||
const ENABLED_KEY = "ihasmail:pushEnabled";
|
||||
|
||||
export function pushEnabledHere(): boolean {
|
||||
if (!isDeviceTrusted()) return false;
|
||||
try {
|
||||
return localStorage.getItem(ENABLED_KEY) === "1";
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function setPushEnabledHere(on: boolean): void {
|
||||
try {
|
||||
if (on) localStorage.setItem(ENABLED_KEY, "1");
|
||||
else localStorage.removeItem(ENABLED_KEY);
|
||||
} catch {
|
||||
/* private mode: push will not survive the session there anyway */
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* How close to expiry a subscription is re-registered rather than left alone.
|
||||
*
|
||||
* Two days against a ceiling of seven, so an app opened even once over a
|
||||
* weekend keeps its notifications. Renewing is a single idempotent call, so
|
||||
* being early costs almost nothing and being late costs everything.
|
||||
*/
|
||||
export const RENEW_WITHIN_MS = 2 * 24 * 60 * 60 * 1000;
|
||||
|
||||
/** This browser's registered subscription, out of everything the account has. */
|
||||
export function findSubscription(subs: JmapPushSubscription[], deviceId: string): JmapPushSubscription | null {
|
||||
return subs.find((s) => s.deviceClientId === deviceId) ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this browser's subscription needs registering again.
|
||||
*
|
||||
* A JMAP push subscription expires -- seven days is the ceiling -- and it is
|
||||
* the client's job to re-register before it does. Nothing did: `enableWebPush`
|
||||
* was reachable only from the Settings switch, so the
|
||||
* first version of this quietly stopped delivering within a week of being
|
||||
* turned on, and stayed off until somebody thought to toggle it. On a phone,
|
||||
* where the app is opened for a minute at a time and Settings almost never,
|
||||
* that is indistinguishable from the feature not working.
|
||||
*
|
||||
* An expiry that will not parse counts as needing renewal. It should never
|
||||
* happen; if it does, one extra write is the cheaper way to be wrong.
|
||||
*/
|
||||
export function needsRenewal(subs: JmapPushSubscription[], deviceId: string, now: number = Date.now()): boolean {
|
||||
const mine = findSubscription(subs, deviceId);
|
||||
if (!mine) return true;
|
||||
// No expiry: the server is not going to take it away, so leave it alone.
|
||||
if (!mine.expires) return false;
|
||||
const at = Date.parse(mine.expires);
|
||||
if (Number.isNaN(at)) return true;
|
||||
return at - now <= RENEW_WITHIN_MS;
|
||||
}
|
||||
|
||||
export async function listSubscriptions(): Promise<JmapPushSubscription[]> {
|
||||
const res = await client.call<GetResponse<JmapPushSubscription>>("PushSubscription/get", { ids: null }, [CAP.core, VAPID_CAP]);
|
||||
return res.list;
|
||||
@@ -200,4 +267,5 @@ export async function unsubscribeThisDevice(): Promise<void> {
|
||||
} catch {
|
||||
/* signing out must not fail over this */
|
||||
}
|
||||
setPushEnabledHere(false);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user