From 2f14f264c930a54c2cf4888559ee9e7ec179aa70 Mon Sep 17 00:00:00 2001 From: Bob Wombat Hogg Date: Tue, 7 Feb 2017 12:14:55 -0800 Subject: [PATCH] Automatically retry logins (#113) --- index.js | 36 +++++++++++++++++++---- tests/index.js | 79 ++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 12 deletions(-) diff --git a/index.js b/index.js index bfdeb2e..6e2ec1b 100644 --- a/index.js +++ b/index.js @@ -550,6 +550,9 @@ class UserAgent { this.version = version; this._cacheMe(); + + this._maxSessionAttempts = 3; + this._setState('sessionAttempt', 0); } /** @@ -575,19 +578,40 @@ class UserAgent { /** * Log this user agent in. - * After calling this function, - * `cachedAgents[this.username]._state.loginPromise` is set to a promise - * that, when resolved, will verify that the OAuth token is available. + * If the login is unsuccessful, it is retried a maximum of two additional + * times, after which it throws an error. + * + * @return {ChakramPromise} A promise resolving to the result of the login + * request. * * @private */ _login = () => { - this._setState('loginPromise', utils.login({ + let loginPromise = this._getState('loginPromise'); + if (loginPromise) { + return loginPromise; + } + + let sessionAttempt = this._getState('sessionAttempt') + 1; + this._setState('sessionAttempt', sessionAttempt); + if (sessionAttempt > this._maxSessionAttempts) { + throw new Error('Max number of login attempts exceeded for user: ' + this.username); + } + + loginPromise = utils.login({ username: this.username, password: this.password, version: this.version, xthorn: 'Agent', - }).then(this._updateAuthState)); + }).then((response) => { + this._updateAuthState(response); + this._setState('sessionAttempt', 0); + }).catch(() => { + this._setState('loginPromise', null); + return this._login(); + }); + this._setState('loginPromise', loginPromise); + return loginPromise; }; /** @@ -604,7 +628,7 @@ class UserAgent { _requestSkeleton = (chakramMethod, args) => { args[0] = utils.constructUrl(this.version, args[0]); - return this._getState('loginPromise').then(() => { + return this._login().then(() => { // must wait for login promise to resolve or else OAuth-Token may not be available let paramIndex = args.length - 1; // FIXME: eventually will want to support multiple types of headers diff --git a/tests/index.js b/tests/index.js index 59139a6..75b51a7 100644 --- a/tests/index.js +++ b/tests/index.js @@ -641,14 +641,12 @@ describe('Thorn', () => { }); describe('Agent', () => { - beforeEach(() => { - nock(process.env.THORN_SERVER_URL) - .post(isTokenReq) - .reply(200, ACCESS); - }); - describe('as', () => { it('should return an Agent with cached username and password', () => { + nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(200, ACCESS); + let myAgent = Agent.as(process.env.THORN_ADMIN_USERNAME); expect(myAgent.username).to.equal(process.env.THORN_ADMIN_USERNAME); @@ -656,6 +654,10 @@ describe('Thorn', () => { }); it('should return the same agent if called twice with the same username', () => { + nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(200, ACCESS); + let agent1 = Agent.as(process.env.THORN_ADMIN_USERNAME); let agent2 = Agent.as(process.env.THORN_ADMIN_USERNAME); @@ -669,12 +671,73 @@ describe('Thorn', () => { it('should throw an error if given username is not found', () => { expect(() => Agent.as('nonexistent')).to.throw('No credentials available for user: nonexistent'); }); + + it('should retry login 2 times', function*() { + let server = nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(401) + .post(isTokenReq) + .reply(401) + .post(isTokenReq) + .reply(200, ACCESS) + .get(/not\/real\/endpoint/) + .reply(200, { fake: 'data' }); + + let myAgent = Agent.as(process.env.THORN_ADMIN_USERNAME); + + // note: Agent.as does not expose a Promise, so have to wait on an arbitrary request + yield myAgent.get('not/real/endpoint'); + + expect(server.isDone()).to.be.true; + }); + + it('should give up after 3 login attempts', function*() { + nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(401) + .post(isTokenReq) + .reply(401) + .post(isTokenReq) + .reply(401); + + let myAgent = Agent.as(process.env.THORN_ADMIN_USERNAME); + + // note: Agent.as does not expose a Promise, so have to wait on an arbitrary request + yield myAgent.get('not/real/endpoint').catch((e) => { + let msg = 'Max number of login attempts exceeded for user: ' + process.env.THORN_ADMIN_USERNAME; + expect(e.message).to.equal(msg); + }); + }); + + // Apache sometimes sends 200 responses with empty bodies on thread death, + // which is interpreted by Chakram as ECONNRESET and results in an empty + // response.response object. Ensure this scenario is handled appropriately. + it('should retry login on empty response body', function*() { + let server = nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(200) + .post(isTokenReq) + .reply(200, ACCESS) + .get(/not\/real\/endpoint/) + .reply(200, { fake: 'data' }); + + let myAgent = Agent.as(process.env.THORN_ADMIN_USERNAME); + + // note: Agent.as does not expose a Promise, so have to wait on an arbitrary request + yield myAgent.get('not/real/endpoint'); + + expect(server.isDone()).to.be.true; + }); }); describe('on', () => { let myAgent; beforeEach(() => { + nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(200, ACCESS); + myAgent = Agent.as(process.env.THORN_ADMIN_USERNAME); }); @@ -702,6 +765,10 @@ describe('Thorn', () => { } beforeEach(() => { + nock(process.env.THORN_SERVER_URL) + .post(isTokenReq) + .reply(200, ACCESS); + myAgent = Agent.as(process.env.THORN_ADMIN_USERNAME); endpoint = 'not/real/endpoint'; });