Merge pull request #179 from tiobe/32943-retry_mechanism

Added retry mechanism for all calls made within the action

package.json

@@ -33,16 +33,17 @@
     "@actions/core": "^1.10.1",
     "@actions/exec": "^1.1.1",
     "@actions/github": "^5.1.1",
+    "@actions/http-client": "^2.1.1",
+    "@octokit/plugin-retry": "^6.0.1",
+    "@octokit/request-error": "^5.0.1",
     "canonical-path": "^1.0.0",
     "compare-versions": "^6.1.0",
-    "proxy-agent": "^6.3.1",
     "semver": "^7.5.4",
     "underscore": "^1.13.6"
   },
   "devDependencies": {
     "@types/jest": "^29.5.5",
     "@types/node": "^20.8.0",
-    "@types/node-fetch": "^2.6.6",
     "@types/underscore": "^1.11.9",
     "@typescript-eslint/parser": "^6.7.3",
     "@vercel/ncc": "^0.38.0",

src/configuration.ts

@@ -1,6 +1,6 @@
 import { getBooleanInput, getInput, isDebug } from '@actions/core';
-import * as github from '@actions/github';
-import { ProxyAgent } from 'proxy-agent';
+import { context, getOctokit } from '@actions/github';
+import { HttpClient, HttpCodes } from '@actions/http-client';
 import { getTicsWebBaseUrlFromUrl } from './tics/api_helper';
 import { EOL } from 'os';
 
@@ -12,16 +12,16 @@ export const githubConfig = {
   basebranchname: process.env.GITHUB_BASE_REF ? process.env.GITHUB_BASE_REF : '',
   branchdir: process.env.GITHUB_WORKSPACE ? process.env.GITHUB_WORKSPACE : '',
   commitSha: process.env.GITHUB_SHA ? process.env.GITHUB_SHA : '',
-  eventName: github.context.eventName,
-  id: `${github.context.runId.toString()}-${process.env.GITHUB_RUN_ATTEMPT}`,
+  eventName: context.eventName,
+  id: `${context.runId.toString()}-${process.env.GITHUB_RUN_ATTEMPT}`,
   runnerOS: process.env.RUNNER_OS ? process.env.RUNNER_OS : '',
   pullRequestNumber: getPullRequestNumber(),
   debugger: isDebug()
 };
 
 function getPullRequestNumber() {
-  if (github.context.payload.pull_request) {
-    return github.context.payload.pull_request.number;
+  if (context.payload.pull_request) {
+    return context.payload.pull_request.number;
   } else if (process.env.PULL_REQUEST_NUMBER) {
     return parseInt(process.env.PULL_REQUEST_NUMBER);
   } else {
@@ -67,7 +67,26 @@ export const ticsConfig = {
   viewerUrl: getInput('viewerUrl')
 };
 
-export const octokit = github.getOctokit(ticsConfig.githubToken);
-export const requestInit = { agent: new ProxyAgent(), headers: {} };
+export const retryConfig = {
+  maxRetries: 10,
+  retryCodes: [HttpCodes.BadGateway, HttpCodes.ServiceUnavailable, HttpCodes.GatewayTimeout]
+};
+
+const ignoreSslError: boolean =
+  ticsConfig.hostnameVerification === '0' ||
+  ticsConfig.hostnameVerification === 'false' ||
+  ticsConfig.trustStrategy === 'self-signed' ||
+  ticsConfig.trustStrategy === 'all';
+
+export const octokit = getOctokit(
+  ticsConfig.githubToken,
+  { request: { retries: retryConfig.maxRetries, retryAfter: 5 } },
+  require('@octokit/plugin-retry').retry
+);
+export const httpClient = new HttpClient('tics-github-action', undefined, {
+  allowRetries: true,
+  maxRetries: retryConfig.maxRetries,
+  ignoreSslError: ignoreSslError
+});
 export const baseUrl = getTicsWebBaseUrlFromUrl(ticsConfig.ticsConfiguration);
 export const viewerUrl = ticsConfig.viewerUrl ? ticsConfig.viewerUrl.replace(/\/+$/, '') : baseUrl;

src/github/annotations.ts

@@ -2,6 +2,7 @@ import { logger } from '../helper/logger';
 import { githubConfig, octokit } from '../configuration';
 import { ReviewComment } from './interfaces';
 import { AnalysisResults, TicsReviewComment } from '../helper/interfaces';
+import { handleOctokitError } from '../helper/error';
 
 /**
  * Gets a list of all reviews posted on the pull request.
@@ -18,8 +19,7 @@ export async function getPostedReviewComments(): Promise<ReviewComment[]> {
     };
     response = await octokit.paginate(octokit.rest.pulls.listReviewComments, params);
   } catch (error: unknown) {
-    let message = 'reason unkown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.error(`Could not retrieve the review comments: ${message}`);
   }
   return response;
@@ -65,8 +65,7 @@ export function deletePreviousReviewComments(postedReviewComments: ReviewComment
         };
         await octokit.rest.pulls.deleteReviewComment(params);
       } catch (error: unknown) {
-        let message = 'reason unkown';
-        if (error instanceof Error) message = error.message;
+        const message = handleOctokitError(error);
         logger.error(`Could not delete review comment: ${message}`);
       }
     }

src/github/artifacts.ts

@@ -4,6 +4,7 @@ import { create } from '@actions/artifact';
 import { logger } from '../helper/logger';
 import { readdirSync } from 'fs';
 import { join } from 'canonical-path';
+import { handleOctokitError } from '../helper/error';
 
 export async function uploadArtifact(): Promise<void> {
   const artifactClient = create();
@@ -18,8 +19,7 @@ export async function uploadArtifact(): Promise<void> {
       logger.debug(`Failed to upload file(s): ${response.failedItems.join(', ')}`);
     }
   } catch (error: unknown) {
-    let message = 'reason unknown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.debug('Failed to upload artifact: ' + message);
   }
 }

src/github/comments.ts

@@ -5,6 +5,7 @@ import { createErrorSummary } from '../helper/summary';
 import { generateStatusMarkdown } from '../helper/markdown';
 import { Status } from '../helper/enums';
 import { Comment } from './interfaces';
+import { handleOctokitError } from '../helper/error';
 
 /**
  * Gets a list of all comments on the pull request.
@@ -21,8 +22,7 @@ export async function getPostedComments(): Promise<Comment[]> {
     };
     response = await octokit.paginate(octokit.rest.issues.listComments, params);
   } catch (error: unknown) {
-    let message = 'reason unkown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.error(`Could not retrieve the comments: ${message}`);
   }
   return response;
@@ -65,8 +65,7 @@ export async function postComment(body: string): Promise<void> {
     await octokit.rest.issues.createComment(params);
     logger.info('Posted comment for this pull request.');
   } catch (error: unknown) {
-    let message = 'reason unkown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.error(`Posting the comment failed: ${message}`);
   }
 }
@@ -83,8 +82,7 @@ export function deletePreviousComments(comments: Comment[]): void {
         };
         await octokit.rest.issues.deleteComment(params);
       } catch (error: unknown) {
-        let message = 'reason unkown';
-        if (error instanceof Error) message = error.message;
+        const message = handleOctokitError(error);
         logger.error(`Removing a comment failed: ${message}`);
       }
     }

src/github/commits.ts

@@ -2,6 +2,7 @@ import { normalize } from 'canonical-path';
 import { logger } from '../helper/logger';
 import { githubConfig, octokit, ticsConfig } from '../configuration';
 import { ChangedFile } from './interfaces';
+import { handleOctokitError } from '../helper/error';
 
 /**
  * Sends a request to retrieve the changed files for a given pull request to the GitHub API.
@@ -39,8 +40,7 @@ export async function getChangedFilesOfCommit(): Promise<ChangedFile[]> {
     });
     logger.info('Retrieved changed files from commit.');
   } catch (error: unknown) {
-    let message = 'error unknown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.exit(`Could not retrieve the changed files: ${message}`);
   }
   return response;

src/github/interfaces.d.ts

@@ -81,8 +81,8 @@ export interface ReviewComment {
   node_id: string;
   diff_hunk: string;
   path: string;
-  position: number;
-  original_position: number;
+  position?: number;
+  original_position?: number;
   commit_id: string;
   original_commit_id: string;
   in_reply_to_id?: number;

src/github/pulls.ts

@@ -3,6 +3,7 @@ import { normalize, resolve } from 'canonical-path';
 import { logger } from '../helper/logger';
 import { githubConfig, octokit, ticsConfig } from '../configuration';
 import { ChangedFile } from './interfaces';
+import { handleOctokitError } from '../helper/error';
 
 /**
  * Sends a request to retrieve the changed files for a given pull request to the GitHub API.
@@ -39,8 +40,7 @@ export async function getChangedFilesOfPullRequest(): Promise<ChangedFile[]> {
     });
     logger.info('Retrieved changed files from pull request.');
   } catch (error: unknown) {
-    let message = 'error unknown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.exit(`Could not retrieve the changed files: ${message}`);
   }
   return response;

src/github/review.ts

@@ -2,6 +2,7 @@ import { logger } from '../helper/logger';
 import { githubConfig, octokit } from '../configuration';
 import { Events, Status } from '../helper/enums';
 import { generateStatusMarkdown } from '../helper/markdown';
+import { handleOctokitError } from '../helper/error';
 
 /**
  * Create review on the pull request from the analysis given.
@@ -22,8 +23,7 @@ export async function postReview(body: string, event: Events): Promise<void> {
     await octokit.rest.pulls.createReview(params);
     logger.info('Posted review for this pull request.');
   } catch (error: unknown) {
-    let message = 'reason unkown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.error(`Posting the review failed: ${message}`);
   }
 }
@@ -48,8 +48,7 @@ export async function postNothingAnalyzedReview(message: string): Promise<void>
     await octokit.rest.pulls.createReview(params);
     logger.info('Posted review for this pull request.');
   } catch (error: unknown) {
-    let message = 'reason unkown';
-    if (error instanceof Error) message = error.message;
+    const message = handleOctokitError(error);
     logger.error(`Posting the review failed: ${message}`);
   }
 }

src/helper/error.ts

@@ -0,0 +1,14 @@
+import { RequestError } from '@octokit/request-error';
+
+export function handleOctokitError(error: unknown): string {
+  let message = 'reason unkown';
+  if (error instanceof Error) {
+    message = '';
+    const retryCount = (error as RequestError).request?.request?.retryCount;
+    if (retryCount) {
+      message = `Retried ${retryCount} time(s), but got: `;
+    }
+    message += error.message;
+  }
+  return message;
+}

src/tics/api_helper.ts

@@ -1,8 +1,8 @@
 import { OutgoingHttpHeaders } from 'http';
 import { logger } from '../helper/logger';
-import { githubConfig, requestInit, ticsConfig, viewerUrl } from '../configuration';
+import { githubConfig, httpClient, retryConfig, ticsConfig, viewerUrl } from '../configuration';
 import { Analysis, HttpResponse } from '../helper/interfaces';
-import fetch, { Response } from 'node-fetch';
+import { HttpClientResponse } from '@actions/http-client';
 
 /**
  * Executes a GET request to the given url.
@@ -17,40 +17,42 @@ export async function httpRequest<T>(url: string): Promise<T | undefined> {
     headers.Authorization = `Basic ${ticsConfig.ticsAuthToken}`;
   }
 
-  requestInit.headers = headers;
+  const response: HttpClientResponse = await httpClient.get(url, headers);
 
-  const response: Response = await fetch(url, requestInit);
+  let errorMessage = '';
+  if (response.message.statusCode && retryConfig.retryCodes.includes(response.message.statusCode)) {
+    errorMessage += `Retried ${retryConfig.maxRetries} time(s), but got: `;
+  }
+  errorMessage += `HTTP request failed with status ${response.message.statusCode}.`;
 
-  switch (response.status) {
+  switch (response.message.statusCode) {
     case 200:
-      const text = await response.text();
+      const text = await response.readBody();
       try {
         return <T>JSON.parse(text);
       } catch (error: unknown) {
         logger.exit(`${error}. Tried to parse response: ${text}`);
       }
       break;
     case 302:
-      logger.exit(
-        `HTTP request failed with status ${response.status}. Please check if the given ticsConfiguration is correct (possibly http instead of https).`
-      );
+      logger.exit(`${errorMessage} Please check if the given ticsConfiguration is correct (possibly http instead of https).`);
       break;
     case 400:
-      logger.exit(`HTTP request failed with status ${response.status}. ${(<HttpResponse>await response.json()).alertMessages[0].header}`);
+      logger.exit(`${errorMessage} ${(<HttpResponse>JSON.parse(await response.readBody())).alertMessages[0].header}`);
       break;
     case 401:
       logger.exit(
-        `HTTP request failed with status ${response.status}. Please provide a valid TICSAUTHTOKEN in your configuration. Check ${viewerUrl}/Administration.html#page=authToken`
+        `${errorMessage} Please provide a valid TICSAUTHTOKEN in your configuration. Check ${viewerUrl}/Administration.html#page=authToken`
       );
       break;
     case 403:
-      logger.exit(`HTTP request failed with status ${response.status}. Forbidden call: ${url}`);
+      logger.exit(`${errorMessage} Forbidden call: ${url}`);
       break;
     case 404:
-      logger.exit(`HTTP request failed with status ${response.status}. Please check if the given ticsConfiguration is correct.`);
+      logger.exit(`${errorMessage} Please check if the given ticsConfiguration is correct.`);
       break;
     default:
-      logger.exit(`HTTP request failed with status ${response.status}. Please check if your configuration is correct.`);
+      logger.exit(`${errorMessage} ${response.message.statusMessage}`);
       break;
   }
   return;

test/.setup/mock.ts

@@ -23,6 +23,10 @@ jest.mock('../../src/configuration', () => {
       id: '123-1',
       commitSha: 'asdfghjk'
     },
+    retryConfig: {
+      maxRetries: 10,
+      retryCodes: [502, 503, 504]
+    },
     octokit: {
       paginate: jest.fn(),
       rest: {
@@ -42,7 +46,9 @@ jest.mock('../../src/configuration', () => {
         }
       }
     },
-    requestInit: { headers: {} },
+    httpClient: {
+      get: jest.fn()
+    },
     viewerUrl: '<url>',
     baseUrl: 'http://base.com'
   };
@@ -83,7 +89,6 @@ jest.mock('@actions/artifact', () => {
     })
   };
 });
-jest.mock('node-fetch', () => jest.fn());
 jest.mock('fs', () => {
   return {
     writeFileSync: jest.fn(),
@@ -98,9 +103,6 @@ jest.mock('canonical-path', () => {
     join: jest.fn((one, two) => `${one}/${two}`)
   };
 });
-jest.mock('proxy-agent', () => {
-  return jest.fn();
-});
 jest.mock('os', () => {
   return {
     tmpdir: jest.fn(() => '/tmp')