From ce9eae0785a556ab1e9e3cb095353e7045378b0b Mon Sep 17 00:00:00 2001
From: Rob Herley <robherley@github.com>
Date: Tue, 5 Dec 2023 18:35:26 +0000
Subject: [PATCH] consistent promise behavior for download artifact

---
 packages/artifact/__tests__/download-artifact.test.ts     | 4 ----
 .../artifact/src/internal/download/download-artifact.ts   | 8 ++++----
 packages/artifact/src/internal/find/get-artifact.ts       | 8 ++++++--
 packages/artifact/src/internal/shared/errors.ts           | 3 +--
 packages/artifact/src/internal/shared/interfaces.ts       | 4 ----
 5 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/packages/artifact/__tests__/download-artifact.test.ts b/packages/artifact/__tests__/download-artifact.test.ts
index f28d442d..247a5e42 100644
--- a/packages/artifact/__tests__/download-artifact.test.ts
+++ b/packages/artifact/__tests__/download-artifact.test.ts
@@ -164,7 +164,6 @@ describe('download-artifact', () => {
         fixtures.blobStorageUrl
       )
       expectExtractedArchive(fixtures.workspaceDir)
-      expect(response.success).toBe(true)
       expect(response.downloadPath).toBe(fixtures.workspaceDir)
     })
 
@@ -214,7 +213,6 @@ describe('download-artifact', () => {
         fixtures.blobStorageUrl
       )
       expectExtractedArchive(customPath)
-      expect(response.success).toBe(true)
       expect(response.downloadPath).toBe(customPath)
     })
 
@@ -344,7 +342,6 @@ describe('download-artifact', () => {
       const response = await downloadArtifactInternal(fixtures.artifactID)
 
       expectExtractedArchive(fixtures.workspaceDir)
-      expect(response.success).toBe(true)
       expect(response.downloadPath).toBe(fixtures.workspaceDir)
       expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
       expect(mockListArtifacts).toHaveBeenCalledWith({
@@ -396,7 +393,6 @@ describe('download-artifact', () => {
       })
 
       expectExtractedArchive(customPath)
-      expect(response.success).toBe(true)
       expect(response.downloadPath).toBe(customPath)
       expect(mockHttpClient).toHaveBeenCalledWith(getUserAgentString())
       expect(mockListArtifacts).toHaveBeenCalledWith({
diff --git a/packages/artifact/src/internal/download/download-artifact.ts b/packages/artifact/src/internal/download/download-artifact.ts
index cf5b5e15..3bb3de61 100644
--- a/packages/artifact/src/internal/download/download-artifact.ts
+++ b/packages/artifact/src/internal/download/download-artifact.ts
@@ -16,6 +16,7 @@ import {
   ListArtifactsRequest
 } from '../../generated'
 import {getBackendIdsFromToken} from '../shared/util'
+import {ArtifactNotFoundError} from '../shared/errors'
 
 const scrubQueryParameters = (url: string): string => {
   const parsed = new URL(url)
@@ -95,7 +96,7 @@ export async function downloadArtifactPublic(
     throw new Error(`Unable to download and extract artifact: ${error.message}`)
   }
 
-  return {success: true, downloadPath}
+  return {downloadPath}
 }
 
 export async function downloadArtifactInternal(
@@ -118,10 +119,9 @@ export async function downloadArtifactInternal(
   const {artifacts} = await artifactClient.ListArtifacts(listReq)
 
   if (artifacts.length === 0) {
-    core.warning(
+    throw new ArtifactNotFoundError(
       `No artifacts found for ID: ${artifactId}\nAre you trying to download from a different run? Try specifying a github-token with \`actions:read\` scope.`
     )
-    return {success: false}
   }
 
   if (artifacts.length > 1) {
@@ -148,7 +148,7 @@ export async function downloadArtifactInternal(
     throw new Error(`Unable to download and extract artifact: ${error.message}`)
   }
 
-  return {success: true, downloadPath}
+  return {downloadPath}
 }
 
 async function resolveOrCreateDirectory(
diff --git a/packages/artifact/src/internal/find/get-artifact.ts b/packages/artifact/src/internal/find/get-artifact.ts
index 45db581f..b19e5d6e 100644
--- a/packages/artifact/src/internal/find/get-artifact.ts
+++ b/packages/artifact/src/internal/find/get-artifact.ts
@@ -48,7 +48,9 @@ export async function getArtifactPublic(
   }
 
   if (getArtifactResp.data.artifacts.length === 0) {
-    throw new ArtifactNotFoundError(artifactName)
+    throw new ArtifactNotFoundError(
+      `Artifact not found for name: ${artifactName}`
+    )
   }
 
   let artifact = getArtifactResp.data.artifacts[0]
@@ -86,7 +88,9 @@ export async function getArtifactInternal(
   const res = await artifactClient.ListArtifacts(req)
 
   if (res.artifacts.length === 0) {
-    throw new ArtifactNotFoundError(artifactName)
+    throw new ArtifactNotFoundError(
+      `Artifact not found for name: ${artifactName}`
+    )
   }
 
   let artifact = res.artifacts[0]
diff --git a/packages/artifact/src/internal/shared/errors.ts b/packages/artifact/src/internal/shared/errors.ts
index 2ddf79ab..7ca33bdb 100644
--- a/packages/artifact/src/internal/shared/errors.ts
+++ b/packages/artifact/src/internal/shared/errors.ts
@@ -21,8 +21,7 @@ export class InvalidResponseError extends Error {
 }
 
 export class ArtifactNotFoundError extends Error {
-  constructor(name: string) {
-    const message = `No artifact found for name: ${name}`
+  constructor(message = 'Artifact not found') {
     super(message)
     this.name = 'ArtifactNotFoundError'
   }
diff --git a/packages/artifact/src/internal/shared/interfaces.ts b/packages/artifact/src/internal/shared/interfaces.ts
index b72274de..a8a6daa7 100644
--- a/packages/artifact/src/internal/shared/interfaces.ts
+++ b/packages/artifact/src/internal/shared/interfaces.ts
@@ -86,10 +86,6 @@ export interface ListArtifactsResponse {
  *                                                                           *
  *****************************************************************************/
 export interface DownloadArtifactResponse {
-  /**
-   * If the artifact download was successful
-   */
-  success: boolean
   /**
    * The path where the artifact was downloaded to
    */