diff --git a/src/github-api.ts b/src/github-api.ts index e761b9688..8db3b5e22 100644 --- a/src/github-api.ts +++ b/src/github-api.ts @@ -150,6 +150,29 @@ export interface ReleaseHistory { export type ReleaseIteratorOptions = ScmReleaseIteratorOptions; export const MAX_SLEEP_SECONDS = 20; + +const TRANSIENT_GRAPHQL_STATUSES = [500, 502, 503, 504]; +const TRANSIENT_GRAPHQL_MESSAGES = [ + 'something went wrong while executing your query', + 'respond to your request in time', +]; + +export function isTransientGraphqlError(err: unknown): boolean { + const {status, errors} = (err ?? {}) as { + status?: number; + errors?: {message?: string}[]; + }; + if (typeof status === 'number') { + return TRANSIENT_GRAPHQL_STATUSES.includes(status); + } + return ( + Array.isArray(errors) && + errors.some(e => + TRANSIENT_GRAPHQL_MESSAGES.some(m => e.message?.toLowerCase().includes(m)) + ) + ); +} + export const MAX_ISSUE_BODY_SIZE = 65536; export class GitHubApi { @@ -263,7 +286,7 @@ export class GitHubApi { } this.logger.trace('no GraphQL response, retrying'); } catch (err) { - if ((err as GitHubAPIError).status !== 502) { + if (!isTransientGraphqlError(err)) { throw err; } if (maxRetries === 0) { @@ -271,7 +294,7 @@ export class GitHubApi { throw err; } this.logger.info( - `received 502 error, ${maxRetries} attempts remaining` + `received transient GraphQL error, ${maxRetries} attempts remaining` ); if (typeof opts.num === 'number') { if (maxRetries === 1) { diff --git a/src/github.ts b/src/github.ts index a83fd7c55..5892bc179 100644 --- a/src/github.ts +++ b/src/github.ts @@ -32,7 +32,11 @@ import {ReleasePullRequest} from './release-pull-request'; import {Update} from './update'; import {Release} from './release'; import {ROOT_PROJECT_PATH} from './manifest'; -import {GitHubApi, GitHubCreateOptions} from './github-api'; +import { + GitHubApi, + GitHubCreateOptions, + isTransientGraphqlError, +} from './github-api'; import {signoffCommitMessage} from './util/signoff-commit-message'; import { RepositoryFileCache, @@ -470,7 +474,7 @@ export class GitHub implements Scm { } this.logger.trace('no GraphQL response, retrying'); } catch (err) { - if ((err as GitHubAPIError).status !== 502) { + if (!isTransientGraphqlError(err)) { throw err; } if (maxRetries === 0) { @@ -478,7 +482,7 @@ export class GitHub implements Scm { throw err; } this.logger.info( - `received 502 error, ${maxRetries} attempts remaining` + `received transient GraphQL error, ${maxRetries} attempts remaining` ); if (typeof opts.num === 'number') { if (maxRetries === 1) { diff --git a/test/github.ts b/test/github.ts index c28be0617..dc6020cf2 100644 --- a/test/github.ts +++ b/test/github.ts @@ -24,7 +24,11 @@ import * as sinon from 'sinon'; import * as codeSuggester from '../src/util/code-suggester'; import {GitHub, GitHubRelease} from '../src/github'; -import {GitHubApi, GH_API_URL} from '../src/github-api'; +import { + GitHubApi, + GH_API_URL, + isTransientGraphqlError, +} from '../src/github-api'; import {PullRequest} from '../src/pull-request'; import {TagName} from '../src/util/tag-name'; import {Version} from '../src/version'; @@ -1264,5 +1268,174 @@ describe('GitHub', () => { realClearInterval(tickInterval); req.done(); }); + + it('should retry a transient GraphQL error returned with HTTP 200', async () => { + let attempt = 0; + req = nock('https://api.github.com') + .post('/graphql') + .times(2) + .reply(() => { + attempt++; + if (attempt === 1) { + return [ + 200, + { + data: null, + errors: [ + {message: 'Something went wrong while executing your query.'}, + ], + }, + ]; + } + return [ + 200, + { + data: { + repository: { + ref: { + target: { + history: { + nodes: [], + pageInfo: {hasNextPage: false, endCursor: null}, + }, + }, + }, + }, + }, + }, + ]; + }); + + const promise = github.commitsSince('main', () => false, { + batchSize: 100, + }); + const tickInterval = realSetInterval(() => { + clock.tick(10000); + }, 10); + try { + await promise; + } finally { + realClearInterval(tickInterval); + } + expect(attempt).to.equal(2); + req.done(); + }); + + for (const status of [500, 503, 504]) { + it(`should retry a transient HTTP ${status} GraphQL error`, async () => { + let attempt = 0; + req = nock('https://api.github.com') + .post('/graphql') + .times(2) + .reply(() => { + attempt++; + if (attempt === 1) { + return [status, {message: 'Server Error'}]; + } + return [ + 200, + { + data: { + repository: { + ref: { + target: { + history: { + nodes: [], + pageInfo: {hasNextPage: false, endCursor: null}, + }, + }, + }, + }, + }, + }, + ]; + }); + + const promise = github.commitsSince('main', () => false, { + batchSize: 100, + }); + const tickInterval = realSetInterval(() => { + clock.tick(10000); + }, 10); + try { + await promise; + } finally { + realClearInterval(tickInterval); + } + expect(attempt).to.equal(2); + req.done(); + }); + } + + it('should not retry a GraphQL error that is not transient', async () => { + let attempt = 0; + req = nock('https://api.github.com') + .post('/graphql') + .reply(() => { + attempt++; + return [ + 200, + { + data: null, + errors: [{message: "Field 'foo' doesn't exist on type 'Query'"}], + }, + ]; + }); + + let thrown: unknown; + try { + await github.commitsSince('main', () => false, {batchSize: 100}); + } catch (err) { + thrown = err; + } + expect(thrown).to.not.be.undefined; + expect(attempt).to.equal(1); + req.done(); + }); + }); + + describe('isTransientGraphqlError', () => { + for (const status of [500, 502, 503, 504]) { + it(`should treat HTTP ${status} as transient`, () => { + expect(isTransientGraphqlError({status})).to.be.true; + }); + } + + for (const status of [401, 403, 404, 422, 429]) { + it(`should not treat HTTP ${status} as transient`, () => { + expect(isTransientGraphqlError({status})).to.be.false; + }); + } + + it('should treat the known GraphQL error messages as transient', () => { + expect( + isTransientGraphqlError({ + errors: [ + {message: 'Something went wrong while executing your query.'}, + ], + }) + ).to.be.true; + expect( + isTransientGraphqlError({ + errors: [{message: "We couldn't respond to your request in time."}], + }) + ).to.be.true; + }); + + it('should not treat other GraphQL errors as transient', () => { + expect( + isTransientGraphqlError({ + errors: [{message: "Field 'foo' doesn't exist on type 'Query'"}], + }) + ).to.be.false; + }); + + it('should not treat empty or odd input as transient', () => { + expect(isTransientGraphqlError(undefined)).to.be.false; + expect(isTransientGraphqlError(null)).to.be.false; + expect(isTransientGraphqlError(new Error('boom'))).to.be.false; + expect(isTransientGraphqlError({errors: []})).to.be.false; + expect(isTransientGraphqlError({errors: [{}]})).to.be.false; + }); }); });