From 75a35860615d2c8cb85640d8ec0e2c936c952bf9 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 17:35:46 +0000 Subject: [PATCH 1/7] consistent promise behavior for upload artifact --- .../__tests__/upload-artifact.test.ts | 19 ++++----- packages/artifact/src/artifact.ts | 1 + .../artifact/src/internal/shared/errors.ts | 21 ++++++++++ .../src/internal/shared/interfaces.ts | 5 --- .../src/internal/upload/blob-upload.ts | 42 ++++++------------- .../src/internal/upload/upload-artifact.ts | 28 +++++-------- 6 files changed, 53 insertions(+), 63 deletions(-) create mode 100644 packages/artifact/src/internal/shared/errors.ts diff --git a/packages/artifact/__tests__/upload-artifact.test.ts b/packages/artifact/__tests__/upload-artifact.test.ts index 439a96a3..b0dca5c8 100644 --- a/packages/artifact/__tests__/upload-artifact.test.ts +++ b/packages/artifact/__tests__/upload-artifact.test.ts @@ -7,6 +7,7 @@ import {Timestamp, ArtifactServiceClientJSON} from '../src/generated' import * as blobUpload from '../src/internal/upload/blob-upload' import {uploadArtifact} from '../src/internal/upload/upload-artifact' import {noopLogs} from './common' +import {FilesNotFoundError} from '../src/internal/shared/errors' describe('upload-artifact', () => { beforeEach(() => { @@ -59,7 +60,6 @@ describe('upload-artifact', () => { ) jest.spyOn(blobUpload, 'uploadZipToBlobStorage').mockReturnValue( Promise.resolve({ - isSuccess: true, uploadSize: 1234, sha256Hash: 'test-sha256-hash' }) @@ -84,7 +84,7 @@ describe('upload-artifact', () => { '/home/user/files/plz-upload' ) - expect(uploadResp).resolves.toEqual({success: true, size: 1234, id: 1}) + expect(uploadResp).resolves.toEqual({size: 1234, id: 1}) }) it('should throw an error if the root directory is invalid', () => { @@ -107,7 +107,7 @@ describe('upload-artifact', () => { expect(uploadResp).rejects.toThrow('Invalid root directory') }) - it('should return false if there are no files to upload', () => { + it('should reject if there are no files to upload', () => { jest .spyOn(uploadZipSpecification, 'validateRootDirectory') .mockReturnValue() @@ -124,7 +124,7 @@ describe('upload-artifact', () => { ], '/home/user/files/plz-upload' ) - expect(uploadResp).resolves.toEqual({success: false}) + expect(uploadResp).rejects.toThrowError(FilesNotFoundError) }) it('should reject if no backend IDs are found', () => { @@ -217,7 +217,7 @@ describe('upload-artifact', () => { '/home/user/files/plz-upload' ) - expect(uploadResp).resolves.toEqual({success: false}) + expect(uploadResp).rejects.toThrow() }) it('should return false if blob storage upload is unsuccessful', () => { @@ -262,7 +262,7 @@ describe('upload-artifact', () => { ) jest .spyOn(blobUpload, 'uploadZipToBlobStorage') - .mockReturnValue(Promise.resolve({isSuccess: false})) + .mockReturnValue(Promise.reject(new Error('boom'))) // ArtifactHttpClient mocks jest.spyOn(config, 'getRuntimeToken').mockReturnValue('test-token') @@ -280,10 +280,10 @@ describe('upload-artifact', () => { '/home/user/files/plz-upload' ) - expect(uploadResp).resolves.toEqual({success: false}) + expect(uploadResp).rejects.toThrow() }) - it('should return false if finalize artifact fails', () => { + it('should reject if finalize artifact fails', () => { const mockDate = new Date('2020-01-01') jest .spyOn(uploadZipSpecification, 'validateRootDirectory') @@ -325,7 +325,6 @@ describe('upload-artifact', () => { ) jest.spyOn(blobUpload, 'uploadZipToBlobStorage').mockReturnValue( Promise.resolve({ - isSuccess: true, uploadSize: 1234, sha256Hash: 'test-sha256-hash' }) @@ -350,6 +349,6 @@ describe('upload-artifact', () => { '/home/user/files/plz-upload' ) - expect(uploadResp).resolves.toEqual({success: false}) + expect(uploadResp).rejects.toThrow() }) }) diff --git a/packages/artifact/src/artifact.ts b/packages/artifact/src/artifact.ts index 5742b32d..3ba38448 100644 --- a/packages/artifact/src/artifact.ts +++ b/packages/artifact/src/artifact.ts @@ -4,6 +4,7 @@ import {ArtifactClient, Client} from './internal/client' * Exported functionality that we want to expose for any users of @actions/artifact */ export * from './internal/shared/interfaces' +export * from './internal/shared/errors' export {ArtifactClient} export function create(): ArtifactClient { diff --git a/packages/artifact/src/internal/shared/errors.ts b/packages/artifact/src/internal/shared/errors.ts new file mode 100644 index 00000000..37b92d14 --- /dev/null +++ b/packages/artifact/src/internal/shared/errors.ts @@ -0,0 +1,21 @@ +export class FilesNotFoundError extends Error { + files: string[] + + constructor(files: string[] = []) { + let message = 'No files were found to upload' + if (files.length > 0) { + message += `: ${files.join(', ')}` + } + + super(message) + this.files = files + this.name = 'FilesNotFoundError' + } +} + +export class InvalidResponseError extends Error { + constructor(message: string) { + super(message) + this.name = 'InvalidResponseError' + } +} diff --git a/packages/artifact/src/internal/shared/interfaces.ts b/packages/artifact/src/internal/shared/interfaces.ts index 61994b2b..654c5981 100644 --- a/packages/artifact/src/internal/shared/interfaces.ts +++ b/packages/artifact/src/internal/shared/interfaces.ts @@ -4,11 +4,6 @@ * * *****************************************************************************/ export interface UploadArtifactResponse { - /** - * Denotes if an artifact was successfully uploaded - */ - success: boolean - /** * Total size of the artifact in bytes. Not provided if no artifact was uploaded */ diff --git a/packages/artifact/src/internal/upload/blob-upload.ts b/packages/artifact/src/internal/upload/blob-upload.ts index cb7a11b7..59472385 100644 --- a/packages/artifact/src/internal/upload/blob-upload.ts +++ b/packages/artifact/src/internal/upload/blob-upload.ts @@ -7,11 +7,6 @@ import * as crypto from 'crypto' import * as stream from 'stream' export interface BlobUploadResponse { - /** - * If the upload was successful or not - */ - isSuccess: boolean - /** * The total reported upload size in bytes. Empty if the upload failed */ @@ -55,41 +50,28 @@ export async function uploadZipToBlobStorage( zipUploadStream.pipe(uploadStream) // This stream is used for the upload zipUploadStream.pipe(hashStream).setEncoding('hex') // This stream is used to compute a hash of the zip content that gets used. Integrity check - try { - core.info('Beginning upload of artifact content to blob storage') + core.info('Beginning upload of artifact content to blob storage') - await blockBlobClient.uploadStream( - uploadStream, - bufferSize, - maxConcurrency, - options - ) + await blockBlobClient.uploadStream( + uploadStream, + bufferSize, + maxConcurrency, + options + ) - core.info('Finished uploading artifact content to blob storage!') + core.info('Finished uploading artifact content to blob storage!') - hashStream.end() - sha256Hash = hashStream.read() as string - core.info(`SHA256 hash of uploaded artifact zip is ${sha256Hash}`) - } catch (error) { - core.warning( - `Failed to upload artifact zip to blob storage, error: ${error}` - ) - return { - isSuccess: false - } - } + hashStream.end() + sha256Hash = hashStream.read() as string + core.info(`SHA256 hash of uploaded artifact zip is ${sha256Hash}`) if (uploadByteCount === 0) { core.warning( - `No data was uploaded to blob storage. Reported upload byte count is 0` + `No data was uploaded to blob storage. Reported upload byte count is 0.` ) - return { - isSuccess: false - } } return { - isSuccess: true, uploadSize: uploadByteCount, sha256Hash } diff --git a/packages/artifact/src/internal/upload/upload-artifact.ts b/packages/artifact/src/internal/upload/upload-artifact.ts index 6546f98f..55921252 100644 --- a/packages/artifact/src/internal/upload/upload-artifact.ts +++ b/packages/artifact/src/internal/upload/upload-artifact.ts @@ -19,6 +19,7 @@ import { FinalizeArtifactRequest, StringValue } from '../../generated' +import {FilesNotFoundError, InvalidResponseError} from '../shared/errors' export async function uploadArtifact( name: string, @@ -34,10 +35,9 @@ export async function uploadArtifact( rootDirectory ) if (zipSpecification.length === 0) { - core.warning(`No files were found to upload`) - return { - success: false - } + throw new FilesNotFoundError( + zipSpecification.flatMap(s => (s.sourcePath ? [s.sourcePath] : [])) + ) } const zipUploadStream = await createZipUploadStream( @@ -68,10 +68,9 @@ export async function uploadArtifact( const createArtifactResp = await artifactClient.CreateArtifact(createArtifactReq) if (!createArtifactResp.ok) { - core.warning(`Failed to create artifact`) - return { - success: false - } + throw new InvalidResponseError( + 'CreateArtifact: response from backend was not ok' + ) } // Upload zip to blob storage @@ -79,11 +78,6 @@ export async function uploadArtifact( createArtifactResp.signedUploadUrl, zipUploadStream ) - if (uploadResult.isSuccess === false) { - return { - success: false - } - } // finalize the artifact const finalizeArtifactReq: FinalizeArtifactRequest = { @@ -104,10 +98,9 @@ export async function uploadArtifact( const finalizeArtifactResp = await artifactClient.FinalizeArtifact(finalizeArtifactReq) if (!finalizeArtifactResp.ok) { - core.warning(`Failed to finalize artifact`) - return { - success: false - } + throw new InvalidResponseError( + 'FinalizeArtifact: response from backend was not ok' + ) } const artifactId = BigInt(finalizeArtifactResp.artifactId) @@ -116,7 +109,6 @@ export async function uploadArtifact( ) return { - success: true, size: uploadResult.uploadSize, id: Number(artifactId) } From d3c5f358d11f9b074021398c0c6bc98f6c9bff5f Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 17:56:18 +0000 Subject: [PATCH 2/7] consistent promise behavior for get artifact --- .../artifact/__tests__/get-artifact.test.ts | 34 ++++++++----------- .../src/internal/find/get-artifact.ts | 22 ++++-------- .../artifact/src/internal/shared/errors.ts | 8 +++++ .../src/internal/shared/interfaces.ts | 5 --- 4 files changed, 29 insertions(+), 40 deletions(-) diff --git a/packages/artifact/__tests__/get-artifact.test.ts b/packages/artifact/__tests__/get-artifact.test.ts index 82008a4b..56ed0d34 100644 --- a/packages/artifact/__tests__/get-artifact.test.ts +++ b/packages/artifact/__tests__/get-artifact.test.ts @@ -8,6 +8,10 @@ import * as config from '../src/internal/shared/config' import {ArtifactServiceClientJSON, Timestamp} from '../src/generated' import * as util from '../src/internal/shared/util' import {noopLogs} from './common' +import { + ArtifactNotFoundError, + InvalidResponseError +} from '../src/internal/shared/errors' type MockedRequest = jest.MockedFunction> @@ -76,7 +80,6 @@ describe('get-artifact', () => { ) expect(response).toEqual({ - success: true, artifact: fixtures.artifacts[0] }) }) @@ -107,7 +110,6 @@ describe('get-artifact', () => { ) expect(response).toEqual({ - success: true, artifact: fixtures.artifacts[1] }) }) @@ -124,7 +126,7 @@ describe('get-artifact', () => { } }) - const response = await getArtifactPublic( + const response = getArtifactPublic( fixtures.artifacts[0].name, fixtures.runId, fixtures.owner, @@ -132,9 +134,7 @@ describe('get-artifact', () => { fixtures.token ) - expect(response).toEqual({ - success: false - }) + expect(response).rejects.toThrowError(ArtifactNotFoundError) }) it('should fail if non-200 response', async () => { @@ -147,7 +147,7 @@ describe('get-artifact', () => { data: {} }) - const response = await getArtifactPublic( + const response = getArtifactPublic( fixtures.artifacts[0].name, fixtures.runId, fixtures.owner, @@ -155,9 +155,7 @@ describe('get-artifact', () => { fixtures.token ) - expect(response).toEqual({ - success: false - }) + expect(response).rejects.toThrowError(InvalidResponseError) }) }) @@ -192,7 +190,6 @@ describe('get-artifact', () => { const response = await getArtifactInternal(fixtures.artifacts[0].name) expect(response).toEqual({ - success: true, artifact: fixtures.artifacts[0] }) }) @@ -213,7 +210,6 @@ describe('get-artifact', () => { const response = await getArtifactInternal(fixtures.artifacts[0].name) expect(response).toEqual({ - success: true, artifact: fixtures.artifacts[1] }) }) @@ -225,21 +221,19 @@ describe('get-artifact', () => { artifacts: [] }) - const response = await getArtifactInternal(fixtures.artifacts[0].name) + const response = getArtifactInternal(fixtures.artifacts[0].name) - expect(response).toEqual({ - success: false - }) + expect(response).rejects.toThrowError(ArtifactNotFoundError) }) it('should fail if non-200 response', async () => { jest .spyOn(ArtifactServiceClientJSON.prototype, 'ListArtifacts') - .mockRejectedValue(new Error('test error')) + .mockRejectedValue(new Error('boom')) - await expect( - getArtifactInternal(fixtures.artifacts[0].name) - ).rejects.toThrow('test error') + const response = getArtifactInternal(fixtures.artifacts[0].name) + + expect(response).rejects.toThrow() }) }) }) diff --git a/packages/artifact/src/internal/find/get-artifact.ts b/packages/artifact/src/internal/find/get-artifact.ts index 91c85818..45db581f 100644 --- a/packages/artifact/src/internal/find/get-artifact.ts +++ b/packages/artifact/src/internal/find/get-artifact.ts @@ -10,6 +10,7 @@ import {getBackendIdsFromToken} from '../shared/util' import {getUserAgentString} from '../shared/user-agent' import {internalArtifactTwirpClient} from '../shared/artifact-twirp-client' import {ListArtifactsRequest, StringValue, Timestamp} from '../../generated' +import {ArtifactNotFoundError, InvalidResponseError} from '../shared/errors' export async function getArtifactPublic( artifactName: string, @@ -41,17 +42,13 @@ export async function getArtifactPublic( ) if (getArtifactResp.status !== 200) { - core.warning(`non-200 response from GitHub API: ${getArtifactResp.status}`) - return { - success: false - } + throw new InvalidResponseError( + `Invalid response from GitHub API: ${getArtifactResp.status} (${getArtifactResp?.headers?.['x-github-request-id']})` + ) } if (getArtifactResp.data.artifacts.length === 0) { - core.warning('no artifacts found') - return { - success: false - } + throw new ArtifactNotFoundError(artifactName) } let artifact = getArtifactResp.data.artifacts[0] @@ -63,7 +60,6 @@ export async function getArtifactPublic( } return { - success: true, artifact: { name: artifact.name, id: artifact.id, @@ -90,10 +86,7 @@ export async function getArtifactInternal( const res = await artifactClient.ListArtifacts(req) if (res.artifacts.length === 0) { - core.warning('no artifacts found') - return { - success: false - } + throw new ArtifactNotFoundError(artifactName) } let artifact = res.artifacts[0] @@ -103,12 +96,11 @@ export async function getArtifactInternal( )[0] core.debug( - `more than one artifact found for a single name, returning newest (id: ${artifact.databaseId})` + `More than one artifact found for a single name, returning newest (id: ${artifact.databaseId})` ) } return { - success: true, artifact: { name: artifact.name, id: Number(artifact.databaseId), diff --git a/packages/artifact/src/internal/shared/errors.ts b/packages/artifact/src/internal/shared/errors.ts index 37b92d14..2ddf79ab 100644 --- a/packages/artifact/src/internal/shared/errors.ts +++ b/packages/artifact/src/internal/shared/errors.ts @@ -19,3 +19,11 @@ export class InvalidResponseError extends Error { this.name = 'InvalidResponseError' } } + +export class ArtifactNotFoundError extends Error { + constructor(name: string) { + const message = `No artifact found for name: ${name}` + super(message) + this.name = 'ArtifactNotFoundError' + } +} diff --git a/packages/artifact/src/internal/shared/interfaces.ts b/packages/artifact/src/internal/shared/interfaces.ts index 654c5981..b72274de 100644 --- a/packages/artifact/src/internal/shared/interfaces.ts +++ b/packages/artifact/src/internal/shared/interfaces.ts @@ -53,11 +53,6 @@ export interface UploadArtifactOptions { *****************************************************************************/ export interface GetArtifactResponse { - /** - * If an artifact was found - */ - success: boolean - /** * Metadata about the artifact that was found */ From ce9eae0785a556ab1e9e3cb095353e7045378b0b Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 18:35:26 +0000 Subject: [PATCH 3/7] 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 */ From b9872153b88fc1b4de6d067c02f8a730caba295e Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 18:42:36 +0000 Subject: [PATCH 4/7] update GHES warning behavior --- packages/artifact/src/internal/client.ts | 75 +++++++------------ .../artifact/src/internal/shared/errors.ts | 9 +++ .../src/internal/shared/interfaces.ts | 2 +- 3 files changed, 35 insertions(+), 51 deletions(-) diff --git a/packages/artifact/src/internal/client.ts b/packages/artifact/src/internal/client.ts index 45ba0bfe..921d5faa 100644 --- a/packages/artifact/src/internal/client.ts +++ b/packages/artifact/src/internal/client.ts @@ -17,6 +17,7 @@ import { } from './download/download-artifact' import {getArtifactPublic, getArtifactInternal} from './find/get-artifact' import {listArtifactsPublic, listArtifactsInternal} from './find/list-artifacts' +import {GHESNotSupportError} from './shared/errors' export interface ArtifactClient { /** @@ -52,6 +53,7 @@ export interface ArtifactClient { /** * Finds an artifact by name. * If there are multiple artifacts with the same name in the same workflow run, this will return the latest. + * If the artifact is not found, it will throw. * * If options.findBy is specified, this will use the public List Artifacts API with a name filter which can get artifacts from other runs. * https://docs.github.com/en/rest/actions/artifacts?apiVersion=2022-11-28#list-workflow-run-artifacts @@ -99,16 +101,11 @@ export class Client implements ArtifactClient { rootDirectory: string, options?: UploadArtifactOptions ): Promise { - if (isGhes()) { - warning( - `@actions/artifact v2.0.0+ and upload-artifact@v4+ are not currently supported on GHES.` - ) - return { - success: false - } - } - try { + if (isGhes()) { + throw new GHESNotSupportError() + } + return uploadArtifact(name, files, rootDirectory, options) } catch (error) { warning( @@ -118,9 +115,8 @@ Errors can be temporary, so please try again and optionally run the action with If the error persists, please check whether Actions is operating normally at [https://githubstatus.com](https://www.githubstatus.com).` ) - return { - success: false - } + + throw error } } @@ -131,16 +127,11 @@ If the error persists, please check whether Actions is operating normally at [ht artifactId: number, options?: DownloadArtifactOptions & FindOptions ): Promise { - if (isGhes()) { - warning( - `@actions/artifact v2.0.0+ and download-artifact@v4+ are not currently supported on GHES.` - ) - return { - success: false - } - } - try { + if (isGhes()) { + throw new GHESNotSupportError() + } + if (options?.findBy) { const { findBy: {repositoryOwner, repositoryName, token}, @@ -159,16 +150,14 @@ If the error persists, please check whether Actions is operating normally at [ht return downloadArtifactInternal(artifactId, options) } catch (error) { warning( - `Artifact download failed with error: ${error}. + `Download Artifact failed with error: ${error}. Errors can be temporary, so please try again and optionally run the action with debug mode enabled for more information. If the error persists, please check whether Actions and API requests are operating normally at [https://githubstatus.com](https://www.githubstatus.com).` ) - return { - success: false - } + throw error } } @@ -178,16 +167,11 @@ If the error persists, please check whether Actions and API requests are operati async listArtifacts( options?: ListArtifactsOptions & FindOptions ): Promise { - if (isGhes()) { - warning( - `@actions/artifact v2.0.0+ and download-artifact@v4+ are not currently supported on GHES.` - ) - return { - artifacts: [] - } - } - try { + if (isGhes()) { + throw new GHESNotSupportError() + } + if (options?.findBy) { const { findBy: {workflowRunId, repositoryOwner, repositoryName, token} @@ -212,9 +196,7 @@ Errors can be temporary, so please try again and optionally run the action with If the error persists, please check whether Actions and API requests are operating normally at [https://githubstatus.com](https://www.githubstatus.com).` ) - return { - artifacts: [] - } + throw error } } @@ -225,16 +207,11 @@ If the error persists, please check whether Actions and API requests are operati artifactName: string, options?: FindOptions ): Promise { - if (isGhes()) { - warning( - `@actions/artifact v2.0.0+ and download-artifact@v4+ are not currently supported on GHES.` - ) - return { - success: false - } - } - try { + if (isGhes()) { + throw new GHESNotSupportError() + } + if (options?.findBy) { const { findBy: {workflowRunId, repositoryOwner, repositoryName, token} @@ -252,15 +229,13 @@ If the error persists, please check whether Actions and API requests are operati return getArtifactInternal(artifactName) } catch (error: unknown) { warning( - `Fetching Artifact failed with error: ${error}. + `Get Artifact failed with error: ${error}. Errors can be temporary, so please try again and optionally run the action with debug mode enabled for more information. If the error persists, please check whether Actions and API requests are operating normally at [https://githubstatus.com](https://www.githubstatus.com).` ) - return { - success: false - } + throw error } } } diff --git a/packages/artifact/src/internal/shared/errors.ts b/packages/artifact/src/internal/shared/errors.ts index 7ca33bdb..71e8e304 100644 --- a/packages/artifact/src/internal/shared/errors.ts +++ b/packages/artifact/src/internal/shared/errors.ts @@ -26,3 +26,12 @@ export class ArtifactNotFoundError extends Error { this.name = 'ArtifactNotFoundError' } } + +export class GHESNotSupportError extends Error { + constructor( + message = '@actions/artifact v2.0.0+ and download-artifact@v4+ are not currently supported on GHES.' + ) { + super(message) + this.name = 'NotSupportedGHESError' + } +} diff --git a/packages/artifact/src/internal/shared/interfaces.ts b/packages/artifact/src/internal/shared/interfaces.ts index a8a6daa7..dfd3e1da 100644 --- a/packages/artifact/src/internal/shared/interfaces.ts +++ b/packages/artifact/src/internal/shared/interfaces.ts @@ -56,7 +56,7 @@ export interface GetArtifactResponse { /** * Metadata about the artifact that was found */ - artifact?: Artifact + artifact: Artifact } /***************************************************************************** From a3053b5cc2ac2a8274ef96127b0f6c592a850660 Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 18:47:37 +0000 Subject: [PATCH 5/7] fix typo --- packages/artifact/src/internal/client.ts | 10 +++++----- packages/artifact/src/internal/shared/errors.ts | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/artifact/src/internal/client.ts b/packages/artifact/src/internal/client.ts index 921d5faa..4dd40b84 100644 --- a/packages/artifact/src/internal/client.ts +++ b/packages/artifact/src/internal/client.ts @@ -17,7 +17,7 @@ import { } from './download/download-artifact' import {getArtifactPublic, getArtifactInternal} from './find/get-artifact' import {listArtifactsPublic, listArtifactsInternal} from './find/list-artifacts' -import {GHESNotSupportError} from './shared/errors' +import {GHESNotSupportedError} from './shared/errors' export interface ArtifactClient { /** @@ -103,7 +103,7 @@ export class Client implements ArtifactClient { ): Promise { try { if (isGhes()) { - throw new GHESNotSupportError() + throw new GHESNotSupportedError() } return uploadArtifact(name, files, rootDirectory, options) @@ -129,7 +129,7 @@ If the error persists, please check whether Actions is operating normally at [ht ): Promise { try { if (isGhes()) { - throw new GHESNotSupportError() + throw new GHESNotSupportedError() } if (options?.findBy) { @@ -169,7 +169,7 @@ If the error persists, please check whether Actions and API requests are operati ): Promise { try { if (isGhes()) { - throw new GHESNotSupportError() + throw new GHESNotSupportedError() } if (options?.findBy) { @@ -209,7 +209,7 @@ If the error persists, please check whether Actions and API requests are operati ): Promise { try { if (isGhes()) { - throw new GHESNotSupportError() + throw new GHESNotSupportedError() } if (options?.findBy) { diff --git a/packages/artifact/src/internal/shared/errors.ts b/packages/artifact/src/internal/shared/errors.ts index 71e8e304..a46a01ff 100644 --- a/packages/artifact/src/internal/shared/errors.ts +++ b/packages/artifact/src/internal/shared/errors.ts @@ -27,11 +27,11 @@ export class ArtifactNotFoundError extends Error { } } -export class GHESNotSupportError extends Error { +export class GHESNotSupportedError extends Error { constructor( message = '@actions/artifact v2.0.0+ and download-artifact@v4+ are not currently supported on GHES.' ) { super(message) - this.name = 'NotSupportedGHESError' + this.name = 'GHESNotSupportedError' } } From c390199be6c889df82e741b932657f08e54d00aa Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 13:51:51 -0500 Subject: [PATCH 6/7] Update artifact-tests.yml --- .github/workflows/artifact-tests.yml | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/artifact-tests.yml b/.github/workflows/artifact-tests.yml index c6988efe..6c1d8bc2 100644 --- a/.github/workflows/artifact-tests.yml +++ b/.github/workflows/artifact-tests.yml @@ -68,15 +68,10 @@ jobs: const uploadResult = await artifact.create().uploadArtifact(artifactName, fileContents, './') console.log(uploadResult) - const success = uploadResult.success const size = uploadResult.size const id = uploadResult.id - if (!success) { - throw new Error('Failed to upload artifact') - } else { - console.log(`Successfully uploaded artifact ${id}`) - } + console.log(`Successfully uploaded artifact ${id}`) verify: runs-on: ubuntu-latest From 5f152b798e7a41bc42501229800f5863d28dfe1b Mon Sep 17 00:00:00 2001 From: Rob Herley Date: Tue, 5 Dec 2023 13:54:14 -0500 Subject: [PATCH 7/7] Update artifact-tests.yml --- .github/workflows/artifact-tests.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/artifact-tests.yml b/.github/workflows/artifact-tests.yml index 6c1d8bc2..7bbb4aa9 100644 --- a/.github/workflows/artifact-tests.yml +++ b/.github/workflows/artifact-tests.yml @@ -24,10 +24,10 @@ jobs: steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set Node.js 20.x - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: 20.x @@ -54,8 +54,8 @@ jobs: echo '${{ env.file1 }}' > artifact-path/first.txt echo '${{ env.file2 }}' > artifact-path/second.txt - - name: Upload Artifacts using actions/github-script@v6 - uses: actions/github-script@v6 + - name: Upload Artifacts using actions/github-script@v7 + uses: actions/github-script@v7 with: script: | const artifact = require('./packages/artifact/lib/artifact') @@ -78,10 +78,10 @@ jobs: needs: [build] steps: - name: Checkout - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set Node.js 20.x - uses: actions/setup-node@v3 + uses: actions/setup-node@v4 with: node-version: 20.x @@ -96,8 +96,8 @@ jobs: npm run tsc working-directory: packages/artifact - - name: List artifacts using actions/github-script@v6 - uses: actions/github-script@v6 + - name: List artifacts using actions/github-script@v7 + uses: actions/github-script@v7 with: script: | const artifact = require('./packages/artifact/lib/artifact')