Merge pull request #3618 from LLK/hotfix/join-retry-session

[Develop] Hotfix/join retry session

src/components/join-flow/join-flow.jsx

@@ -121,13 +121,16 @@ class JoinFlow extends React.Component {
     //   * "birth_month": ["Ensure this value is greater than or equal to 1."]
     handleRegistrationResponse (err, body, res) {
         this.setState({
-            numAttempts: this.state.numAttempts + 1,
-            waiting: false
+            numAttempts: this.state.numAttempts + 1
         }, () => {
             const success = this.registrationIsSuccessful(err, body, res);
             if (success) {
-                this.props.refreshSession();
-                this.setState({step: this.state.step + 1});
+                this.props.refreshSessionWithRetry().then(() => {
+                    this.setState({
+                        step: this.state.step + 1,
+                        waiting: false
+                    });
+                });
                 return;
             }
             // now we know something went wrong -- either an actual error (client-side
@@ -135,7 +138,10 @@ class JoinFlow extends React.Component {
 
             // if an actual error, prompt user to try again.
             if (err || res.statusCode !== 200) {
-                this.setState({registrationError: {errorAllowsTryAgain: true}});
+                this.setState({
+                    registrationError: {errorAllowsTryAgain: true},
+                    waiting: false
+                });
                 return;
             }
 
@@ -164,7 +170,8 @@ class JoinFlow extends React.Component {
                 registrationError: {
                     errorAllowsTryAgain: false,
                     errorMsg: errorMsg
-                }
+                },
+                waiting: false
             });
         });
     }
@@ -283,15 +290,15 @@ JoinFlow.propTypes = {
     createProjectOnComplete: PropTypes.bool,
     intl: intlShape,
     onCompleteRegistration: PropTypes.func,
-    refreshSession: PropTypes.func
+    refreshSessionWithRetry: PropTypes.func
 };
 
 const IntlJoinFlow = injectIntl(JoinFlow);
 
 const mapDispatchToProps = dispatch => ({
-    refreshSession: () => {
-        dispatch(sessionActions.refreshSession());
-    }
+    refreshSessionWithRetry: () => (
+        dispatch(sessionActions.refreshSessionWithRetry())
+    )
 });
 
 // Allow incoming props to override redux-provided props. Used to mock in tests.

src/lib/session.js

+const api = require('./api');
+
+module.exports = {};
+
+/*
+requestSessionWithRetry()
+
+Retries the session api call until it has either reached the limit of number of
+retries, or received a successful response with that contains a 'user' field in
+its body.
+
+Each time it retries, it will double its previous waiting time, and subtract
+that time from the totalDelayMS parameter.
+
+example of what this might look like:
+
+1st call:
+receives params: retriesLeft=3 and totalDelayMS=3500
+performs api call
+delay until next call: 3500 / (2^3 - 1) = 500ms
+next call params: retriesLeft=2, totalDelayMS=3000
+
+2nd call:
+receives params: retriesLeft=2 and totalDelayMS=3000
+performs api call
+delay until next call: 3000 / (2^2 - 1) = 1000ms
+next call: retriesLeft=1, totalDelayMS=2000
+
+3rd call:
+receives params: retriesLeft=1, totalDelayMS=2000
+performs api call
+delay until next call: 2000 / (2^1 - 1) = 2000ms
+next call: retriesLeft=0, totalDelayMS=0
+
+4th call:
+receives params: retriesLeft=0, totalDelayMS=0
+performs api call
+returns the response, even if it is undefined or empty
+
+total api calls: 4
+total delay time: 3500ms
+*/
+module.exports.requestSessionWithRetry = (resolve, reject, retriesLeft, totalDelayMS) => {
+    api({
+        host: '',
+        uri: '/session/'
+    }, (err, body, response) => {
+        if (err || (response && response.statusCode === 404)) {
+            return reject(err);
+        }
+        if (typeof body === 'undefined' || body === null || !body.user) {
+            if (retriesLeft < 1) {
+                return resolve(body);
+            }
+            const nextTimeout = totalDelayMS / (Math.pow(2, retriesLeft) - 1);
+            return setTimeout(
+                module.exports.requestSessionWithRetry.bind(
+                    null, resolve, reject, retriesLeft - 1, totalDelayMS - nextTimeout
+                ),
+                nextTimeout
+            );
+        }
+        return resolve(body);
+    });
+};
+
+module.exports.requestSession = (resolve, reject) => (
+    module.exports.requestSessionWithRetry(resolve, reject, 0, 0)
+);

