Skip to content

Commit

Permalink
fix(framework-core): retry mechanism for when forked git object isn't…
Browse files Browse the repository at this point in the history
… quite ready (#78)

* fix(bug): retry mechanism for when forked git object isn't quite ready

* docs

* lint fix

* lint

* commit message

* remove deadline logic
  • Loading branch information
TomKristie committed Aug 13, 2020
1 parent 44c13c6 commit 326145f
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 15 deletions.
8 changes: 5 additions & 3 deletions package.json
Expand Up @@ -43,11 +43,15 @@
"dependencies": {
"@octokit/rest": "^18.0.1",
"@types/yargs": "^15.0.5",
"async-retry": "^1.3.1",
"glob": "^7.1.6",
"pino": "^6.3.2",
"yargs": "^15.4.1"
},
"devDependencies": {
"@microsoft/api-documenter": "^7.8.10",
"@microsoft/api-extractor": "^7.8.10",
"@types/async-retry": "^1.4.2",
"@types/chai": "^4.2.11",
"@types/mocha": "^8.0.0",
"@types/node": "^14.0.20",
Expand All @@ -70,8 +74,6 @@
"ts-loader": "^8.0.0",
"typescript": "^3.9.5",
"webpack": "^4.41.2",
"webpack-cli": "^3.3.10",
"@microsoft/api-documenter": "^7.8.10",
"@microsoft/api-extractor": "^7.8.10"
"webpack-cli": "^3.3.10"
}
}
10 changes: 7 additions & 3 deletions src/github-handler/fork-handler.ts
Expand Up @@ -18,7 +18,8 @@ import {logger} from '../logger';

/**
* Fork the GitHub owner's repository.
* Returns the fork owner and fork repo if successful. Otherwise throws error.
* Returns the fork owner and fork repo when the fork creation request to GitHub succeeds.
* Otherwise throws error.
*
* If fork already exists no new fork is created, no error occurs, and the existing Fork data is returned
* with the `updated_at` + any historical repo changes.
Expand All @@ -37,11 +38,14 @@ async function fork(
repo: upstream.repo,
})
).data;
logger.info(`Fork successfully exists on ${upstream.repo}`);
return {
const origin: RepoDomain = {
repo: forkedRepo.name,
owner: forkedRepo.owner.login,
};
logger.info(
`Create fork request was successful for ${origin.owner}/${origin.repo}`
);
return origin;
} catch (err) {
logger.error('Error when forking');
throw Error(err.toString());
Expand Down
38 changes: 31 additions & 7 deletions src/index.ts
Expand Up @@ -25,12 +25,23 @@ import {Octokit} from '@octokit/rest';
import {Logger} from 'pino';
import {logger, setupLogger} from './logger';
import {addPullRequestDefaults} from './default-options-handler';
import * as retry from 'async-retry';

/**
* Make a new GitHub Pull Request with a set of changes applied on top of primary branch HEAD.
* The changes are committed into a new branch based on the upstream repository options using the authenticated Octokit account.
* Then a Pull Request is made from that branch.
* If the upstream
*
* Also throws error if git data from the fork is not ready in 5 minutes.
*
* From the docs
* https://developer.github.com/v3/repos/forks/#create-a-fork
* """
* Forking a Repository happens asynchronously.
* You may have to wait a short period of time before you can access the git objects.
* If this takes longer than 5 minutes, be sure to contact GitHub Support or GitHub Premium Support.
* """
*
* If changes are empty then the workflow will not run.
* Rethrows an HttpError if Octokit GitHub API returns an error. HttpError Octokit access_token and client_secret headers redact all sensitive information.
* @param {Octokit} octokit The authenticated octokit instance, instantiated with an access token having permissiong to create a fork on the target repository
Expand Down Expand Up @@ -65,13 +76,26 @@ async function createPullRequest(
...origin,
branch: gitHubConfigs.branch,
};
const refHeadSha: string = await handler.branch(
octokit,
origin,
upstream,
originBranch.branch,
gitHubConfigs.primary
const refHeadSha: string = await retry(
async () =>
await handler.branch(
octokit,
origin,
upstream,
originBranch.branch,
gitHubConfigs.primary
),
{
retries: 5,
factor: 2.8411, // https://www.wolframalpha.com/input/?i=Sum%5B3000*x%5Ek%2C+%7Bk%2C+0%2C+4%7D%5D+%3D+5+*+60+*+1000
minTimeout: 3000,
randomize: false,
onRetry: () => {
logger.info('Retrying at a later time...');
},
}
);

await handler.commitAndPush(
octokit,
refHeadSha,
Expand Down
16 changes: 14 additions & 2 deletions test/main-make-pr.ts
Expand Up @@ -19,11 +19,12 @@ import * as sinon from 'sinon';
import {Changes, FileData, CreatePullRequestUserOptions} from '../src/types';
import {Octokit} from '@octokit/rest';
import * as proxyquire from 'proxyquire';

import * as retry from 'async-retry';
before(() => {
setup();
});

/* eslint-disable @typescript-eslint/no-unused-vars */
// tslint:disable:no-unused-expression
// .true triggers ts-lint failure, but is valid chai
describe('Make PR main function', () => {
Expand Down Expand Up @@ -144,7 +145,6 @@ describe('Make PR main function', () => {
});
it('Passes up the error message with a throw when create branch helper fails', async () => {
// setup

const stubHelperHandlers = {
fork: (octokit: Octokit, upstream: {owner: string; repo: string}) => {
expect(upstream.owner).equals(upstreamOwner);
Expand All @@ -160,6 +160,18 @@ describe('Make PR main function', () => {
};
const stubMakePr = proxyquire.noCallThru()('../src/', {
'./github-handler': stubHelperHandlers,
'async-retry': async (
fn: Function,
options: {[index: string]: unknown}
) => {
expect(options.retries).equals(5);
expect(options.factor).equals(2.8411);
expect(options.minTimeout).equals(3000);
expect(options.randomize).equals(false);
await retry(() => fn(), {
retries: 0,
});
},
});
try {
await stubMakePr.createPullRequest(octokit, changes, options);
Expand Down

0 comments on commit 326145f

Please sign in to comment.