diff --git a/src/lib/__tests__/push-notifications.test.ts b/src/lib/__tests__/push-notifications.test.ts index 221ab701..5726c888 100644 --- a/src/lib/__tests__/push-notifications.test.ts +++ b/src/lib/__tests__/push-notifications.test.ts @@ -1118,6 +1118,7 @@ describe('setupPushNotifications at the server\'s subscription limit', () => { await AsyncStorage.clear(); await AsyncStorage.setItem(deviceClientIdKey(ACCOUNT_ID), OUR_DCID); await AsyncStorage.setItem(SUB_KEY, 'existing'); + await AsyncStorage.setItem('push:relayBaseUrl:v1', RELAY); listMock.mockResolvedValue([recorded(80)]); installFetch({}); }); @@ -1128,6 +1129,26 @@ describe('setupPushNotifications at the server\'s subscription limit', () => { updateMock.mockImplementation(async () => undefined); }); + it('keeps the working record when a forced re-register moves to another relay', async () => { + // Stalwart answers overQuota before it looks at the URL; freeing a place + // would only lead to the new relay being refused with the old record gone. + let attempts = 0; + createMock.mockImplementation(async () => { + attempts += 1; + throw attempts === 1 + ? overQuota() + : new JMAPSetError('invalidProperties', 'Host is not a public domain name', ['url']); + }); + + const err = await setupPushNotifications({ relayBaseUrl: 'https://relay.home.lan', forceRecreate: true }).catch((e) => e); + + expect(err).toBeInstanceOf(PushSetupError); + expect(err.message).toContain('too many push registrations on the mail server'); + expect(createMock).toHaveBeenCalledTimes(1); + expect(destroyMock.mock.calls.map((c) => c[0])).not.toContain('existing'); + expect(await AsyncStorage.getItem(SUB_KEY)).toBe('existing'); + }); + it('a forced re-register frees the old record\'s place and tries once more', async () => { createMock.mockRejectedValueOnce(overQuota()); diff --git a/src/lib/push-notifications.ts b/src/lib/push-notifications.ts index 254cdfb3..1b4e9b4f 100644 --- a/src/lib/push-notifications.ts +++ b/src/lib/push-notifications.ts @@ -1388,8 +1388,11 @@ async function setupPushNotificationsInner( // the one being replaced, so at the cap a forced re-register could never // create its replacement. The user asked for a new record: free the old // one's place and try once more. Should that fail too, push is off and - // the error says why. - if (!params.forceRecreate || !replacedServerId || !isJMAPSetError(err) || err.type !== 'overQuota') throw err; + // the error says why. Not when this run moves to another relay: Stalwart + // counts the subscriptions before it looks at the URL, so a relay it + // refuses would only show up after the working record is gone. + const relayChanged = previousRelayBaseUrl !== relayBaseUrl; + if (!params.forceRecreate || !replacedServerId || relayChanged || !isJMAPSetError(err) || err.type !== 'overQuota') throw err; logPhase('jmap', 'subscription limit reached, replacing the old record first'); await destroyPushSubscription(replacedServerId); await AsyncStorage.removeItem(subKey);