Updates to @actions/artifact (#396)

* Add support for 429s and Exponential backoff

* Refactor status-reporter so it can be used with download and upload

* Extra logs

* Fixes around download & gzip

* Cleanup headers and add extra tests

* Improved Docs

* Spelling bloopers

* Improved error messages

* User http client version 1.0.7

packages/artifact/__tests__/download.test.ts

@@ -4,6 +4,8 @@ import * as io from '../../io/src/io'
 import * as net from 'net'
 import * as path from 'path'
 import * as configVariables from '../src/internal/config-variables'
+import {promises as fs} from 'fs'
+import {DownloadItem} from '../src/internal/download-specification'
 import {HttpClient, HttpClientResponse} from '@actions/http-client'
 import {DownloadHttpClient} from '../src/internal/download-http-client'
 import {
@@ -11,7 +13,7 @@ import {
   QueryArtifactResponse
 } from '../src/internal/contracts'
 
-const root = path.join(__dirname, '_temp', 'artifact-download')
+const root = path.join(__dirname, '_temp', 'artifact-download-tests')
 
 jest.mock('../src/internal/config-variables')
 jest.mock('@actions/http-client')
@@ -19,12 +21,16 @@ jest.mock('@actions/http-client')
 describe('Download Tests', () => {
   beforeAll(async () => {
     await io.rmRF(root)
+    await fs.mkdir(path.join(root), {
+      recursive: true
+    })
 
     // mock all output so that there is less noise when running tests
     jest.spyOn(console, 'log').mockImplementation(() => {})
     jest.spyOn(core, 'debug').mockImplementation(() => {})
     jest.spyOn(core, 'info').mockImplementation(() => {})
     jest.spyOn(core, 'warning').mockImplementation(() => {})
+    jest.spyOn(core, 'error').mockImplementation(() => {})
   })
 
   /**
@@ -107,6 +113,56 @@ describe('Download Tests', () => {
     )
   })
 
+  it('Test downloading an individual artifact with gzip', async () => {
+    setupDownloadItemResponse(true, 200)
+    const downloadHttpClient = new DownloadHttpClient()
+
+    const items: DownloadItem[] = []
+    items.push({
+      sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileA.txt`,
+      targetPath: path.join(root, 'FileA.txt')
+    })
+
+    await expect(
+      downloadHttpClient.downloadSingleArtifact(items)
+    ).resolves.not.toThrow()
+  })
+
+  it('Test downloading an individual artifact without gzip', async () => {
+    setupDownloadItemResponse(false, 200)
+    const downloadHttpClient = new DownloadHttpClient()
+
+    const items: DownloadItem[] = []
+    items.push({
+      sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileB.txt`,
+      targetPath: path.join(root, 'FileB.txt')
+    })
+
+    await expect(
+      downloadHttpClient.downloadSingleArtifact(items)
+    ).resolves.not.toThrow()
+  })
+
+  it('Test retryable status codes during artifact download', async () => {
+    // The first http response should return a retryable status call while the subsequent call should return a 200 so
+    // the download should successfully finish
+    const retryableStatusCodes = [429, 502, 503, 504]
+    for (const statusCode of retryableStatusCodes) {
+      setupDownloadItemResponse(false, statusCode)
+      const downloadHttpClient = new DownloadHttpClient()
+
+      const items: DownloadItem[] = []
+      items.push({
+        sourceLocation: `${configVariables.getRuntimeUrl()}_apis/resources/Containers/13?itemPath=my-artifact%2FFileC.txt`,
+        targetPath: path.join(root, 'FileC.txt')
+      })
+
+      await expect(
+        downloadHttpClient.downloadSingleArtifact(items)
+      ).resolves.not.toThrow()
+    }
+  })
+
   /**
    * Helper used to setup mocking for the HttpClient
    */
@@ -164,6 +220,60 @@ describe('Download Tests', () => {
     })
   }
 
+  /**
+   * Setups up HTTP GET response for downloading items
+   * @param isGzip is the downloaded item gzip encoded
+   * @param firstHttpResponseCode the http response code that should be returned
+   */
+  function setupDownloadItemResponse(
+    isGzip: boolean,
+    firstHttpResponseCode: number
+  ): void {
+    jest
+      .spyOn(DownloadHttpClient.prototype, 'pipeResponseToFile')
+      .mockImplementationOnce(async () => {
+        return new Promise<void>(resolve => {
+          resolve()
+        })
+      })
+
+    jest
+      .spyOn(HttpClient.prototype, 'get')
+      .mockImplementationOnce(async () => {
+        const mockMessage = new http.IncomingMessage(new net.Socket())
+        mockMessage.statusCode = firstHttpResponseCode
+        if (isGzip) {
+          mockMessage.headers = {
+            'content-type': 'gzip'
+          }
+        }
+
+        return new Promise<HttpClientResponse>(resolve => {
+          resolve({
+            message: mockMessage,
+            readBody: emptyMockReadBody
+          })
+        })
+      })
+      .mockImplementationOnce(async () => {
+        // chained response, if the HTTP GET function gets called again, return a successful response
+        const mockMessage = new http.IncomingMessage(new net.Socket())
+        mockMessage.statusCode = 200
+        if (isGzip) {
+          mockMessage.headers = {
+            'content-type': 'gzip'
+          }
+        }
+
+        return new Promise<HttpClientResponse>(resolve => {
+          resolve({
+            message: mockMessage,
+            readBody: emptyMockReadBody
+          })
+        })
+      })
+  }
+
   /**
    * Setups up HTTP GET response when querying for container items
    */

packages/artifact/__tests__/upload.test.ts

@@ -12,6 +12,7 @@ import {
   PatchArtifactSizeSuccessResponse
 } from '../src/internal/contracts'
 import {UploadSpecification} from '../src/internal/upload-specification'
+import {getArtifactUrl} from '../src/internal/utils'
 
 const root = path.join(__dirname, '_temp', 'artifact-upload')
 const file1Path = path.join(root, 'file1.txt')
@@ -36,6 +37,7 @@ describe('Upload Tests', () => {
     jest.spyOn(core, 'debug').mockImplementation(() => {})
     jest.spyOn(core, 'info').mockImplementation(() => {})
     jest.spyOn(core, 'warning').mockImplementation(() => {})
+    jest.spyOn(core, 'error').mockImplementation(() => {})
 
     // setup mocking for calls that got through the HttpClient
     setupHttpClientMock()
@@ -99,7 +101,7 @@ describe('Upload Tests', () => {
       uploadHttpClient.createArtifactInFileContainer(artifactName)
     ).rejects.toEqual(
       new Error(
-        'Unable to create a container for the artifact invalid-artifact-name'
+        `Unable to create a container for the artifact invalid-artifact-name at ${getArtifactUrl()}`
       )
     )
   })

packages/artifact/__tests__/util.test.ts

@@ -4,7 +4,12 @@ import * as path from 'path'
 import * as utils from '../src/internal/utils'
 import * as core from '@actions/core'
 import {HttpCodes} from '@actions/http-client'
-import {getRuntimeUrl, getWorkFlowRunId} from '../src/internal/config-variables'
+import {
+  getRuntimeUrl,
+  getWorkFlowRunId,
+  getInitialRetryIntervalInMilliseconds,
+  getRetryMultiplier
+} from '../src/internal/config-variables'
 
 jest.mock('../src/internal/config-variables')
 
@@ -17,6 +22,30 @@ describe('Utils', () => {
     jest.spyOn(core, 'warning').mockImplementation(() => {})
   })
 
+  it('Check exponential retry range', () => {
+    // No retries should return the initial retry interval
+    const retryWaitTime0 = utils.getExponentialRetryTimeInMilliseconds(0)
+    expect(retryWaitTime0).toEqual(getInitialRetryIntervalInMilliseconds())
+
+    const testMinMaxRange = (retryCount: number): void => {
+      const retryWaitTime = utils.getExponentialRetryTimeInMilliseconds(
+        retryCount
+      )
+      const minRange =
+        getInitialRetryIntervalInMilliseconds() *
+        getRetryMultiplier() *
+        retryCount
+      const maxRange = minRange * getRetryMultiplier()
+
+      expect(retryWaitTime).toBeGreaterThanOrEqual(minRange)
+      expect(retryWaitTime).toBeLessThan(maxRange)
+    }
+
+    for (let i = 1; i < 10; i++) {
+      testMinMaxRange(i)
+    }
+  })
+
   it('Check Artifact Name for any invalid characters', () => {
     const invalidNames = [
       'my\\artifact',
@@ -88,13 +117,13 @@ describe('Utils', () => {
     )
   })
 
-  it('Test constructing headers with all optional parameters', () => {
-    const type = 'application/json'
+  it('Test constructing upload headers with all optional parameters', () => {
+    const contentType = 'application/octet-stream'
     const size = 24
     const uncompressedLength = 100
     const range = 'bytes 0-199/200'
-    const options = utils.getRequestOptions(
-      type,
+    const options = utils.getUploadRequestOptions(
+      contentType,
       true,
       true,
       uncompressedLength,
@@ -103,9 +132,9 @@ describe('Utils', () => {
     )
     expect(Object.keys(options).length).toEqual(8)
     expect(options['Accept']).toEqual(
-      `${type};api-version=${utils.getApiVersion()}`
+      `application/json;api-version=${utils.getApiVersion()}`
     )
-    expect(options['Content-Type']).toEqual(type)
+    expect(options['Content-Type']).toEqual(contentType)
     expect(options['Connection']).toEqual('Keep-Alive')
     expect(options['Keep-Alive']).toEqual('10')
     expect(options['Content-Encoding']).toEqual('gzip')
@@ -114,9 +143,33 @@ describe('Utils', () => {
     expect(options['Content-Range']).toEqual(range)
   })
 
-  it('Test constructing headers with only required parameter', () => {
-    const options = utils.getRequestOptions()
-    expect(Object.keys(options).length).toEqual(1)
+  it('Test constructing upload headers with only required parameter', () => {
+    const options = utils.getUploadRequestOptions('application/octet-stream')
+    expect(Object.keys(options).length).toEqual(2)
+    expect(options['Accept']).toEqual(
+      `application/json;api-version=${utils.getApiVersion()}`
+    )
+    expect(options['Content-Type']).toEqual('application/octet-stream')
+  })
+
+  it('Test constructing download headers with all optional parameters', () => {
+    const contentType = 'application/json'
+    const options = utils.getDownloadRequestOptions(contentType, true, true)
+    expect(Object.keys(options).length).toEqual(5)
+    expect(options['Content-Type']).toEqual(contentType)
+    expect(options['Connection']).toEqual('Keep-Alive')
+    expect(options['Keep-Alive']).toEqual('10')
+    expect(options['Accept-Encoding']).toEqual('gzip')
+    expect(options['Accept']).toEqual(
+      `application/octet-stream;api-version=${utils.getApiVersion()}`
+    )
+  })
+
+  it('Test constructing download headers with only required parameter', () => {
+    const options = utils.getDownloadRequestOptions('application/octet-stream')
+    expect(Object.keys(options).length).toEqual(2)
+    expect(options['Content-Type']).toEqual('application/octet-stream')
+    // check for default accept type
     expect(options['Accept']).toEqual(
       `application/json;api-version=${utils.getApiVersion()}`
     )
@@ -137,11 +190,23 @@ describe('Utils', () => {
       true
     )
     expect(utils.isRetryableStatusCode(HttpCodes.GatewayTimeout)).toEqual(true)
+    expect(utils.isRetryableStatusCode(429)).toEqual(true)
     expect(utils.isRetryableStatusCode(HttpCodes.OK)).toEqual(false)
     expect(utils.isRetryableStatusCode(HttpCodes.NotFound)).toEqual(false)
     expect(utils.isRetryableStatusCode(HttpCodes.Forbidden)).toEqual(false)
   })
 
+  it('Test Throttled Status Code', () => {
+    expect(utils.isThrottledStatusCode(429)).toEqual(true)
+    expect(utils.isThrottledStatusCode(HttpCodes.InternalServerError)).toEqual(
+      false
+    )
+    expect(utils.isThrottledStatusCode(HttpCodes.BadGateway)).toEqual(false)
+    expect(utils.isThrottledStatusCode(HttpCodes.ServiceUnavailable)).toEqual(
+      false
+    )
+  })
+
   it('Test Creating Artifact Directories', async () => {
     const root = path.join(__dirname, '_temp', 'artifact-download')
     // remove directory before starting