diff --git a/tools/@aws-cdk/prlint/linter-base.ts b/tools/@aws-cdk/prlint/linter-base.ts index 58b7988c4068e..3c9b4974653e4 100644 --- a/tools/@aws-cdk/prlint/linter-base.ts +++ b/tools/@aws-cdk/prlint/linter-base.ts @@ -168,6 +168,11 @@ export class PullRequestLinterBase { message, }); } catch (e: any) { + if (githubResponseErrors(e).includes('Can not dismiss a dismissed pull request review')) { + // Can happen because of race conditions. We already achieved the result we wanted, so ignore. + continue; + } + // This can fail with a "not found" for some reason throw new Error(`Dismissing review failed, user is probably not authorized: ${JSON.stringify(e, undefined, 2)}`); } @@ -386,4 +391,11 @@ export function mergeLinterActions(a: LinterActions, b: LinterActions): LinterAc function nonEmpty(xs: A[]): A[] | undefined { return xs.length > 0 ? xs : undefined; +} + +/** + * Return error messages from an exception thrown by GitHub + */ +function githubResponseErrors(e: Error): string[] { + return (e as any).response?.data?.errors ?? []; } \ No newline at end of file diff --git a/tools/@aws-cdk/prlint/test/linter-base.test.ts b/tools/@aws-cdk/prlint/test/linter-base.test.ts index d424e3a3b4662..950a6559c4916 100644 --- a/tools/@aws-cdk/prlint/test/linter-base.test.ts +++ b/tools/@aws-cdk/prlint/test/linter-base.test.ts @@ -21,7 +21,7 @@ const linter = new PullRequestLinterBase({ }); beforeEach(() => { - jest.clearAllMocks(); + jest.resetAllMocks(); octomock.pulls.get.mockReturnValue({ data: { @@ -34,6 +34,66 @@ beforeEach(() => { octomock.repos.listCommitStatusesForRef.mockReturnValue({ data: [] }); }); +test('ignore if dismissing reviews throws a specific "already dismissed" error', async () => { + // GIVEN + octomock.pulls.listReviews.mockReturnValue({ + data: [ + { + id: 1111, + user: { login: 'aws-cdk-automation' }, + state: 'CHANGES_REQUESTED', + body: 'some comment', + }, + ] + }); + octomock.pulls.dismissReview.mockRejectedValue({ + status: 422, + response: { + data: { + errors: [ + 'Can not dismiss a dismissed pull request review', + ], + }, + }, + }); + + // WHEN + await linter.executeActions({ + dismissPreviousReview: true, + }); + + // THEN: no error +}); + +test('throw if dismissing reviews throws any other error', async () => { + // GIVEN + octomock.pulls.listReviews.mockReturnValue({ + data: [ + { + id: 1111, + user: { login: 'aws-cdk-automation' }, + state: 'CHANGES_REQUESTED', + body: 'some comment', + }, + ] + }); + octomock.pulls.dismissReview.mockRejectedValue({ + status: 422, + response: { + data: { + errors: [ + 'Review not found', + ], + }, + }, + }); + + // THEN + await expect(linter.executeActions({ + dismissPreviousReview: true, + })).rejects.toThrow(/Dismissing review failed/); +}); + test('dismissing a review dismisses and changes the text of all previous reviews', async () => { // GIVEN octomock.pulls.listReviews.mockReturnValue({