fix: handle GitHub API errors (#19)

* test: add failing test for #18 * fix: handle API errors - Attempt to retry failed requests 5x times
AriPerkkio · Aug 25, 2021 · 71dbfed · 71dbfed
1 parent 1cb8cb2
commit 71dbfed
Show file tree

Hide file tree

Showing 3 changed files with 90 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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]`);
+    });
 });