src/redux/navigation.js

@@ -110,8 +110,9 @@ module.exports.handleCompleteRegistration = createProject => (dispatch => {
         // to be logged in before we try creating a project due to replication lag.
         window.location = '/';
     } else {
-        dispatch(sessionActions.refreshSession());
-        dispatch(module.exports.setRegistrationOpen(false));
+        dispatch(sessionActions.refreshSessionWithRetry()).then(
+            dispatch(module.exports.setRegistrationOpen(false))
+        );
     }
 });
 

src/redux/session.js

 const keyMirror = require('keymirror');
 const defaults = require('lodash.defaults');
 
-const api = require('../lib/api');
+const {requestSession, requestSessionWithRetry} = require('../lib/session');
 const messageCountActions = require('./message-count.js');
 const permissionsActions = require('./permissions.js');
 
@@ -61,45 +61,60 @@ module.exports.setStatus = status => ({
     status: status
 });
 
+const handleSessionResponse = (dispatch, body) => {
+    if (typeof body === 'undefined') return dispatch(module.exports.setSessionError('No session content'));
+    if (
+        body.user &&
+        body.user.banned &&
+        banWhitelistPaths.indexOf(window.location.pathname) === -1
+    ) {
+        window.location = '/accounts/banned-response/';
+        return;
+    } else if (
+        body.flags &&
+        body.flags.must_complete_registration &&
+        window.location.pathname !== '/classes/complete_registration'
+    ) {
+        window.location = '/classes/complete_registration';
+        return;
+    } else if (
+        body.flags &&
+        body.flags.must_reset_password &&
+        !body.flags.must_complete_registration &&
+        window.location.pathname !== '/classes/student_password_reset/'
+    ) {
+        window.location = '/classes/student_password_reset/';
+        return;
+    }
+    dispatch(module.exports.setSession(body));
+    dispatch(module.exports.setStatus(module.exports.Status.FETCHED));
+
+    // get the permissions from the updated session
+    dispatch(permissionsActions.storePermissions(body.permissions));
+    if (typeof body.user !== 'undefined') {
+        dispatch(messageCountActions.getCount(body.user.username));
+    }
+    return;
+};
+
 module.exports.refreshSession = () => (dispatch => {
     dispatch(module.exports.setStatus(module.exports.Status.FETCHING));
-    api({
-        host: '',
-        uri: '/session/'
-    }, (err, body) => {
-        if (err) return dispatch(module.exports.setSessionError(err));
-        if (typeof body === 'undefined') return dispatch(module.exports.setSessionError('No session content'));
-        if (
-            body.user &&
-            body.user.banned &&
-            banWhitelistPaths.indexOf(window.location.pathname) === -1
-        ) {
-            window.location = '/accounts/banned-response/';
-            return;
-        } else if (
-            body.flags &&
-            body.flags.must_complete_registration &&
-            window.location.pathname !== '/classes/complete_registration'
-        ) {
-            window.location = '/classes/complete_registration';
-            return;
-        } else if (
-            body.flags &&
-            body.flags.must_reset_password &&
-            !body.flags.must_complete_registration &&
-            window.location.pathname !== '/classes/student_password_reset/'
-        ) {
-            window.location = '/classes/student_password_reset/';
-            return;
-        }
-        dispatch(module.exports.setSession(body));
-        dispatch(module.exports.setStatus(module.exports.Status.FETCHED));
+    return new Promise((resolve, reject) => (
+        requestSession(resolve, reject).then(body => {
+            handleSessionResponse(dispatch, body);
+        }, err => {
+            dispatch(module.exports.setSessionError(err));
+        })
+    ));
+});
 
-        // get the permissions from the updated session
-        dispatch(permissionsActions.storePermissions(body.permissions));
-        if (typeof body.user !== 'undefined') {
-            dispatch(messageCountActions.getCount(body.user.username));
-        }
-        return;
+module.exports.refreshSessionWithRetry = () => (dispatch => {
+    dispatch(module.exports.setStatus(module.exports.Status.FETCHING));
+    return new Promise((resolve, reject) => (
+        requestSessionWithRetry(resolve, reject, 4, 7500)
+    )).then(body => {
+        handleSessionResponse(dispatch, body);
+    }, err => {
+        dispatch(module.exports.setSessionError(err));
     });
 });

test/unit/components/join-flow.test.jsx

@@ -41,7 +41,7 @@ describe('JoinFlow', () => {
 
     beforeEach(() => {
         store = mockStore({sessionActions: {
-            refreshSession: jest.fn()
+            refreshSessionWithRetry: jest.fn()
         }});
     });
 
@@ -283,16 +283,31 @@ describe('JoinFlow', () => {
         expect(success).toEqual(false);
     });
 
-    test('handleRegistrationResponse when passed body with success', () => {
+    test('handleRegistrationResponse calls refreshSessionWithRetry() when passed body with success', done => {
         const props = {
-            refreshSession: jest.fn()
+            refreshSessionWithRetry: () => (new Promise(() => { // eslint-disable-line no-undef
+                done(); // ensures that joinFlowInstance.props.refreshSessionWithRetry() was called
+            }))
         };
         const joinFlowInstance = getJoinFlowWrapper(props).instance();
         joinFlowInstance.handleRegistrationResponse(null, responseBodySuccess, {statusCode: 200});
-        expect(joinFlowInstance.state.registrationError).toEqual(null);
-        expect(joinFlowInstance.props.refreshSession).toHaveBeenCalled();
-        expect(joinFlowInstance.state.step).toEqual(1);
-        expect(joinFlowInstance.state.waiting).toBeFalsy();
+    });
+
+    test('handleRegistrationResponse advances to next step when passed body with success', () => {
+        const props = {
+            refreshSessionWithRetry: () => (new Promise(resolve => { // eslint-disable-line no-undef
+                resolve();
+            }))
+        };
+        const joinFlowInstance = getJoinFlowWrapper(props).instance();
+        joinFlowInstance.handleRegistrationResponse(null, responseBodySuccess, {statusCode: 200});
+        process.nextTick(
+            () => {
+                expect(joinFlowInstance.state.registrationError).toEqual(null);
+                expect(joinFlowInstance.state.step).toEqual(1);
+                expect(joinFlowInstance.state.waiting).toBeFalsy();
+            }
+        );
     });
 
     test('handleRegistrationResponse when passed body with preset server error', () => {
@@ -306,7 +321,7 @@ describe('JoinFlow', () => {
 
     test('handleRegistrationResponse with failure response, with error fields missing', () => {
         const props = {
-            refreshSession: jest.fn()
+            refreshSessionWithRetry: jest.fn()
         };
         const joinFlowInstance = getJoinFlowWrapper(props).instance();
         const responseErr = null;
@@ -320,7 +335,7 @@ describe('JoinFlow', () => {
             statusCode: 200
         };
         joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj);
-        expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled();
+        expect(joinFlowInstance.props.refreshSessionWithRetry).not.toHaveBeenCalled();
         expect(joinFlowInstance.state.registrationError).toEqual({
             errorAllowsTryAgain: false,
             errorMsg: null
@@ -339,7 +354,7 @@ describe('JoinFlow', () => {
 
     test('handleRegistrationResponse with failure response, with no text explanation', () => {
         const props = {
-            refreshSession: jest.fn()
+            refreshSessionWithRetry: jest.fn()
         };
         const joinFlowInstance = getJoinFlowWrapper(props).instance();
         const responseErr = null;
@@ -352,7 +367,7 @@ describe('JoinFlow', () => {
             statusCode: 200
         };
         joinFlowInstance.handleRegistrationResponse(responseErr, responseBody, responseObj);
-        expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled();
+        expect(joinFlowInstance.props.refreshSessionWithRetry).not.toHaveBeenCalled();
         expect(joinFlowInstance.state.registrationError).toEqual({
             errorAllowsTryAgain: false,
             errorMsg: null
@@ -369,11 +384,11 @@ describe('JoinFlow', () => {
 
     test('handleRegistrationResponse when passed status 400', () => {
         const props = {
-            refreshSession: jest.fn()
+            refreshSessionWithRetry: jest.fn()
         };
         const joinFlowInstance = getJoinFlowWrapper(props).instance();
         joinFlowInstance.handleRegistrationResponse({}, responseBodyMultipleErrs, {statusCode: 400});
-        expect(joinFlowInstance.props.refreshSession).not.toHaveBeenCalled();
+        expect(joinFlowInstance.props.refreshSessionWithRetry).not.toHaveBeenCalled();
         expect(joinFlowInstance.state.registrationError).toEqual({
             errorAllowsTryAgain: true
         });

test/unit/lib/session.test.js

+describe('session library', () => {
+    // respond to session requests with empty session object
+    let sessionNoUser = jest.fn((opts, callback) => {
+        callback(null, {}, {statusCode: 200});
+    });
+    // respond to session requests with session object that indicates
+    // successfully logged-in user
+    let sessionYesUser = jest.fn((opts, callback) => {
+        callback(null, {user: {username: 'test_username'}}, {statusCode: 200});
+    });
+    // respond to first two requests with empty session object; after that,
+    // respond with user in object
+    let sessionNoThenYes = jest.fn((opts, callback) => {
+        if (sessionNoThenYes.mock.calls.length <= 2) {
+            callback(null, {}, {statusCode: 200});
+        } else {
+            callback(null, {user: {username: 'test_username'}}, {statusCode: 200});
+        }
+    });
+    // respond to session requests with response code 404, indicating no session
+    // found for that user
+    let sessionNotFound = jest.fn((opts, callback) => {
+        callback(null, null, {statusCode: 404});
+    });
+    // respond to session requests with response code 503, indicating connection failure
+    let sessionConnectFailure = jest.fn((opts, callback) => {
+        callback(null, null, {statusCode: 503});
+    });
+
+    // by changing whichMockAPIRequest, we can simulate different api responses
+    let whichMockAPIRequest = null;
+    let mockAPIRequest = (opts, callback) => {
+        whichMockAPIRequest(opts, callback);
+    };
+
+    // mock lib/api.js, and include our mocked version in lib/session.js
+    jest.mock('../../../src/lib/api', () => {
+        return mockAPIRequest;
+    });
+    const sessionLib = require('../../../src/lib/session'); // eslint-disable-line global-require
+
+    afterEach(() => {
+        jest.clearAllMocks();
+    });
+
+    test('requestSession can call api 1 time, when session found', done => {
+        whichMockAPIRequest = sessionYesUser;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSession(resolve, reject);
+        }).then(body => {
+            expect(sessionYesUser).toHaveBeenCalledTimes(1);
+            expect(body).toEqual({user: {username: 'test_username'}});
+            done();
+        });
+    });
+
+    test('requestSession can call api 1 time, when session not found', done => {
+        whichMockAPIRequest = sessionNoUser;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSession(resolve, reject);
+        }).then(body => {
+            expect(sessionNoUser).toHaveBeenCalledTimes(1);
+            expect(body).toEqual({});
+            done();
+        });
+    });
+
+    test('requestSessionWithRetry can call api once', done => {
+        whichMockAPIRequest = sessionNoUser;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSessionWithRetry(resolve, reject, 0, 0);
+        }).then(() => {
+            expect(sessionNoUser).toHaveBeenCalledTimes(1);
+            done();
+        });
+    });
+
+    test('requestSessionWithRetry can call api multiple times', done => {
+        whichMockAPIRequest = sessionNoUser;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSessionWithRetry(resolve, reject, 2, 0);
+        }).then(() => {
+            expect(sessionNoUser).toHaveBeenCalledTimes(3);
+            done();
+        });
+    });
+
+    test('requestSessionWithRetry respects total delay time param within a reasonable tolerance', done => {
+        whichMockAPIRequest = sessionNoUser;
+        const startTime = new Date().getTime();
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSessionWithRetry(resolve, reject, 2, 2500);
+        }).then(() => {
+            const endTime = new Date().getTime();
+            expect(endTime - startTime > 2000).toBeTruthy();
+            expect(endTime - startTime < 3000).toBeTruthy();
+            done();
+        });
+    });
+
+    test('requestSessionWithRetry will retry if no user found, then stop when user is found', done => {
+        whichMockAPIRequest = sessionNoThenYes;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSessionWithRetry(resolve, reject, 4, 3000);
+        }).then(body => {
+            expect(body).toEqual({user: {username: 'test_username'}});
+            expect(sessionNoThenYes).toHaveBeenCalledTimes(3);
+            done();
+        });
+    });
+
+    test('requestSessionWithRetry handles session not found as immediate error', done => {
+        whichMockAPIRequest = sessionNotFound;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSessionWithRetry(resolve, reject, 2, 0);
+        }).then(() => {}, err => {
+            expect(err).toBeFalsy();
+            done();
+        });
+    });
+
+    test('requestSessionWithRetry handles connection failure by retrying', done => {
+        whichMockAPIRequest = sessionConnectFailure;
+        new Promise((resolve, reject) => { // eslint-disable-line no-undef
+            sessionLib.requestSessionWithRetry(resolve, reject, 2, 0);
+        }).then(body => {
+            expect(sessionConnectFailure).toHaveBeenCalledTimes(3);
+            expect(body).toBeFalsy();
+            done();
+        });
+    });
+
+});