From 71dbfedc05f4882028acdf415b04e6532f149649 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ari=20Perkki=C3=B6?= <ari.perkkio@gmail.com>
Date: Wed, 25 Aug 2021 19:38:54 +0300
Subject: [PATCH] fix: handle GitHub API errors (#19)

* test: add failing test for #18

* fix: handle API errors

- Attempt to retry failed requests 5x times
---
 src/github-client.ts             | 68 +++++++++++++++++++++-----------
 test/__mocks__/GithubAPI.mock.ts |  7 +++-
 test/github-client.test.ts       | 38 ++++++++++++++++++
 3 files changed, 90 insertions(+), 23 deletions(-)

diff --git a/src/github-client.ts b/src/github-client.ts
index b59960e..a414a80 100644
--- a/src/github-client.ts
+++ b/src/github-client.ts
@@ -18,10 +18,27 @@ try {
 class GithubClient {
     private octokit: ReturnType<typeof getOctokit>;
 
+    /** Indicates how many times failed request is retried */
+    private MAX_RETRIES = 5;
+
     constructor() {
         this.octokit = getOctokit(githubToken);
     }
 
+    private async requestAndRetry<T>(request: () => Promise<T>): Promise<T> {
+        for (let retryCount = 1; retryCount <= this.MAX_RETRIES; retryCount++) {
+            try {
+                return await request();
+            } catch (error) {
+                core.info(
+                    `Request failed. Retrying ${retryCount}/${this.MAX_RETRIES}.`
+                );
+            }
+        }
+
+        return await request();
+    }
+
     /**
      * Post results to existing issue as comment or create new issue using
      * results as body
@@ -35,25 +52,30 @@ class GithubClient {
         }
 
         core.info(`Reusing existing issue #${existingIssue}`);
-        await this.octokit.issues.createComment({
-            owner: context.repo.owner,
-            repo: context.repo.repo,
-            issue_number: existingIssue,
-            body,
-        });
+
+        await this.requestAndRetry(() =>
+            this.octokit.issues.createComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                issue_number: existingIssue,
+                body,
+            })
+        );
     }
 
     private async getExistingIssue(): Promise<number | undefined> {
-        const response = await this.octokit.search.issuesAndPullRequests({
-            sort: 'created',
-            order: 'desc',
-            q: [
-                `${issueTitle} in:title`,
-                'is:issue',
-                'is:open',
-                `repo:${context.repo.owner}/${context.repo.repo}`,
-            ].join(' '),
-        });
+        const response = await this.requestAndRetry(() =>
+            this.octokit.search.issuesAndPullRequests({
+                sort: 'created',
+                order: 'desc',
+                q: [
+                    `${issueTitle} in:title`,
+                    'is:issue',
+                    'is:open',
+                    `repo:${context.repo.owner}/${context.repo.repo}`,
+                ].join(' '),
+            })
+        );
 
         const { items } = response.data;
         core.info(`Found ${items.length} open issues with title ${issueTitle}`);
@@ -64,12 +86,14 @@ class GithubClient {
     }
 
     private async createIssue(body: string): Promise<void> {
-        await this.octokit.issues.create({
-            owner: context.repo.owner,
-            repo: context.repo.repo,
-            title: issueTitle,
-            body,
-        });
+        await this.requestAndRetry(() =>
+            this.octokit.issues.create({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                title: issueTitle,
+                body,
+            })
+        );
     }
 }
 
diff --git a/test/__mocks__/GithubAPI.mock.ts b/test/__mocks__/GithubAPI.mock.ts
index 94e4855..9207901 100644
--- a/test/__mocks__/GithubAPI.mock.ts
+++ b/test/__mocks__/GithubAPI.mock.ts
@@ -1,8 +1,9 @@
 import { setupServer } from 'msw/node';
 import { rest } from 'msw';
 
-const API_URL = 'https://api.github.com';
+export const API_URL = 'https://api.github.com';
 export const mockNoExistingIssues = jest.fn().mockReturnValue(false);
+export const mockApiError = jest.fn().mockReturnValue(false);
 export const onComment = jest.fn();
 export const onIssueCreated = jest.fn();
 export const expectedIssueNumber = 999;
@@ -29,6 +30,10 @@ export default setupServer(
             return res(ctx.status(404));
         }
 
+        if (mockApiError()) {
+            return res(ctx.status(500));
+        }
+
         return res(
             ctx.json({
                 // Use mockNoExistingIssues to temporarily simulate "No issues" -case
diff --git a/test/github-client.test.ts b/test/github-client.test.ts
index 318621d..a1dc2e0 100644
--- a/test/github-client.test.ts
+++ b/test/github-client.test.ts
@@ -4,6 +4,7 @@ import {
     onIssueCreated,
     mockNoExistingIssues,
     expectedIssueNumber,
+    mockApiError,
 } from './__mocks__/GithubAPI.mock';
 
 const body = 'mock-comment-body';
@@ -40,4 +41,41 @@ describe('github-client', () => {
         });
         expect(onIssueCreated).not.toHaveBeenCalled();
     });
+
+    test('should recover from 5x API errors', async () => {
+        // Request should fail 5 times
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+
+        mockNoExistingIssues.mockReturnValueOnce(true);
+
+        await GithubClient.postResults(body);
+
+        expect(onIssueCreated).toHaveBeenCalledWith({
+            owner: 'mock-owner',
+            repo: 'mock-repo',
+            body: {
+                body,
+                title: 'mock-issue-title',
+            },
+        });
+    });
+
+    test('should fail request after 6x failures', async () => {
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+        mockApiError.mockReturnValueOnce(true);
+
+        mockNoExistingIssues.mockReturnValueOnce(true);
+
+        await expect(
+            GithubClient.postResults(body)
+        ).rejects.toMatchInlineSnapshot(`[HttpError]`);
+    });
 });