Add artifact upload error handling

.gitignore

@@ -1,2 +1,2 @@
-#node_modules/
+# node_modules/
 __tests__/runner/*

README.md

@@ -13,7 +13,6 @@ This action will create a github release and optionally upload an artifact to it
 - **draft**: Optionally marks this release as a draft release. Set to `true` to enable.
 - **name**: An optional name for the release. If this is omitted the tag will be used.
 - **prerelease**: Optionally marks this release as prerelease. Set to true to enable.
-- **replacesArtifacts**: Indicates if existing release artifacts should be replaced. Defaults to true.
 - **tag**: An optional tag for the release. If this is omitted the git ref will be used (if it is a tag).
 - **token**: (**Required**) The Github token. Typically this will be `${{ secrets.GITHUB_TOKEN }}`.
 

__tests__/Action.test.ts

@@ -5,9 +5,7 @@ import { Releases } from "../src/Releases";
 import { ArtifactUploader } from "../src/ArtifactUploader";
 
 const createMock = jest.fn()
-const deleteMock = jest.fn()
 const getMock = jest.fn()
-const listArtifactsMock = jest.fn()
 const listMock = jest.fn()
 const updateMock = jest.fn()
 const uploadMock = jest.fn()
@@ -23,8 +21,6 @@ const draft = true
 const id = 100
 const name = 'name'
 const prerelease = true
-const releaseId = 101
-const replacesArtifacts = true
 const tag = 'tag'
 const token = 'token'
 const url = 'http://api.example.com'
@@ -55,7 +51,7 @@ describe("Action", () => {
         await action.perform()
 
         expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease)
-        expect(uploadMock).toBeCalledWith(artifacts, releaseId, url)
+        expect(uploadMock).toBeCalledWith(artifacts, url)
     })
 
     it('creates release if no draft releases', async () => {
@@ -71,7 +67,7 @@ describe("Action", () => {
         await action.perform()
 
         expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease)
-        expect(uploadMock).toBeCalledWith(artifacts, releaseId, url)
+        expect(uploadMock).toBeCalledWith(artifacts, url)
 
     })
 
@@ -81,7 +77,7 @@ describe("Action", () => {
         await action.perform()
 
         expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease)
-        expect(uploadMock).toBeCalledWith(artifacts, releaseId, url)
+        expect(uploadMock).toBeCalledWith(artifacts, url)
     })
 
     it('throws error when create fails', async () => {
@@ -152,7 +148,7 @@ describe("Action", () => {
         }
 
         expect(createMock).toBeCalledWith(tag, body, commit, draft, name, prerelease)
-        expect(uploadMock).toBeCalledWith(artifacts, releaseId, url)
+        expect(uploadMock).toBeCalledWith(artifacts, url)
     })
 
     it('updates draft release', async () => {
@@ -169,7 +165,7 @@ describe("Action", () => {
         await action.perform()
 
         expect(updateMock).toBeCalledWith(id, tag, body, commit, draft, name, prerelease)
-        expect(uploadMock).toBeCalledWith(artifacts, releaseId, url)
+        expect(uploadMock).toBeCalledWith(artifacts, url)
 
     })
 
@@ -189,7 +185,7 @@ describe("Action", () => {
         await action.perform()
 
         expect(updateMock).toBeCalledWith(id, tag, body, commit, draft, name, prerelease)
-        expect(uploadMock).toBeCalledWith(artifacts, releaseId, url)
+        expect(uploadMock).toBeCalledWith(artifacts, url)
 
     })
 
@@ -203,18 +199,15 @@ describe("Action", () => {
         const MockReleases = jest.fn<Releases, any>(() => {
             return {
                 create: createMock,
-                deleteArtifact: deleteMock,
                 getByTag: getMock,
-                listArtifactsForRelease: listArtifactsMock,
                 listReleases: listMock,
                 update: updateMock,
-                uploadArtifact: jest.fn()
+                uploadArtifact: uploadMock
             }
         })
 
         createMock.mockResolvedValue({
             data: {
-                id: releaseId,
                 upload_url: url
             }
         })
@@ -228,7 +221,6 @@ describe("Action", () => {
         })
         updateMock.mockResolvedValue({
             data: {
-                id: releaseId,
                 upload_url: url
             }
         })
@@ -243,7 +235,6 @@ describe("Action", () => {
                 draft: draft,
                 name: name,
                 prerelease: prerelease,
-                replacesArtifacts: replacesArtifacts,
                 tag: tag,
                 token: token,
                 readArtifact: () => artifactData

__tests__/ArtifactUploader.test.ts

@@ -8,12 +8,8 @@ const artifacts = [
 ]
 const fileContents = Buffer.from('artful facts', 'utf-8')
 const contentLength = 42
-const releaseId = 100
-const url = 'http://api.example.com'
-
-const deleteMock = jest.fn()
-const listArtifactsMock = jest.fn()
 const uploadMock = jest.fn()
+const url = 'http://api.example.com'
 
 jest.mock('fs', () => {
     return {
@@ -23,115 +19,27 @@ jest.mock('fs', () => {
 })
 
 describe('ArtifactUploader', () => {
-    beforeEach(() => {
+    it('uploads artifacts', () => {
-        deleteMock.mockClear()
+        const uploader = createUploader()
-        listArtifactsMock.mockClear()
+        
-        uploadMock.mockClear()
+        uploader.uploadArtifacts(artifacts, url)
-    })
+        
-
-    it('replaces all artifacts', async () => {
-        mockDeleteSuccess()
-        mockListWithAssets()
-        const uploader = createUploader(true)
-
-        await uploader.uploadArtifacts(artifacts, releaseId, url)
-
         expect(uploadMock).toBeCalledTimes(2)
         expect(uploadMock)
             .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1')
         expect(uploadMock)
             .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2')
-
-        expect(deleteMock).toBeCalledTimes(2)
-        expect(deleteMock).toBeCalledWith(1)
-        expect(deleteMock).toBeCalledWith(2)
-    })
-    
-    it('replaces no artifacts when previous asset list empty', async () => {
-        mockDeleteSuccess()
-        mockListWithoutAssets()
-        const uploader = createUploader(true)
-
-        await uploader.uploadArtifacts(artifacts, releaseId, url)
-
-        expect(uploadMock).toBeCalledTimes(2)
-        expect(uploadMock)
-            .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1')
-        expect(uploadMock)
-            .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2')
-
-        expect(deleteMock).toBeCalledTimes(0)
     })
 
-    it('throws error from replace', async () => {
+    function createUploader(): GithubArtifactUploader {
-        mockDeleteError()
-        mockListWithAssets()
-        const uploader = createUploader(true)
-
-        expect.hasAssertions()
-        try {
-            await uploader.uploadArtifacts(artifacts, releaseId, url)
-        } catch (error) {
-            expect(error).toEqual("error")
-        }
-    })
-
-    it('updates all artifacts, delete none', async () => {
-        mockDeleteError()
-        mockListWithAssets()
-        const uploader = createUploader(false)
-
-        await uploader.uploadArtifacts(artifacts, releaseId, url)
-
-        expect(uploadMock).toBeCalledTimes(2)
-        expect(uploadMock)
-            .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art1')
-        expect(uploadMock)
-            .toBeCalledWith(url, contentLength, 'raw', fileContents, 'art2')
-
-        expect(deleteMock).toBeCalledTimes(0)
-    })
-
-    function createUploader(replaces: boolean): GithubArtifactUploader {
-        uploadMock.mockResolvedValue({})
         const MockReleases = jest.fn<Releases, any>(() => {
             return {
-                create: jest.fn(),
-                deleteArtifact: deleteMock,
                 getByTag: jest.fn(),
-                listArtifactsForRelease: listArtifactsMock,
+                create: jest.fn(),
-                listReleases: jest.fn(),
                 update: jest.fn(),
                 uploadArtifact: uploadMock
             }
         })
-        return new GithubArtifactUploader(new MockReleases(), replaces)
+        return new GithubArtifactUploader(new MockReleases())
-    }
-
-    function mockDeleteError(): any {
-        deleteMock.mockRejectedValue("error")
-    }
-
-    function mockDeleteSuccess(): any {
-        deleteMock.mockResolvedValue({})
-    }
-
-    function mockListWithAssets() {
-        listArtifactsMock.mockResolvedValue({
-            data: [
-                {
-                    name: "art1",
-                    id: 1
-                },
-                {
-                    name: "art2",
-                    id: 2
-                }
-            ]
-        })
-    }
-
-    function mockListWithoutAssets() {
-        listArtifactsMock.mockResolvedValue({ data: [] })
     }
 });

__tests__/Inputs.test.ts

@@ -144,17 +144,6 @@ describe('Inputs', () => {
         })
     })
 
-    describe('replacesArtifacts', () => {
-        it('returns false', () => {
-            expect(inputs.replacesArtifacts).toBe(false)
-        })
-
-        it('returns true', () => {
-            mockGetInput.mockReturnValue('true')
-            expect(inputs.replacesArtifacts).toBe(true)
-        })
-    })
-
     describe('tag', () => {
         it('returns input tag', () => {
             mockGetInput.mockReturnValue('tag')

action.yml

@@ -31,10 +31,7 @@ inputs:
     default: ''
   prerelease:
     description: "Optionally marks this release as prerelease. Set to true to enable."
-    default: ''
+    default: ''  
-  replacesArtifacts:
-    description: "Indicates if existing release artifacts should be replaced. Defaults to true."
-    default: 'true'  
   tag:
     description: 'An optional tag for the release. If this is omitted the git ref will be used (if it is a tag).'
     default: ''

lib/Action.js

@@ -18,12 +18,10 @@ class Action {
     }
     perform() {
         return __awaiter(this, void 0, void 0, function* () {
-            const releaseResponse = yield this.createOrUpdateRelease();
+            const uploadUrl = yield this.createOrUpdateRelease();
-            const releaseId = releaseResponse.id;
-            const uploadUrl = releaseResponse.upload_url;
             const artifacts = this.inputs.artifacts;
             if (artifacts.length > 0) {
-                yield this.uploader.uploadArtifacts(artifacts, releaseId, uploadUrl);
+                yield this.uploader.uploadArtifacts(artifacts, uploadUrl);
             }
         });
     }
@@ -51,7 +49,7 @@ class Action {
     updateRelease(id) {
         return __awaiter(this, void 0, void 0, function* () {
             const response = yield this.releases.update(id, this.inputs.tag, this.inputs.body, this.inputs.commit, this.inputs.draft, this.inputs.name, this.inputs.prerelease);
-            return response.data;
+            return response.data.upload_url;
         });
     }
     noPublishedRelease(error) {
@@ -82,7 +80,7 @@ class Action {
     createRelease() {
         return __awaiter(this, void 0, void 0, function* () {
             const response = yield this.releases.create(this.inputs.tag, this.inputs.body, this.inputs.commit, this.inputs.draft, this.inputs.name, this.inputs.prerelease);
-            return response.data;
+            return response.data.upload_url;
         });
     }
 }

lib/ArtifactUploader.js

@@ -18,17 +18,12 @@ var __importStar = (this && this.__importStar) || function (mod) {
 Object.defineProperty(exports, "__esModule", { value: true });
 const core = __importStar(require("@actions/core"));
 class GithubArtifactUploader {
-    constructor(releases, replacesExistingArtifacts) {
+    constructor(releases) {
-        this.replacesExistingArtifacts = true;
         this.releases = releases;
-        this.replacesExistingArtifacts = replacesExistingArtifacts;
     }
-    uploadArtifacts(artifacts, releaseId, uploadUrl) {
+    uploadArtifacts(artifacts, uploadUrl) {
         return __awaiter(this, void 0, void 0, function* () {
-            if (this.replacesExistingArtifacts) {
+            artifacts.forEach((artifact) => __awaiter(this, void 0, void 0, function* () {
-                yield this.deleteUpdatedArtifacts(artifacts, releaseId);
-            }
-            for (const artifact of artifacts) {
                 try {
                     yield this.releases.uploadArtifact(uploadUrl, artifact.contentLength, artifact.contentType, artifact.readFile(), artifact.name);
                 }
@@ -36,24 +31,7 @@ class GithubArtifactUploader {
                     const message = `Failed to upload artifact ${artifact.name}. Does it already exist?`;
                     core.warning(message);
                 }
-            }
+            }));
-            return Promise.resolve();
-        });
-    }
-    deleteUpdatedArtifacts(artifacts, releaseId) {
-        return __awaiter(this, void 0, void 0, function* () {
-            const response = yield this.releases.listArtifactsForRelease(releaseId);
-            const releaseAssets = response.data;
-            const assetByName = new Map();
-            releaseAssets.forEach(asset => {
-                assetByName[asset.name] = asset;
-            });
-            for (const artifact of artifacts) {
-                const asset = assetByName[artifact.name];
-                if (asset) {
-                    yield this.releases.deleteArtifact(asset.id);
-                }
-            }
         });
     }
 }

lib/Inputs.js

@@ -59,12 +59,8 @@ class CoreInputs {
         return this.tag;
     }
     get prerelease() {
-        const preRelease = core.getInput('prerelease');
+        const draft = core.getInput('prerelease');
-        return preRelease == 'true';
+        return draft == 'true';
-    }
-    get replacesArtifacts() {
-        const replaces = core.getInput('replacesArtifacts');
-        return replaces == 'true';
     }
     get tag() {
         const tag = core.getInput('tag');

lib/Main.js

@@ -43,7 +43,7 @@ function createAction() {
     const globber = new ArtifactGlobber_1.FileArtifactGlobber();
     const inputs = new Inputs_1.CoreInputs(globber, context);
     const releases = new Releases_1.GithubReleases(context, git);
-    const uploader = new ArtifactUploader_1.GithubArtifactUploader(releases, inputs.replacesArtifacts);
+    const uploader = new ArtifactUploader_1.GithubArtifactUploader(releases);
     return new Action_1.Action(inputs, releases, uploader);
 }
 run();

lib/Releases.js

@@ -28,24 +28,6 @@ class GithubReleases {
             });
         });
     }
-    deleteArtifact(assetId) {
-        return __awaiter(this, void 0, void 0, function* () {
-            return this.git.repos.deleteReleaseAsset({
-                asset_id: assetId,
-                owner: this.context.repo.owner,
-                repo: this.context.repo.repo
-            });
-        });
-    }
-    listArtifactsForRelease(releaseId) {
-        return __awaiter(this, void 0, void 0, function* () {
-            return this.git.repos.listAssetsForRelease({
-                owner: this.context.repo.owner,
-                release_id: releaseId,
-                repo: this.context.repo.repo
-            });
-        });
-    }
     listReleases() {
         return __awaiter(this, void 0, void 0, function* () {
             return this.git.repos.listReleases({

src/Action.ts

@@ -1,6 +1,5 @@
 import { Inputs } from "./Inputs";
 import { Releases } from "./Releases";
-import { ReposCreateReleaseResponse } from "@octokit/rest";
 import { ArtifactUploader } from "./ArtifactUploader";
 import { ErrorMessage } from "./ErrorMessage";
 
@@ -16,17 +15,15 @@ export class Action {
     }
 
     async perform() {
-        const releaseResponse = await this.createOrUpdateRelease();
+        const uploadUrl = await this.createOrUpdateRelease()
-        const releaseId = releaseResponse.id
-        const uploadUrl = releaseResponse.upload_url
 
         const artifacts = this.inputs.artifacts
         if (artifacts.length > 0) {
-            await this.uploader.uploadArtifacts(artifacts, releaseId, uploadUrl)
+            await this.uploader.uploadArtifacts(artifacts, uploadUrl)
         }
     }
 
-    private async createOrUpdateRelease(): Promise<ReposCreateReleaseResponse> {
+    private async createOrUpdateRelease(): Promise<string> {
         if (this.inputs.allowUpdates) {
             try {
                 const getResponse = await this.releases.getByTag(this.inputs.tag)
@@ -43,7 +40,7 @@ export class Action {
         }
     }
 
-    private async updateRelease(id: number): Promise<ReposCreateReleaseResponse> {
+    private async updateRelease(id: number): Promise<string> {
         const response = await this.releases.update(
             id,
             this.inputs.tag,
@@ -54,7 +51,7 @@ export class Action {
             this.inputs.prerelease
         )
 
-        return response.data
+        return response.data.upload_url
     }
 
     private noPublishedRelease(error: any): boolean {
@@ -62,7 +59,7 @@ export class Action {
         return errorMessage.status == 404
     }
 
-    private async updateDraftOrCreateRelease(): Promise<ReposCreateReleaseResponse> {
+    private async updateDraftOrCreateRelease(): Promise<string> {
         const draftReleaseId = await this.findMatchingDraftReleaseId()
         if (draftReleaseId) {
             return await this.updateRelease(draftReleaseId)
@@ -80,7 +77,7 @@ export class Action {
         return draftRelease?.id
     }
 
-    private async createRelease(): Promise<ReposCreateReleaseResponse> {
+    private async createRelease(): Promise<string> {
         const response = await this.releases.create(
             this.inputs.tag,
             this.inputs.body,
@@ -90,6 +87,6 @@ export class Action {
             this.inputs.prerelease
         )
 
-        return response.data
+        return response.data.upload_url
     }
 }

src/ArtifactUploader.ts

@@ -1,56 +1,30 @@
 import * as core from '@actions/core';
 import { Artifact } from "./Artifact";
 import { Releases } from "./Releases";
-import { ReposListAssetsForReleaseResponseItem } from "@octokit/rest";
-import { ErrorMessage } from './ErrorMessage';
 
 export interface ArtifactUploader {
-    uploadArtifacts(artifacts: Artifact[], releaseId: number, uploadUrl: string): Promise<void>
+    uploadArtifacts(artifacts: Artifact[], uploadUrl: string): Promise<void>
 }
 
 export class GithubArtifactUploader implements ArtifactUploader {
     private releases: Releases
-    private replacesExistingArtifacts: boolean = true
 
-    constructor(releases: Releases, replacesExistingArtifacts: boolean) {
+    constructor(releases: Releases) {
         this.releases = releases
-        this.replacesExistingArtifacts = replacesExistingArtifacts
     }
 
-    async uploadArtifacts(artifacts: Artifact[],
+    async uploadArtifacts(artifacts: Artifact[], uploadUrl: string) {
-        releaseId: number,
+        artifacts.forEach(async artifact => {
-        uploadUrl: string): Promise<void> {
-        if(this.replacesExistingArtifacts)  {
-            await this.deleteUpdatedArtifacts(artifacts, releaseId)
-        }
-
-        for (const artifact of artifacts) {
             try {
                 await this.releases.uploadArtifact(uploadUrl,
                     artifact.contentLength,
                     artifact.contentType,
                     artifact.readFile(),
                     artifact.name)
-            } catch (error) {
+            } catch(error) {
                 const message = `Failed to upload artifact ${artifact.name}. Does it already exist?`
                 core.warning(message)
             }
-        }
-        return Promise.resolve()
-    }
-
-    async deleteUpdatedArtifacts(artifacts: Artifact[], releaseId: number) {
-        const response = await this.releases.listArtifactsForRelease(releaseId)
-        const releaseAssets = response.data
-        const assetByName = new Map<string, ReposListAssetsForReleaseResponseItem>()
-        releaseAssets.forEach(asset => {
-            assetByName[asset.name] = asset
         });
-        for (const artifact of artifacts) {
-            const asset = assetByName[artifact.name]
-            if (asset) {
-                await this.releases.deleteArtifact(asset.id)
-            }
-        }
     }
 }

src/Inputs.ts

@@ -12,7 +12,6 @@ export interface Inputs {
     readonly draft: boolean
     readonly name: string
     readonly prerelease: boolean
-    readonly replacesArtifacts: boolean
     readonly tag: string
     readonly token: string
 }
@@ -80,13 +79,8 @@ export class CoreInputs implements Inputs {
     }
 
     get prerelease(): boolean {
-        const preRelease = core.getInput('prerelease')
+        const draft = core.getInput('prerelease')
-        return preRelease == 'true'
+        return draft == 'true'
-    }
-
-    get replacesArtifacts(): boolean {
-        const replaces = core.getInput('replacesArtifacts')
-        return replaces == 'true'
     }
 
     get tag(): string {

src/Main.ts

@@ -25,7 +25,7 @@ function createAction(): Action {
 
   const inputs = new CoreInputs(globber, context)
   const releases = new GithubReleases(context, git)
-  const uploader = new GithubArtifactUploader(releases, inputs.replacesArtifacts)
+  const uploader = new GithubArtifactUploader(releases)
   return new Action(inputs, releases, uploader)
 }
 

src/Releases.ts

@@ -1,6 +1,6 @@
 import { Context } from "@actions/github/lib/context";
 import { GitHub } from "@actions/github";
-import { AnyResponse, Response, ReposDeleteReleaseAssetResponse, ReposListAssetsForReleaseResponse, ReposCreateReleaseResponse, ReposGetReleaseByTagResponse, ReposListReleasesResponse } from "@octokit/rest";
+import { AnyResponse, Response, ReposCreateReleaseResponse, ReposGetReleaseByTagResponse, ReposListReleasesResponse } from "@octokit/rest";
 
 export interface Releases {
     create(
@@ -12,12 +12,8 @@ export interface Releases {
         prerelease?: boolean
     ): Promise<Response<ReposCreateReleaseResponse>>
 
-    deleteArtifact(assetId: number): Promise<Response<ReposDeleteReleaseAssetResponse>>
-
     getByTag(tag: string): Promise<Response<ReposGetReleaseByTagResponse>>
 
-    listArtifactsForRelease(releaseId: number): Promise<Response<ReposListAssetsForReleaseResponse>>
-
     listReleases(): Promise<Response<ReposListReleasesResponse>>
 
     update(
@@ -68,26 +64,6 @@ export class GithubReleases implements Releases {
         })
     }
 
-    async deleteArtifact(
-        assetId: number
-    ): Promise<Response<ReposDeleteReleaseAssetResponse>> {
-        return this.git.repos.deleteReleaseAsset({
-            asset_id: assetId,
-            owner: this.context.repo.owner,
-            repo: this.context.repo.repo
-        })
-    }
-
-    async listArtifactsForRelease(
-        releaseId: number
-    ): Promise<Response<ReposListAssetsForReleaseResponse>> {
-        return this.git.repos.listAssetsForRelease({
-            owner: this.context.repo.owner,
-            release_id: releaseId,
-            repo: this.context.repo.repo
-        })
-    }
-
     async listReleases(): Promise<Response<ReposListReleasesResponse>> {
         return this.git.repos.listReleases({
             owner: this.context.repo.owner,