@actions/artifact package updates (#408)

* Clear error message when storage quota has been hit

* Improved download of empty files

* Extra info to RELEASES.md

* PR Feedback

packages/artifact/RELEASES.md

@@ -10,3 +10,12 @@
 - GZip file compression to speed up downloads
 - Improved logging and output
 - Extra documentation
+
+### 0.3.0
+
+- Fixes to gzip decompression when downloading artifacts
+- Support handling 429 response codes
+- Improved download experience when dealing with empty files
+- Exponential backoff when retryable status codes are encountered
+- Clearer error message if storage quota has been reached
+- Improved logging and output during artifact download

packages/artifact/__tests__/download-specification.test.ts

@@ -21,7 +21,8 @@ function getPartialContainerEntry(): ContainerEntry {
     lastModifiedBy: '82f0bf89-6e55-4e5a-b8b6-f75eb992578c',
     itemLocation: 'ADD_INFORMATION',
     contentLocation: 'ADD_INFORMATION',
-    contentId: ''
+    contentId: '',
+    fileLength: 100
   }
 }
 
@@ -72,13 +73,13 @@ function createContentLocation(relativePath: string): string {
                 /dir3
                     /dir4
                         file4.txt
-                        file5.txt
-
+                        file5.txt (no length property)
+                        file6.txt (empty file)
     /my-artifact-extra
         /file1.txt
 */
 
-// main artfact
+// main artifact
 const file1Path = path.join(artifact1Name, 'file1.txt')
 const file2Path = path.join(artifact1Name, 'file2.txt')
 const dir1Path = path.join(artifact1Name, 'dir1')
@@ -88,6 +89,7 @@ const dir3Path = path.join(dir2Path, 'dir3')
 const dir4Path = path.join(dir3Path, 'dir4')
 const file4Path = path.join(dir4Path, 'file4.txt')
 const file5Path = path.join(dir4Path, 'file5.txt')
+const file6Path = path.join(dir4Path, 'file6.txt')
 
 const rootDirectoryEntry = createDirectoryEntry(artifact1Name)
 const directoryEntry1 = createDirectoryEntry(dir1Path)
@@ -98,7 +100,11 @@ const fileEntry1 = createFileEntry(file1Path)
 const fileEntry2 = createFileEntry(file2Path)
 const fileEntry3 = createFileEntry(file3Path)
 const fileEntry4 = createFileEntry(file4Path)
-const fileEntry5 = createFileEntry(file5Path)
+
+const missingLengthFileEntry = createFileEntry(file5Path)
+missingLengthFileEntry.fileLength = undefined // one file does not have a fileLength
+const emptyLengthFileEntry = createFileEntry(file6Path)
+emptyLengthFileEntry.fileLength = 0 // empty file path
 
 // extra artifact
 const artifact2File1Path = path.join(artifact2Name, 'file1.txt')
@@ -115,7 +121,8 @@ const artifactContainerEntries: ContainerEntry[] = [
   directoryEntry3,
   directoryEntry4,
   fileEntry4,
-  fileEntry5,
+  missingLengthFileEntry,
+  emptyLengthFileEntry,
   rootDirectoryEntry2,
   extraFileEntry
 ]
@@ -170,6 +177,14 @@ describe('Search', () => {
       'dir4',
       'file5.txt'
     )
+    const item6ExpectedTargetPath = path.join(
+      testDownloadPath,
+      'dir1',
+      'dir2',
+      'dir3',
+      'dir4',
+      'file6.txt'
+    )
 
     const targetLocations = specification.filesToDownload.map(
       item => item.targetPath
@@ -214,6 +229,9 @@ describe('Search', () => {
     expect(specification.directoryStructure).toContain(
       path.join(testDownloadPath, 'dir1', 'dir2', 'dir3', 'dir4')
     )
+
+    expect(specification.emptyFilesToCreate.length).toEqual(1)
+    expect(specification.emptyFilesToCreate).toContain(item6ExpectedTargetPath)
   })
 
   it('Download Specification - Relative Path with no root directory', () => {
@@ -252,6 +270,14 @@ describe('Search', () => {
       'dir4',
       'file5.txt'
     )
+    const item6ExpectedTargetPath = path.join(
+      testDownloadPath,
+      'dir1',
+      'dir2',
+      'dir3',
+      'dir4',
+      'file6.txt'
+    )
 
     const targetLocations = specification.filesToDownload.map(
       item => item.targetPath
@@ -296,6 +322,9 @@ describe('Search', () => {
     expect(specification.directoryStructure).toContain(
       path.join(testDownloadPath, 'dir1', 'dir2', 'dir3', 'dir4')
     )
+
+    expect(specification.emptyFilesToCreate.length).toEqual(1)
+    expect(specification.emptyFilesToCreate).toContain(item6ExpectedTargetPath)
   })
 
   it('Download Specification - Absolute Path with root directory', () => {
@@ -352,6 +381,15 @@ describe('Search', () => {
       'dir4',
       'file5.txt'
     )
+    const item6ExpectedTargetPath = path.join(
+      testDownloadPath,
+      artifact1Name,
+      'dir1',
+      'dir2',
+      'dir3',
+      'dir4',
+      'file6.txt'
+    )
 
     const targetLocations = specification.filesToDownload.map(
       item => item.targetPath
@@ -398,6 +436,9 @@ describe('Search', () => {
     expect(specification.directoryStructure).toContain(
       path.join(testDownloadPath, dir4Path)
     )
+
+    expect(specification.emptyFilesToCreate.length).toEqual(1)
+    expect(specification.emptyFilesToCreate).toContain(item6ExpectedTargetPath)
   })
 
   it('Download Specification - Relative Path with root directory', () => {
@@ -449,6 +490,15 @@ describe('Search', () => {
       'dir4',
       'file5.txt'
     )
+    const item6ExpectedTargetPath = path.join(
+      testDownloadPath,
+      artifact1Name,
+      'dir1',
+      'dir2',
+      'dir3',
+      'dir4',
+      'file6.txt'
+    )
 
     const targetLocations = specification.filesToDownload.map(
       item => item.targetPath
@@ -495,5 +545,8 @@ describe('Search', () => {
     expect(specification.directoryStructure).toContain(
       path.join(testDownloadPath, dir4Path)
     )
+
+    expect(specification.emptyFilesToCreate.length).toEqual(1)
+    expect(specification.emptyFilesToCreate).toContain(item6ExpectedTargetPath)
   })
 })

packages/artifact/__tests__/upload.test.ts

@@ -106,6 +106,18 @@ describe('Upload Tests', () => {
     )
   })
 
+  it('Create Artifact - Storage Quota Error', async () => {
+    const artifactName = 'storage-quota-hit'
+    const uploadHttpClient = new UploadHttpClient()
+    expect(
+      uploadHttpClient.createArtifactInFileContainer(artifactName)
+    ).rejects.toEqual(
+      new Error(
+        'Artifact storage quota has been hit. Unable to upload any new artifacts'
+      )
+    )
+  })
+
   /**
    * Artifact Upload Tests
    */
@@ -367,6 +379,8 @@ describe('Upload Tests', () => {
 
         if (inputData.Name === 'invalid-artifact-name') {
           mockMessage.statusCode = 400
+        } else if (inputData.Name === 'storage-quota-hit') {
+          mockMessage.statusCode = 403
         } else {
           mockMessage.statusCode = 201
           const response: ArtifactResponse = {

packages/artifact/__tests__/util.test.ts

@@ -190,14 +190,14 @@ describe('Utils', () => {
       true
     )
     expect(utils.isRetryableStatusCode(HttpCodes.GatewayTimeout)).toEqual(true)
-    expect(utils.isRetryableStatusCode(429)).toEqual(true)
+    expect(utils.isRetryableStatusCode(HttpCodes.TooManyRequests)).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.TooManyRequests)).toEqual(true)
     expect(utils.isThrottledStatusCode(HttpCodes.InternalServerError)).toEqual(
       false
     )
@@ -207,6 +207,17 @@ describe('Utils', () => {
     )
   })
 
+  it('Test Forbidden Status Code', () => {
+    expect(utils.isForbiddenStatusCode(HttpCodes.Forbidden)).toEqual(true)
+    expect(utils.isForbiddenStatusCode(HttpCodes.InternalServerError)).toEqual(
+      false
+    )
+    expect(utils.isForbiddenStatusCode(HttpCodes.TooManyRequests)).toEqual(
+      false
+    )
+    expect(utils.isForbiddenStatusCode(HttpCodes.OK)).toEqual(false)
+  })
+
   it('Test Creating Artifact Directories', async () => {
     const root = path.join(__dirname, '_temp', 'artifact-download')
     // remove directory before starting
@@ -216,12 +227,43 @@ describe('Utils', () => {
     const directory2 = path.join(directory1, 'folder1')
 
     // Initially should not exist
-    expect(fs.existsSync(directory1)).toEqual(false)
-    expect(fs.existsSync(directory2)).toEqual(false)
+    await expect(fs.promises.access(directory1)).rejects.not.toBeUndefined()
+    await expect(fs.promises.access(directory2)).rejects.not.toBeUndefined()
     const directoryStructure = [directory1, directory2]
     await utils.createDirectoriesForArtifact(directoryStructure)
     // directories should now be created
-    expect(fs.existsSync(directory1)).toEqual(true)
-    expect(fs.existsSync(directory2)).toEqual(true)
+    await expect(fs.promises.access(directory1)).resolves.toEqual(undefined)
+    await expect(fs.promises.access(directory2)).resolves.toEqual(undefined)
+  })
+
+  it('Test Creating Empty Files', async () => {
+    const root = path.join(__dirname, '_temp', 'empty-files')
+    await io.rmRF(root)
+
+    const emptyFile1 = path.join(root, 'emptyFile1')
+    const directoryToCreate = path.join(root, 'folder1')
+    const emptyFile2 = path.join(directoryToCreate, 'emptyFile2')
+
+    // empty files should only be created after the directory structure is fully setup
+    // ensure they are first created by using the createDirectoriesForArtifact method
+    const directoryStructure = [root, directoryToCreate]
+    await utils.createDirectoriesForArtifact(directoryStructure)
+    await expect(fs.promises.access(root)).resolves.toEqual(undefined)
+    await expect(fs.promises.access(directoryToCreate)).resolves.toEqual(
+      undefined
+    )
+
+    await expect(fs.promises.access(emptyFile1)).rejects.not.toBeUndefined()
+    await expect(fs.promises.access(emptyFile2)).rejects.not.toBeUndefined()
+
+    const emptyFilesToCreate = [emptyFile1, emptyFile2]
+    await utils.createEmptyFilesForArtifact(emptyFilesToCreate)
+
+    await expect(fs.promises.access(emptyFile1)).resolves.toEqual(undefined)
+    const size1 = (await fs.promises.stat(emptyFile1)).size
+    expect(size1).toEqual(0)
+    await expect(fs.promises.access(emptyFile2)).resolves.toEqual(undefined)
+    const size2 = (await fs.promises.stat(emptyFile2)).size
+    expect(size2).toEqual(0)
   })
 })

packages/artifact/package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/artifact",
-  "version": "0.2.0",
+  "version": "0.3.0",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

packages/artifact/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/artifact",
-  "version": "0.2.0",
+  "version": "0.3.0",
   "preview": true,
   "description": "Actions artifact lib",
   "keywords": [

packages/artifact/src/internal/artifact-client.ts

@@ -8,7 +8,11 @@ import {UploadResponse} from './upload-response'
 import {UploadOptions} from './upload-options'
 import {DownloadOptions} from './download-options'
 import {DownloadResponse} from './download-response'
-import {checkArtifactName, createDirectoriesForArtifact} from './utils'
+import {
+  checkArtifactName,
+  createDirectoriesForArtifact,
+  createEmptyFilesForArtifact
+} from './utils'
 import {DownloadHttpClient} from './download-http-client'
 import {getDownloadSpecification} from './download-specification'
 import {getWorkSpaceDirectory} from './config-variables'
@@ -174,6 +178,9 @@ export class DefaultArtifactClient implements ArtifactClient {
         downloadSpecification.directoryStructure
       )
       core.info('Directory structure has been setup for the artifact')
+      await createEmptyFilesForArtifact(
+        downloadSpecification.emptyFilesToCreate
+      )
       await downloadHttpClient.downloadSingleArtifact(
         downloadSpecification.filesToDownload
       )
@@ -228,6 +235,9 @@ export class DefaultArtifactClient implements ArtifactClient {
         await createDirectoriesForArtifact(
           downloadSpecification.directoryStructure
         )
+        await createEmptyFilesForArtifact(
+          downloadSpecification.emptyFilesToCreate
+        )
         await downloadHttpClient.downloadSingleArtifact(
           downloadSpecification.filesToDownload
         )

packages/artifact/src/internal/download-specification.ts

@@ -8,6 +8,9 @@ export interface DownloadSpecification {
   // directories that need to be created for all the items in the artifact
   directoryStructure: string[]
 
+  // empty files that are part of the artifact that don't require any downloading
+  emptyFilesToCreate: string[]
+
   // individual files that need to be downloaded as part of the artifact
   filesToDownload: DownloadItem[]
 }
@@ -33,6 +36,7 @@ export function getDownloadSpecification(
   downloadPath: string,
   includeRootDirectory: boolean
 ): DownloadSpecification {
+  // use a set for the directory paths so that there are no duplicates
   const directories = new Set<string>()
 
   const specifications: DownloadSpecification = {
@@ -40,6 +44,7 @@ export function getDownloadSpecification(
       ? path.join(downloadPath, artifactName)
       : downloadPath,
     directoryStructure: [],
+    emptyFilesToCreate: [],
     filesToDownload: []
   }
 
@@ -64,7 +69,10 @@ export function getDownloadSpecification(
       if (entry.itemType === 'file') {
         // Get the directories that we need to create from the filePath for each individual file
         directories.add(path.dirname(filePath))
-
+        if (entry.fileLength === 0) {
+          // An empty file was uploaded, create the empty files locally so that no extra http calls are made
+          specifications.emptyFilesToCreate.push(filePath)
+        } else {
           specifications.filesToDownload.push({
             sourceLocation: entry.contentLocation,
             targetPath: filePath
@@ -72,6 +80,7 @@ export function getDownloadSpecification(
         }
       }
     }
+  }
 
   specifications.directoryStructure = Array.from(directories)
   return specifications

packages/artifact/src/internal/upload-http-client.ts

@@ -15,6 +15,7 @@ import {
   isRetryableStatusCode,
   isSuccessStatusCode,
   isThrottledStatusCode,
+  isForbiddenStatusCode,
   displayHttpDiagnostics,
   getExponentialRetryTimeInMilliseconds,
   tryGetRetryAfterValueTimeInMilliseconds
@@ -68,6 +69,12 @@ export class UploadHttpClient {
 
     if (isSuccessStatusCode(rawResponse.message.statusCode) && body) {
       return JSON.parse(body)
+    } else if (isForbiddenStatusCode(rawResponse.message.statusCode)) {
+      // if a 403 is returned when trying to create a file container, the customer has exceeded
+      // their storage quota so no new artifact containers can be created
+      throw new Error(
+        `Artifact storage quota has been hit. Unable to upload any new artifacts`
+      )
     } else {
       displayHttpDiagnostics(rawResponse)
       throw new Error(

packages/artifact/src/internal/utils.ts

@@ -58,6 +58,14 @@ export function isSuccessStatusCode(statusCode?: number): boolean {
   return statusCode >= 200 && statusCode < 300
 }
 
+export function isForbiddenStatusCode(statusCode?: number): boolean {
+  if (!statusCode) {
+    return false
+  }
+
+  return statusCode === HttpCodes.Forbidden
+}
+
 export function isRetryableStatusCode(statusCode?: number): boolean {
   if (!statusCode) {
     return false
@@ -296,3 +304,11 @@ export async function createDirectoriesForArtifact(
     })
   }
 }
+
+export async function createEmptyFilesForArtifact(
+  emptyFilesToCreate: string[]
+): Promise<void> {
+  for (const filePath of emptyFilesToCreate) {
+    await (await fs.open(filePath, 'w')).close()
+  }
+}