Clean up test code

packages/cache/__tests__/requestUtils.test.ts

@@ -1,36 +1,48 @@
 import {retry} from '../src/internal/requestUtils'
+import {HttpClientError} from '@actions/http-client'
 
-interface TestResponse {
+interface ITestResponse {
   statusCode: number
   result: string | null
+  error: Error | null
+}
+
+function TestResponse(
+  action: number | Error,
+  result: string | null = null
+): ITestResponse {
+  if (action instanceof Error) {
+    return {
+      statusCode: -1,
+      result,
+      error: action
+    }
+  } else {
+    return {
+      statusCode: action,
+      result,
+      error: null
+    }
+  }
 }
 
 async function handleResponse(
-  response: TestResponse | undefined
+  response: ITestResponse | undefined
-): Promise<TestResponse> {
+): Promise<ITestResponse> {
   if (!response) {
     // eslint-disable-next-line no-undef
     fail('Retry method called too many times')
   }
 
-  // Status codes >= 600 will throw an Error instead of returning a response object. This
+  if (response.error) {
-  // mimics the behavior of the http-client *Json methods, which throw an error on any
+    throw response.error
-  // non-successful status codes.  Values in the 6xx, 7xx, and 8xx range are converted
-  // to the corresponding 3xx, 4xx, and 5xx status code.
-  if (response.statusCode >= 900) {
-    throw Error('Test Error')
-  } else if (response.statusCode >= 600) {
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    const error = Error('Test Error with Status Code') as any
-    error['statusCode'] = response.statusCode - 300
-    throw error
   } else {
     return Promise.resolve(response)
   }
 }
 
 async function testRetryExpectingResult(
-  responses: TestResponse[],
+  responses: ITestResponse[],
   expectedResult: string | null
 ): Promise<void> {
   responses = responses.reverse() // Reverse responses since we pop from end
@@ -38,7 +50,7 @@ async function testRetryExpectingResult(
   const actualResult = await retry(
     'test',
     async () => handleResponse(responses.pop()),
-    (response: TestResponse) => response.statusCode,
+    (response: ITestResponse) => response.statusCode,
     2, // maxAttempts
     0 // delay
   )
@@ -47,7 +59,7 @@ async function testRetryExpectingResult(
 }
 
 async function testRetryConvertingErrorToResult(
-  responses: TestResponse[],
+  responses: ITestResponse[],
   expectedStatus: number,
   expectedResult: string | null
 ): Promise<void> {
@@ -56,15 +68,16 @@ async function testRetryConvertingErrorToResult(
   const actualResult = await retry(
     'test',
     async () => handleResponse(responses.pop()),
-    (response: TestResponse) => response.statusCode,
+    (response: ITestResponse) => response.statusCode,
     2, // maxAttempts
     0, // delay
     (e: Error) => {
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      if (e instanceof HttpClientError) {
-      const error = e as any
         return {
-        statusCode: error['statusCode'],
+          statusCode: e.statusCode,
-        result: error['result'] ?? null
+          result: null,
+          error: null
+        }
       }
     }
   )
@@ -74,7 +87,7 @@ async function testRetryConvertingErrorToResult(
 }
 
 async function testRetryExpectingError(
-  responses: TestResponse[]
+  responses: ITestResponse[]
 ): Promise<void> {
   responses = responses.reverse() // Reverse responses since we pop from end
 
@@ -82,7 +95,7 @@ async function testRetryExpectingError(
     retry(
       'test',
       async () => handleResponse(responses.pop()),
-      (response: TestResponse) => response.statusCode,
+      (response: ITestResponse) => response.statusCode,
       2, // maxAttempts,
       0 // delay
     )
@@ -90,103 +103,45 @@ async function testRetryExpectingError(
 }
 
 test('retry works on successful response', async () => {
-  await testRetryExpectingResult(
+  await testRetryExpectingResult([TestResponse(200, 'Ok')], 'Ok')
-    [
-      {
-        statusCode: 200,
-        result: 'Ok'
-      }
-    ],
-    'Ok'
-  )
 })
 
 test('retry works after retryable status code', async () => {
   await testRetryExpectingResult(
-    [
+    [TestResponse(503), TestResponse(200, 'Ok')],
-      {
-        statusCode: 503,
-        result: null
-      },
-      {
-        statusCode: 200,
-        result: 'Ok'
-      }
-    ],
     'Ok'
   )
 })
 
 test('retry fails after exhausting retries', async () => {
   await testRetryExpectingError([
-    {
+    TestResponse(503),
-      statusCode: 503,
+    TestResponse(503),
-      result: null
+    TestResponse(200, 'Ok')
-    },
-    {
-      statusCode: 503,
-      result: null
-    },
-    {
-      statusCode: 200,
-      result: 'Ok'
-    }
   ])
 })
 
 test('retry fails after non-retryable status code', async () => {
-  await testRetryExpectingError([
+  await testRetryExpectingError([TestResponse(500), TestResponse(200, 'Ok')])
-    {
-      statusCode: 500,
-      result: null
-    },
-    {
-      statusCode: 200,
-      result: 'Ok'
-    }
-  ])
 })
 
 test('retry works after error', async () => {
   await testRetryExpectingResult(
-    [
+    [TestResponse(new Error('Test error')), TestResponse(200, 'Ok')],
-      {
-        statusCode: 999,
-        result: null
-      },
-      {
-        statusCode: 200,
-        result: 'Ok'
-      }
-    ],
     'Ok'
   )
 })
 
 test('retry returns after client error', async () => {
   await testRetryExpectingResult(
-    [
+    [TestResponse(400), TestResponse(200, 'Ok')],
-      {
-        statusCode: 400,
-        result: null
-      },
-      {
-        statusCode: 200,
-        result: 'Ok'
-      }
-    ],
     null
   )
 })
 
 test('retry converts errors to response object', async () => {
   await testRetryConvertingErrorToResult(
-    [
+    [TestResponse(new HttpClientError('Test error', 409))],
-      {
-        statusCode: 709, // throw a 409 Conflict error
-        result: null
-      }
-    ],
     409,
     null
   )

packages/cache/package.json

@@ -40,7 +40,7 @@
     "@actions/core": "^1.2.4",
     "@actions/exec": "^1.0.1",
     "@actions/glob": "^0.1.0",
-    "@actions/http-client": "^1.0.8",
+    "@actions/http-client": "^1.0.9",
     "@actions/io": "^1.0.1",
     "@azure/ms-rest-js": "^2.0.7",
     "@azure/storage-blob": "^12.1.2",

packages/cache/src/internal/constants.ts

@@ -11,8 +11,11 @@ export enum CompressionMethod {
   Zstd = 'zstd'
 }
 
+// The default number of retry attempts.
+export const DefaultRetryAttempts = 2
+
 // The default delay in milliseconds between retry attempts.
-export const RetryDelay = 5000
+export const DefaultRetryDelay = 5000
 
 // Socket timeout in milliseconds during download.  If no traffic is received
 // over the socket during this period, the socket is destroyed and the download

packages/cache/src/internal/requestUtils.ts

@@ -1,10 +1,10 @@
 import * as core from '@actions/core'
-import {HttpCodes} from '@actions/http-client'
+import {HttpCodes, HttpClientError} from '@actions/http-client'
 import {
   IHttpClientResponse,
   ITypedResponse
 } from '@actions/http-client/interfaces'
-import {RetryDelay} from './constants'
+import {DefaultRetryDelay, DefaultRetryAttempts} from './constants'
 
 export function isSuccessStatusCode(statusCode?: number): boolean {
   if (!statusCode) {
@@ -40,8 +40,8 @@ export async function retry<T>(
   name: string,
   method: () => Promise<T>,
   getStatusCode: (arg0: T) => number | undefined,
-  maxAttempts = 2,
+  maxAttempts = DefaultRetryAttempts,
-  delay = RetryDelay,
+  delay = DefaultRetryDelay,
   onError: ((arg0: Error) => T | undefined) | undefined = undefined
 ): Promise<T> {
   let errorMessage = ''
@@ -95,22 +95,21 @@ export async function retry<T>(
 export async function retryTypedResponse<T>(
   name: string,
   method: () => Promise<ITypedResponse<T>>,
-  maxAttempts = 2
+  maxAttempts = DefaultRetryAttempts,
+  delay = DefaultRetryDelay
 ): Promise<ITypedResponse<T>> {
   return await retry(
     name,
     method,
     (response: ITypedResponse<T>) => response.statusCode,
     maxAttempts,
-    RetryDelay,
+    delay,
     // If the error object contains the statusCode property, extract it and return
     // an ITypedResponse<T> so it can be processed by the retry logic.
-    (e: Error) => {
+    (error: Error) => {
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
+      if (error instanceof HttpClientError) {
-      const error = e as any
-      if (error['statusCode']) {
         return {
-          statusCode: error['statusCode'],
+          statusCode: error.statusCode,
           result: null,
           headers: {}
         }
@@ -124,13 +123,14 @@ export async function retryTypedResponse<T>(
 export async function retryHttpClientResponse<T>(
   name: string,
   method: () => Promise<IHttpClientResponse>,
-  maxAttempts = 2
+  maxAttempts = DefaultRetryAttempts,
+  delay = DefaultRetryDelay
 ): Promise<IHttpClientResponse> {
   return await retry(
     name,
     method,
     (response: IHttpClientResponse) => response.message.statusCode,
     maxAttempts,
-    RetryDelay
+    delay
   )
 }