From cb3c4ece38f87c1f87c062faccce9f436a01081a Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@keylocation.sg> Date: Fri, 4 Aug 2017 18:13:49 +0200 Subject: [PATCH] refactor: improve github retry logic (#616) 5 retries use settimeout retry on any 5xx error code --- lib/api/github.js | 15 +++++++++++++-- package.json | 2 +- test/api/github.spec.js | 16 +++++++++++++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lib/api/github.js b/lib/api/github.js index feafd3f738..490d79333a 100644 --- a/lib/api/github.js +++ b/lib/api/github.js @@ -1,12 +1,23 @@ let logger = require('../logger'); const ghGot = require('gh-got'); -async function ghGotRetry(path, opts, retries = 3) { +// istanbul ignore next +function sleep(ms) { + // eslint-disable-next-line promise/avoid-new + return new Promise(resolve => setTimeout(resolve, ms)); +} + +async function ghGotRetry(path, opts, retries = 5) { try { const res = await ghGot(path, opts); return res; } catch (err) { - if (err.statusCode === 502 && retries > 0) { + if (err.statusCode >= 500 && err.statusCode < 600 && retries > 0) { + logger.debug(`Retrying statusCode ${err.statusCode}`); + // istanbul ignore if + if (process.env.NODE_ENV !== 'test') { + await sleep(5000 / retries); + } return ghGotRetry(path, opts, retries - 1); } throw err; diff --git a/package.json b/package.json index 285dc8f6ab..a8bf4ff8c6 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "build": "yarn && npm run transpile && cp -R lib/config/templates dist/config", "heroku-push": "git push heroku master", "heroku-scheduler": "heroku addons:open scheduler", - "jest": "LOG_LEVEL=fatal jest", + "jest": "NODE_ENV=test LOG_LEVEL=fatal jest", "lint-fix": "eslint --fix lib/**/*.js test/**/*.js", "lint": "eslint lib/**/*.js test/**/*.js", "prepublishOnly": "npm run build", diff --git a/test/api/github.spec.js b/test/api/github.spec.js index ba85677974..e3d215a60f 100644 --- a/test/api/github.spec.js +++ b/test/api/github.spec.js @@ -93,7 +93,7 @@ describe('api/github', () => { } expect(err.statusCode).toBe(404); }); - it('should give up after 3 retries', async () => { + it('should give up after 5 retries', async () => { ghGot.mockImplementationOnce(() => Promise.reject({ statusCode: 502, @@ -104,6 +104,16 @@ describe('api/github', () => { statusCode: 502, }) ); + ghGot.mockImplementationOnce(() => + Promise.reject({ + statusCode: 500, + }) + ); + ghGot.mockImplementationOnce(() => + Promise.reject({ + statusCode: 500, + }) + ); ghGot.mockImplementationOnce(() => Promise.reject({ statusCode: 502, @@ -542,7 +552,7 @@ describe('api/github', () => { // getBranchProtection ghGot.mockImplementationOnce(() => Promise.reject({ - statusCode: 500, + statusCode: 600, }) ); return github.initRepo(...args); @@ -553,7 +563,7 @@ describe('api/github', () => { } catch (err) { e = err; } - expect(e.statusCode).toBe(500); + expect(e.statusCode).toBe(600); }); }); describe('setBaseBranch(branchName)', () => { -- GitLab