Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Commit

Permalink
Merge pull request #32867 from ferjm/bug1212874.sync.errors
Browse files Browse the repository at this point in the history 
Bug 1212874 - Losing connection while syncing disables Firefox Sync. …
  • Loading branch information
@ferjm
ferjm committed Nov 1, 2015
2 parents 8bd1fd2 + 67765f8 commit 0346df4
Show file tree
Hide file tree
Showing 5 changed files with 204 additions and 70 deletions.
41 changes: 32 additions & 9 deletions apps/system/js/sync_manager.js
View file
Expand Up @@ -258,6 +258,15 @@
_handle_onsyncenabled: function(event) {
this.debug('onsyncenabled observed');
this.updateState();

// If we got to this point with no user, something went wrong, so we
// need to disable Sync.
if (!this.user) {
this.debug('No user');
Service.request('SyncStateMachine:disable');
return;
}

this.registerSyncRequest().then(() => {
this.debug('Sync request registered');
var from = event.detail && event.detail.from;
Expand Down Expand Up @@ -343,11 +352,14 @@
// We don't update the state until we set the error.
this.updateState();

// If the error is not recoverable, we disable Sync.
if (SyncRecoverableErrors.indexOf(error) <= -1) {
this.debug('Unrecoverable error');
Service.request('SyncStateMachine:disable');
// If the error is recoverable we go back to the enabled state,
// otherwise, we disable Sync.
if (SyncRecoverableErrors.indexOf(error) > -1) {
Service.request('SyncStateMachine:enable');
return;
}
this.debug('Unrecoverable error');
Service.request('SyncStateMachine:disable');
},

_handle_onsyncsyncing: function() {
Expand Down Expand Up @@ -610,9 +622,13 @@
collections: collections
}).then(result => {
if (result.error) {
console.error('Error while trying to sync', result.error);
// XXX The sync app needs to propagate a less general error.
Service.request('SyncStateMachine:error', ERROR_SYNC_APP_GENERIC);
var error = result.error;
error = error.message ? error.message : error;
console.error('Error while trying to sync', error);

error = SyncErrors[error] || ERROR_SYNC_APP_GENERIC;

Service.request('SyncStateMachine:error', error);
return;
}

Expand Down Expand Up @@ -746,8 +762,15 @@

user: {
set: function(user) {
asyncStorage.setItem(SYNC_USER, user, () => {
this.store.set(SYNC_USER, user);
// We need to check if there's a user logged in during the enabled
// state handler, so we cannot wait for the asyncStorage write.
this.store.set(SYNC_USER, user);
asyncStorage.setItem(SYNC_USER, user, () => {}, () => {
// This should never happen, but if something goes wrong storing the
// user we delete the user from memory and disable Sync to avoid
// inconsistent states.
this.store.delete(SYNC_USER);
Service.request('SyncStateMachine:disable');
});
},
get: function() {
Expand Down
91 changes: 63 additions & 28 deletions apps/system/test/unit/sync_manager_test.js
View file
Expand Up @@ -11,7 +11,6 @@
/* global ERROR_GET_FXA_ASSERTION */
/* global ERROR_INVALID_SYNC_ACCOUNT */
/* global ERROR_SYNC_APP_KILLED */
/* global ERROR_SYNC_REQUEST */
/* global ERROR_UNVERIFIED_ACCOUNT */
/* global expect */
/* global FxAccountsClient */
Expand All @@ -20,6 +19,7 @@
/* global MockNavigatormozSetMessageHandler */
/* global MockNavigatorSettings */
/* global MockService */
/* global SyncRecoverableErrors */

requireApp('system/js/service.js');
requireApp('system/test/unit/mock_asyncStorage.js');
Expand Down Expand Up @@ -115,6 +115,11 @@ suite('system/SyncManager >', () => {
requestStub = this.sinon.stub(MockService, 'request', () => {
return Promise.resolve();
});
this.sinon.stub(syncManager, 'getAccount', () => {
syncManager.user = 'someone';
return Promise.resolve('someone');
});

syncManager.start().then(done);
});
});
Expand Down Expand Up @@ -361,6 +366,7 @@ suite('system/SyncManager >', () => {
requestStub = this.sinon.stub(MockService, 'request', () => {
return Promise.resolve();
});
syncManager.user = 'someone';
});

teardown(() => {
Expand Down Expand Up @@ -427,6 +433,16 @@ suite('system/SyncManager >', () => {
done();
});
});

test('onsyncenabled received - no user should disable sync', done => {
syncManager.user = null;
window.dispatchEvent(new CustomEvent('onsyncenabled'));
setTimeout(() => {
this.sinon.assert.calledOnce(requestStub);
assert.ok(requestStub.calledWith('SyncStateMachine:disable'));
done();
});
});
});

suite('onsyncenabling', () => {
Expand Down Expand Up @@ -573,7 +589,6 @@ suite('system/SyncManager >', () => {
done();
});
});

});

suite('onsyncerrored', () => {
Expand All @@ -582,6 +597,23 @@ suite('system/SyncManager >', () => {
var updateStateSpy;
var requestStub;

const ERROR_SYNC_APP_KILLED = 'fxsync-error-app-killed';
const ERROR_SYNC_APP_SYNC_IN_PROGRESS =
'fxsync-error-app-fxsync-in-progress';
const ERROR_SYNC_APP_GENERIC = 'fxsync-error-app-generic';
const ERROR_SYNC_APP_TRY_LATER = 'fxsync-error-app-try-later';
const ERROR_UNVERIFIED_ACCOUNT = 'fxsync-error-unverified-account';
const ERROR_DIALOG_CLOSED_BY_USER = 'fxsync-error-dialog-closed-by-user';
const ERROR_GET_FXA_ASSERTION = 'fxsync-error-get-fxa-assertion';
const ERROR_INVALID_SYNC_ACCOUNT = 'fxsync-error-invalid-account';
const ERROR_OFFLINE = 'fxsync-error-offline';
const ERROR_REQUEST_SYNC_REGISTRATION =
'fxsync-error-request-fxsync-registration';
const ERROR_SYNC_INVALID_REQUEST_OPTIONS =
'fxsync-error-invalid-request-options';
const ERROR_SYNC_REQUEST = 'fxsync-error-request-failed';
const ERROR_UNKNOWN = 'fxsync-error-unknown';

suiteSetup(() => {
syncManager = BaseModule.instantiate('SyncManager');
syncManager.start();
Expand All @@ -603,32 +635,35 @@ suite('system/SyncManager >', () => {
requestStub.restore();
});

test('onsyncerrored received - recoverable error', done => {
window.dispatchEvent(new CustomEvent('onsyncerrored', {
detail: {
args: [ERROR_SYNC_APP_KILLED]
}
}));
setTimeout(() => {
this.sinon.assert.calledOnce(updateStateSpy);
assert.equal(syncManager.error, ERROR_SYNC_APP_KILLED);
this.sinon.assert.notCalled(requestStub);
done();
});
});

test('onsyncerrored received - unrecoverable error', done => {
window.dispatchEvent(new CustomEvent('onsyncerrored', {
detail: {
args: [ERROR_SYNC_REQUEST]
}
}));
setTimeout(() => {
this.sinon.assert.calledOnce(updateStateSpy);
assert.equal(syncManager.error, ERROR_SYNC_REQUEST);
this.sinon.assert.calledOnce(requestStub);
assert.ok(requestStub.calledWith('SyncStateMachine:disable'));
done();
[ERROR_SYNC_APP_KILLED,
ERROR_SYNC_APP_SYNC_IN_PROGRESS,
ERROR_SYNC_APP_GENERIC,
ERROR_SYNC_APP_TRY_LATER,
ERROR_UNVERIFIED_ACCOUNT,
ERROR_DIALOG_CLOSED_BY_USER,
ERROR_GET_FXA_ASSERTION,
ERROR_INVALID_SYNC_ACCOUNT,
ERROR_OFFLINE,
ERROR_REQUEST_SYNC_REGISTRATION,
ERROR_SYNC_INVALID_REQUEST_OPTIONS,
ERROR_SYNC_REQUEST,
ERROR_UNKNOWN].forEach(error => {
test(`onerrored received - ${error}`, done => {
window.dispatchEvent(new CustomEvent('onsyncerrored', {
detail: {
args: [error]
}
}));
setTimeout(() => {
this.sinon.assert.calledOnce(updateStateSpy);
assert.equal(syncManager.error, error);
if (SyncRecoverableErrors.indexOf(error) > -1) {
assert.ok(requestStub.calledWith('SyncStateMachine:enable'));
} else {
assert.ok(requestStub.calledWith('SyncStateMachine:disable'));
}
done();
});
});
});
});
Expand Down
13 changes: 10 additions & 3 deletions shared/js/sync/errors.js
View file
Expand Up @@ -4,9 +4,11 @@

'use strict';

/* global ERROR_DIALOG_CLOSED_BY_USER */
/* global ERROR_INVALID_SYNC_ACCOUNT */
/* global ERROR_OFFLINE */
/* global ERROR_DIALOG_CLOSED_BY_USER */
/* global ERROR_SYNC_APP_TRY_LATER */
/* global ERROR_UNKNOWN */

/**
* These errors are shared by all Firefox Sync related modules. The key should
Expand Down Expand Up @@ -45,6 +47,8 @@
ERROR_SYNC_APP_SYNC_IN_PROGRESS: 'fxsync-error-app-fxsync-in-progress',
// Error while trying to sync.
ERROR_SYNC_APP_GENERIC: 'fxsync-error-app-generic',
// The server cannot be reached.
ERROR_SYNC_APP_TRY_LATER: 'fxsync-error-app-try-later',
// The user is logged in with an unverified account.
ERROR_UNVERIFIED_ACCOUNT: 'fxsync-error-unverified-account'
};
Expand All @@ -64,10 +68,13 @@
// Map between external (FxA, Syncto, kinto.js) and Sync errors.
exports.SyncErrors = {
'invalid account': ERROR_INVALID_SYNC_ACCOUNT,
'UNVERIFIED_ACCOUNT': ERROR_INVALID_SYNC_ACCOUNT,
'No keyFetchToken': ERROR_INVALID_SYNC_ACCOUNT,
'OFFLINE': ERROR_OFFLINE,
'UI_ERROR': ERROR_DIALOG_CLOSED_BY_USER
'try later': ERROR_SYNC_APP_TRY_LATER,
'UI_ERROR': ERROR_DIALOG_CLOSED_BY_USER,
'unauthorized': ERROR_INVALID_SYNC_ACCOUNT,
'UNVERIFIED_ACCOUNT': ERROR_INVALID_SYNC_ACCOUNT,
'unrecoverable': ERROR_UNKNOWN
};

}(window));
38 changes: 29 additions & 9 deletions tv_apps/smart-system/js/sync_manager.js
View file
Expand Up @@ -216,8 +216,15 @@
},

set user(user) {
asyncStorage.setItem(SYNC_USER, user, () => {
this.store.set(SYNC_USER, user);
// We need to check if there's an user logged in during the enabled
// state handler, so we cannot wait for the asyncStorage write.
this.store.set(SYNC_USER, user);
asyncStorage.setItem(SYNC_USER, user, () => {}, () => {
// This should never happen, but if something goes wrong storing the
// user we delete the user from memory and disable Sync to avoid
// inconsistent states.
this.store.delete(SYNC_USER);
SyncStateMachine.disable();
});
},

Expand Down Expand Up @@ -303,6 +310,15 @@
onenabled(from) {
this.debug('onenabled observed');
this.updateState();

// If we got to this point with no user, something went wrong, so we
// need to disable Sync.
if (!this.user) {
this.debug('No user');
SyncStateMachine.disable();
return;
}

this.registerSyncRequest().then(() => {
this.debug('Sync request registered');
// After login in with a new account or rebooting the device,
Expand Down Expand Up @@ -385,11 +401,14 @@
// We don't update the state until we set the error.
this.updateState();

// If the error is not recoverable, we disable Sync.
if (SyncRecoverableErrors.indexOf(error) <= -1) {
this.debug('Unrecoverable error');
SyncStateMachine.disable();
// If the error is recoverable we go back to the enabled state,
// otherwise, we disable Sync.
if (SyncRecoverableErrors.indexOf(error) > -1) {
SyncStateMachine.enable();
return;
}
this.debug('Unrecoverable error');
SyncStateMachine.disable();
},

onsyncing() {
Expand Down Expand Up @@ -682,9 +701,10 @@
var error = result.error;
error = error.message ? error.message : error;
console.error('Error while trying to sync', error);
// XXX The sync app needs to propagate a less general error.
// Bug 1210412
SyncStateMachine.error(ERROR_SYNC_APP_GENERIC);

error = SyncErrors[error] || ERROR_SYNC_APP_GENERIC;

SyncStateMachine.error(error);
return;
}
this.debug('Sync succeded');
Expand Down

0 comments on commit 0346df4

Please sign in to comment.