Commit a0308de1 authored by Robert Knight's avatar Robert Knight

Add automated retry with backoff for session loads

If a session load fails, subsequent calls to
session.load() would trigger another request immediately.
Together with feature flag checks triggering session.load(),
this could result in a cycle if no network connection was present:

 1. App loads during initial digest cycle, 'flag.featureEnabled()'
    is called to test whether to show certain UI elements
 2. session.load() is triggered
 3. session.load() fails and the HTTP response error triggers
    a digest cycle
 4. GOTO 1

This commit resolves the problem by automatically retrying
session.load() calls with an exponential backoff.
parent 8cea7bf2
...@@ -13,7 +13,7 @@ streamer = require('./streamer') ...@@ -13,7 +13,7 @@ streamer = require('./streamer')
resolve = resolve =
# Ensure that we have available a) the current authenticated userid, and b) # Ensure that we have available a) the current authenticated userid, and b)
# the list of user groups. # the list of user groups.
sessionState: ['session', (session) -> session.load().$promise] sessionState: ['session', (session) -> session.load()]
store: ['store', (store) -> store.$promise] store: ['store', (store) -> store.$promise]
streamer: streamer.connect streamer: streamer.connect
threading: [ threading: [
......
...@@ -74,7 +74,7 @@ function checkAuthentication($injector, $q, session) { ...@@ -74,7 +74,7 @@ function checkAuthentication($injector, $q, session) {
var deferred = $q.defer(); var deferred = $q.defer();
authPromise = deferred.promise; authPromise = deferred.promise;
session.load().$promise session.load()
.then(function (data) { .then(function (data) {
if (data.userid) { if (data.userid) {
$injector.invoke(fetchToken).then(function (token) { $injector.invoke(fetchToken).then(function (token) {
......
'use strict';
var retry = require('retry');
var Promise = require('core-js/library/es6/promise');
/**
* Retry a Promise-returning operation until it succeeds or
* fails after a set number of attempts.
*
* @param {Function} opFn - The operation to retry
* @param {Object} options - The options object to pass to retry.operation()
*
* @return A promise for the first successful result of the operation, if
* it succeeds within the allowed number of attempts.
*/
function retryPromiseOperation(opFn, options) {
return new Promise(function (resolve, reject) {
var operation = retry.operation(options);
operation.attempt(function () {
opFn().then(function (result) {
operation.retry();
resolve(result);
}).catch(function (err) {
if (!operation.retry(err)) {
reject(err);
}
});
});
});
}
module.exports = {
retryPromiseOperation: retryPromiseOperation,
};
'use strict'; 'use strict';
var Promise = require('core-js/library/es6/promise');
var angular = require('angular'); var angular = require('angular');
var events = require('./events'); var events = require('./events');
var retryUtil = require('./retry-util');
var CACHE_TTL = 5 * 60 * 1000; // 5 minutes var CACHE_TTL = 5 * 60 * 1000; // 5 minutes
...@@ -84,28 +84,35 @@ function session($http, $resource, $rootScope, flash, raven, settings) { ...@@ -84,28 +84,35 @@ function session($http, $resource, $rootScope, flash, raven, settings) {
/** /**
* @name session.load() * @name session.load()
* @description * @description Fetches the session data from the server.
* Fetches the session data from the server. This function returns an object * @returns A promise for the session data.
* with a $promise property which resolves to the session data.
* *
* N.B. The data is cached for CACHE_TTL across all actions of the session * The data is cached for CACHE_TTL across all actions of the session
* service: that is, a call to login() will update the session data and a call * service: that is, a call to login() will update the session data and a call
* within CACHE_TTL milliseconds to load() will return that data rather than * within CACHE_TTL milliseconds to load() will return that data rather than
* triggering a new request. * triggering a new request.
*/ */
resource.load = function () { resource.load = function () {
if (!lastLoadTime || (Date.now() - lastLoadTime) > CACHE_TTL) { if (!lastLoadTime || (Date.now() - lastLoadTime) > CACHE_TTL) {
lastLoad = resource._load();
// The load attempt is automatically retried with a backoff.
//
// This serves to make loading the app in the extension cope better with
// flakey connectivity but it also throttles the frequency of calls to
// the /app endpoint.
lastLoadTime = Date.now(); lastLoadTime = Date.now();
// If the load fails, we need to clear out lastLoadTime so another load lastLoad = retryUtil.retryPromiseOperation(function () {
// attempt will succeed. return resource._load().$promise;
lastLoad.$promise.catch(function () { }).then(function (session) {
lastLoadTime = Date.now();
return session;
}).catch(function (err) {
lastLoadTime = null; lastLoadTime = null;
throw err;
}); });
} }
return lastLoad; return lastLoad;
}; }
/** /**
* @name session.update() * @name session.update()
...@@ -130,8 +137,7 @@ function session($http, $resource, $rootScope, flash, raven, settings) { ...@@ -130,8 +137,7 @@ function session($http, $resource, $rootScope, flash, raven, settings) {
headers[$http.defaults.xsrfHeaderName] = resource.state.csrf; headers[$http.defaults.xsrfHeaderName] = resource.state.csrf;
} }
// Replace lastLoad with the latest data, and update lastLoadTime. lastLoad = Promise.resolve(resource.state);
lastLoad = {$promise: Promise.resolve(model), $resolved: true};
lastLoadTime = Date.now(); lastLoadTime = Date.now();
$rootScope.$broadcast(events.SESSION_CHANGED, { $rootScope.$broadcast(events.SESSION_CHANGED, {
......
/**
* Takes a Promise<T> and returns a Promise<Result>
* where Result = { result: T } | { error: any }.
*
* This is useful for testing that promises are rejected
* as expected in tests.
*/
function toResult(promise) {
return promise.then(function (result) {
return { result: result };
}).catch(function (err) {
return { error: err }
});
}
module.exports = {
toResult: toResult,
};
var Promise = require('core-js/library/es6/promise');
var retryUtil = require('../retry-util');
var toResult = require('./promise-util').toResult;
describe('retry-util', function () {
describe('.retryPromiseOperation', function () {
it('should return the result of the operation function', function () {
var operation = sinon.stub().returns(Promise.resolve(42));
var wrappedOperation = retryUtil.retryPromiseOperation(operation);
return wrappedOperation.then(function (result) {
assert.equal(result, 42);
});
});
it('should retry the operation if it fails', function () {
var results = [new Error('fail'), 'ok'];
var operation = sinon.spy(function () {
var nextResult = results.shift();
if (nextResult instanceof Error) {
return Promise.reject(nextResult);
} else {
return Promise.resolve(nextResult);
}
});
var wrappedOperation = retryUtil.retryPromiseOperation(operation, {
minTimeout: 1,
});
return wrappedOperation.then(function (result) {
assert.equal(result, 'ok');
});
});
it('should return the error if it repeatedly fails', function () {
var error = new Error('error');
var operation = sinon.spy(function () {
return Promise.reject(error);
});
var wrappedOperation = retryUtil.retryPromiseOperation(operation, {
minTimeout: 1,
maxRetries: 2,
});
return toResult(wrappedOperation).then(function (result) {
assert.equal(result.error, error);
});
});
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment