Skip to content

Commit 8014bb0

Browse files
authored
Merge pull request #20103 from mozilla/dont-await-push
fix(push): Failures in push notification should not fail verify code endpoint
2 parents b2907a1 + fddd776 commit 8014bb0

2 files changed

Lines changed: 28 additions & 2 deletions

File tree

packages/fxa-auth-server/lib/routes/session.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,9 @@ module.exports = function (
517517
await request.emitMetricsEvent('account.confirmed', { uid });
518518
glean.login.verifyCodeConfirmed(request, { uid });
519519
await signinUtils.cleanupReminders({ verified: true }, account);
520-
await push.notifyAccountUpdated(uid, devices, 'accountConfirm');
520+
push.notifyAccountUpdated(uid, devices, 'accountConfirm').catch((err) =>
521+
log.error('push.accountConfirm.error', { uid, err })
522+
);
521523

522524
// Send new device login notification email after successful verification
523525
const geoData = request.app.geo;
@@ -885,7 +887,9 @@ module.exports = function (
885887
glean.login.verifyCodeConfirmed(request, { uid });
886888
await signinUtils.cleanupReminders({ verified: true }, account);
887889
const devices = await db.devices(uid);
888-
await push.notifyAccountUpdated(uid, devices, 'accountConfirm');
890+
push.notifyAccountUpdated(uid, devices, 'accountConfirm').catch((err) =>
891+
log.error('push.accountConfirm.error', { uid, err })
892+
);
889893

890894
// Send new device login notification email after successful verification
891895
if (account.primaryEmail.isVerified) {

packages/fxa-auth-server/test/local/routes/session.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1592,6 +1592,28 @@ describe('/session/verify_code', () => {
15921592
assert.calledOnce(fxaMailer.sendNewDeviceLoginEmail);
15931593
});
15941594

1595+
it('should succeed even if push notification fails', async () => {
1596+
setup({ emailVerified: true });
1597+
push.notifyAccountUpdated = sinon.spy(() =>
1598+
Promise.reject(new Error('push timeout'))
1599+
);
1600+
const routes = makeRoutes({
1601+
log,
1602+
config: {},
1603+
db,
1604+
mailer,
1605+
push,
1606+
customs,
1607+
cadReminders,
1608+
gleanMock,
1609+
});
1610+
route = getRoute(routes, '/session/verify_code');
1611+
1612+
const response = await runTest(route, request);
1613+
assert.deepEqual(response, {});
1614+
assert.calledOnce(push.notifyAccountUpdated);
1615+
});
1616+
15951617
it('should fail for invalid code', async () => {
15961618
request.payload.code =
15971619
request.payload.code === '123123' ? '123122' : '123123';

0 commit comments

Comments
 (0)