Skip to content

Commit fd8272b

Browse files
committed
fix(ng-dev): allow for graceful retries when GitHub API is outdated
The error and aggresive failing, even with a single retry caused a few issues in the past. This commit tries to make this more robust and failure tolerant.
1 parent 9194858 commit fd8272b

File tree

3 files changed

+91
-11
lines changed

3 files changed

+91
-11
lines changed

ng-dev/release/publish/actions.ts

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -744,16 +744,24 @@ export abstract class ReleaseAction {
744744
branch: string,
745745
version: semver.SemVer,
746746
previousSha: string,
747-
isRetry = false,
748747
): Promise<string> {
749-
try {
748+
let latestSha: string | null = null;
749+
750+
// Support re-checking as many times as needed. Often times the GitHub API
751+
// is not up-to-date and we don't want to exit the release script then.
752+
while (latestSha === null) {
750753
const commit = await this.getLatestCommitOfBranch(branch);
754+
751755
// Ensure the latest commit in the publish branch is the bump commit.
752756
if (!commit.commit.message.startsWith(getCommitMessageForRelease(version))) {
753757
/** The shortened sha of the commit for usage in the error message. */
754758
const sha = commit.sha.slice(0, 8);
755759
Log.error(` ✘ Latest commit (${sha}) in "${branch}" branch is not a staging commit.`);
756760
Log.error(' Please make sure the staging pull request has been merged.');
761+
762+
if (await Prompt.confirm({message: `Do you want to re-try?`, default: true})) {
763+
continue;
764+
}
757765
throw new FatalReleaseActionError();
758766
}
759767

@@ -762,16 +770,17 @@ export abstract class ReleaseAction {
762770
if (commit.parents[0].sha !== previousSha) {
763771
Log.error(` ✘ Unexpected additional commits have landed while staging the release.`);
764772
Log.error(' Please revert the bump commit and retry, or cut a new version on top.');
773+
774+
if (await Prompt.confirm({message: `Do you want to re-try?`, default: true})) {
775+
continue;
776+
}
765777
throw new FatalReleaseActionError();
766778
}
767779

768-
return commit.sha;
769-
} catch (e: unknown) {
770-
if (isRetry) {
771-
throw e;
772-
}
773-
return this._getAndValidateLatestCommitForPublishing(branch, version, previousSha, true);
780+
latestSha = commit.sha;
774781
}
782+
783+
return latestSha;
775784
}
776785

777786
// TODO: Remove this check and run it as part of common release validation.

ng-dev/release/publish/test/common.spec.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,61 @@ describe('common release action logic', () => {
358358
);
359359
});
360360

361+
it('should allow for re-trying when GitHub API is not reflecting latest commit', async () => {
362+
const {repo, instance, builtPackagesWithInfo, promptConfirmSpy} =
363+
setupReleaseActionForTesting(DelegateTestAction, baseReleaseTrains);
364+
const {version, branchName} = baseReleaseTrains.latest;
365+
const tagName = version.format();
366+
367+
repo
368+
.expectBranchRequest(branchName, {
369+
sha: 'THE_ONE_BEFORE_STAGING_SHA',
370+
parents: [{sha: 'bla'}],
371+
commit: {message: `build: some other change`},
372+
})
373+
.expectTagToBeCreated(tagName, 'STAGING_SHA')
374+
.expectReleaseToBeCreated(version.toString(), tagName, true);
375+
376+
spyOn(Log, 'error');
377+
378+
const promptPending = withResolverPromise<void>();
379+
const promptResolveFns: Array<(val: boolean) => void> = [];
380+
promptConfirmSpy.and.callFake(() => {
381+
promptPending.resolve();
382+
return new Promise<boolean>((resolve) => promptResolveFns.push(resolve));
383+
});
384+
385+
const publishProcess = instance.testPublish(
386+
builtPackagesWithInfo,
387+
version,
388+
branchName,
389+
'BEFORE_STAGING_SHA',
390+
'latest',
391+
);
392+
393+
// Wait for re-try prompt.
394+
await promptPending.promise;
395+
396+
// Re-mock the branch request, with the correct info now.
397+
repo.expectBranchRequest(branchName, {
398+
sha: 'STAGING_SHA',
399+
parents: [{sha: 'BEFORE_STAGING_SHA'}],
400+
commit: {message: `release: cut the v${version} release`},
401+
});
402+
403+
// Reset logs to see if we pass gracefully then.
404+
(Log.error as unknown as jasmine.Spy).calls.reset();
405+
406+
// Kick off the re-try.
407+
promptResolveFns.shift()!(true);
408+
409+
// See if it completes now, without errors.
410+
expect(Log.error).toHaveBeenCalledTimes(0);
411+
412+
// Ensure publish finishes completely and gracefully.
413+
await publishProcess;
414+
});
415+
361416
it('should ensure that no new changes have landed after release staging has started', async () => {
362417
const {repo, instance, builtPackagesWithInfo} = setupReleaseActionForTesting(
363418
DelegateTestAction,
@@ -546,3 +601,19 @@ class MockReleaseNotes extends ReleaseNotes {
546601
super(ngDevConfig, version, commits, git);
547602
}
548603
}
604+
605+
// Polyfill for `Promise.withResolvers`.
606+
function withResolverPromise<T>(): {
607+
promise: Promise<T>;
608+
resolve: (v: T) => void;
609+
reject: (v: unknown) => void;
610+
} {
611+
let resolve!: (v: T) => void;
612+
let reject!: (v: unknown) => void;
613+
614+
const promise = new Promise<T>((res, rej) => {
615+
resolve = res;
616+
reject = rej;
617+
});
618+
return {promise, resolve, reject};
619+
}

ng-dev/utils/prompt.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,11 @@ import {confirm, input, checkbox, select, editor} from '@inquirer/prompts';
1313
* class to allow easier mocking management in test environments.
1414
*/
1515
export class Prompt {
16-
static confirm: typeof confirm = (
16+
static confirm = (
1717
// These are extractions of the PromptConfig from the inquirer types.
1818
_config: Parameters<typeof confirm>[0],
19-
_context: Parameters<typeof confirm>[1],
20-
) => {
19+
_context?: Parameters<typeof confirm>[1],
20+
): Promise<boolean> => {
2121
/** Config to use when creating a confirm prompt, changes the default to `false` instead of `true`. */
2222
const config = {
2323
default: false,

0 commit comments

Comments
 (0)