feat(aet): handle generateEcosystemAnonId update failure

Because:

- When generateEcosystemAnonId calls updateEcosystemAnonId it can fail with a 412 error, which means we tried to update the anon id but another client beat us to it. We need to handle this scenario.

This commit:

- When updateEcosystemAnonId fails in this method, check if it's a 412 error, and retrieve the existing anon id from the auth server to store on the account instead
This commit is contained in:
Jody Heavener 2020-07-25 20:31:44 -04:00
Родитель f8247009d4
Коммит ff592f0de1
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: 065C43AE0EE5A19A
3 изменённых файлов: 105 добавлений и 12 удалений

Просмотреть файл

@ -614,6 +614,12 @@ var ERRORS = {
errno: 1065, errno: 1065,
message: t('The image file size is too large to be uploaded.'), message: t('The image file size is too large to be uploaded.'),
}, },
ECOSYSTEM_ANON_ID_UPDATE_CONFLICT: {
errno: 190,
message: t(
'Could not update Ecosystem Anon ID because precondition failed.'
),
},
}; };
export default _.extend({}, Errors, { export default _.extend({}, Errors, {

Просмотреть файл

@ -363,6 +363,7 @@ const Account = Backbone.Model.extend(
* authenticationMethods, * authenticationMethods,
* authenticatorAssuranceLevel, * authenticatorAssuranceLevel,
* profileChangedAt, * profileChangedAt,
* ecosystemAnonId,
* } * }
*/ */
accountProfile() { accountProfile() {
@ -458,9 +459,29 @@ const Account = Backbone.Model.extend(
); );
} }
} catch (err) { } catch (err) {
// In the event of errors, we don't want to block any user actions, just log it this.unset('ecosystemAnonId');
// TODO: Follow up with some better error handling
console.log('generateEcosystemAnonId failed...', err); const reportException = () => {
if (this._sentryMetrics) {
this._sentryMetrics.captureException(err);
}
};
// If we get 412 Precondition Failed it means we
// tried to update, but another client beat us to
// it. Fetch the new anon ID and use it instead.
if (AuthErrors.is(err, 'ECOSYSTEM_ANON_ID_UPDATE_CONFLICT')) {
try {
const { ecosystemAnonId } = await this.accountProfile();
if (ecosystemAnonId) {
this.set('ecosystemAnonId', ecosystemAnonId);
}
} catch (err) {
reportException(err);
}
} else {
reportException(err);
}
} }
}, },

Просмотреть файл

@ -3210,22 +3210,88 @@ describe('models/account', function () {
assert.isTrue(fxaClient.updateEcosystemAnonId.notCalled); assert.isTrue(fxaClient.updateEcosystemAnonId.notCalled);
}); });
it('swallow any errors', async () => { it('ignores with no options', async () => {
const err = new Error('failure'); await account.generateEcosystemAnonId();
sandbox.restore(); assert.isTrue(fxaClient.updateEcosystemAnonId.notCalled);
sandbox.stub(aet, 'generateEcosystemAnonID').rejects(err); });
sandbox.spy(fxaClient, 'updateEcosystemAnonId');
it('handles 412 errors by looking up existing eco anon id', async () => {
fxaClient.updateEcosystemAnonId.restore();
account.set({ sessionToken: SESSION_TOKEN });
sandbox.spy(account, 'set');
sandbox.spy(account, 'accountProfile');
sandbox.stub(fxaClient, 'accountProfile').resolves({
ecosystemAnonId: 'wow',
});
sandbox.stub(fxaClient, 'sessionReauth').resolves({
keyFetchToken: 'keyFetchToken',
unwrapBKey: 'unwrapBKey',
});
sandbox.stub(fxaClient, 'accountKeys').resolves({
kB: 'kB1',
});
sandbox.stub(fxaClient, 'sessionVerificationStatus').resolves({
sessionVerified: true,
});
sandbox
.stub(fxaClient, 'updateEcosystemAnonId')
.rejects(AuthErrors.toError('ECOSYSTEM_ANON_ID_UPDATE_CONFLICT'));
await account.generateEcosystemAnonId({ await account.generateEcosystemAnonId({
password: 'passwordzxcv', password: 'passwordzxcv',
}); });
assert.isTrue(fxaClient.updateEcosystemAnonId.notCalled); sinon.assert.called(account.accountProfile);
sinon.assert.calledWith(fxaClient.accountProfile, SESSION_TOKEN);
sinon.assert.calledWith(account.set, 'ecosystemAnonId', 'wow');
}); });
it('ignores with no options', async () => { it('reports errors thrown by accountProfile when looking up existing eco anon id because of a 412 error', async () => {
await account.generateEcosystemAnonId(); fxaClient.updateEcosystemAnonId.restore();
assert.isTrue(fxaClient.updateEcosystemAnonId.notCalled); account.set({ sessionToken: SESSION_TOKEN });
sandbox.spy(account._sentryMetrics, 'captureException');
sandbox.spy(account, 'set');
sandbox.spy(account, 'accountProfile');
sandbox
.stub(fxaClient, 'accountProfile')
.rejects(AuthErrors.toError('UNAUTHORIZED'));
sandbox.stub(fxaClient, 'sessionReauth').resolves({
keyFetchToken: 'keyFetchToken',
unwrapBKey: 'unwrapBKey',
});
sandbox.stub(fxaClient, 'accountKeys').resolves({
kB: 'kB1',
});
sandbox.stub(fxaClient, 'sessionVerificationStatus').resolves({
sessionVerified: true,
});
sandbox
.stub(fxaClient, 'updateEcosystemAnonId')
.rejects(AuthErrors.toError('ECOSYSTEM_ANON_ID_UPDATE_CONFLICT'));
await account.generateEcosystemAnonId({
password: 'passwordzxcv',
});
sinon.assert.called(account.accountProfile);
sinon.assert.calledWith(fxaClient.accountProfile, SESSION_TOKEN);
sinon.assert.called(account._sentryMetrics.captureException);
assert.isFalse(account.has('ecosystemAnonId'));
});
it('reports any other errors to sentry and removes the newly create anon id', async () => {
sandbox.restore();
sandbox.spy(account._sentryMetrics, 'captureException');
sandbox
.stub(aet, 'generateEcosystemAnonID')
.rejects(new Error('failure'));
await account.generateEcosystemAnonId({
password: 'passwordzxcv',
});
assert.isFalse(account.has('ecosystemAnonId'));
sinon.assert.called(account._sentryMetrics.captureException);
}); });
}); });
}); });