Fixes #33 Add artifactErrorsFailBuild flag

README.md

@@ -4,6 +4,7 @@ This action will create a github release and optionally upload an artifact to it
 
 ## Action Inputs
 - **allowUpdates**: An optional flag which indicates if we should update a release if it already exists. Defaults to false.
+- **artifactErrorsFailBuild**: An optional flag which indicates if artifact read or upload errors should fail the build.  
 - **artifact**: An optional set of paths representing artifacts to upload to the release. This may be a single path or a comma delimited list of paths (or globs).
 - **artifacts**: An optional set of paths representing artifacts to upload to the release. This may be a single path or a comma delimited list of paths (or globs).
 - **artifactContentType**: The content type of the artifact. Defaults to raw.

__tests__/Action.test.ts

@@ -17,6 +17,7 @@ const artifacts = [
     new Artifact('b/art2')
 ]
 
+const artifactErrorsFailBuild = false
 const createBody = 'createBody'
 const createName = 'createName'
 const commit = 'commit'
@@ -239,6 +240,7 @@ describe("Action", () => {
         const MockInputs = jest.fn<Inputs, any>(() => {
             return {
                 allowUpdates: allowUpdates,
+                artifactErrorsFailBuild: true,
                 artifacts: inputArtifact,
                 createdReleaseBody: createBody,
                 createdReleaseName: createName,

__tests__/ArtifactGlobber.test.ts

@@ -1,8 +1,8 @@
 const warnMock = jest.fn()
 
-import { FileArtifactGlobber } from "../src/ArtifactGlobber"
+import {FileArtifactGlobber} from "../src/ArtifactGlobber"
-import { Globber } from "../src/Globber";
+import {Globber} from "../src/Globber";
-import { Artifact } from "../src/Artifact";
+import {Artifact} from "../src/Artifact";
 import untildify = require("untildify");
 
 const contentType = "raw"
@@ -24,19 +24,19 @@ describe("ArtifactGlobber", () => {
         const expectedArtifacts =
             globResults.map((path) => new Artifact(path, contentType))
 
-        expect(globber.globArtifactString('~/path', 'raw'))
+        expect(globber.globArtifactString('~/path', 'raw', false))
             .toEqual(expectedArtifacts)
         expect(globMock).toBeCalledWith(untildify('~/path'))
         expect(warnMock).not.toBeCalled()
     })
-    
+
     it("globs simple path", () => {
         const globber = createArtifactGlobber()
 
         const expectedArtifacts =
             globResults.map((path) => new Artifact(path, contentType))
-        
+
-        expect(globber.globArtifactString('path', 'raw'))
+        expect(globber.globArtifactString('path', 'raw', false))
             .toEqual(expectedArtifacts)
         expect(globMock).toBeCalledWith('path')
         expect(warnMock).not.toBeCalled()
@@ -50,24 +50,28 @@ describe("ArtifactGlobber", () => {
                 .concat(globResults)
                 .map((path) => new Artifact(path, contentType))
 
-        expect(globber.globArtifactString('path1,path2', 'raw'))
+        expect(globber.globArtifactString('path1,path2', 'raw', false))
             .toEqual(expectedArtifacts)
         expect(globMock).toBeCalledWith('path1')
         expect(globMock).toBeCalledWith('path2')
         expect(warnMock).not.toBeCalled()
     })
 
-    it("warns when no glob results are produced", () => {
+    it("warns when no glob results are produced and empty results shouldn't throw", () => {
         const globber = createArtifactGlobber([])
 
-        const expectedArtifacts =
+        expect(globber.globArtifactString('path', 'raw', false))
-            globResults.map((path) => new Artifact(path, contentType))
-
-        expect(globber.globArtifactString('path', 'raw'))
             .toEqual([])
         expect(warnMock).toBeCalled()
     })
 
+    it("throws when no glob results are produced and empty results shouild throw", () => {
+        const globber = createArtifactGlobber([])
+        expect(() => {
+            globber.globArtifactString('path', 'raw', true)
+        }).toThrow()
+    })
+
     function createArtifactGlobber(results: string[] = globResults): FileArtifactGlobber {
         const MockGlobber = jest.fn<Globber, any>(() => {
             return {

__tests__/ArtifactUploader.test.ts

@@ -1,7 +1,7 @@
-import { Artifact } from "../src/Artifact"
+import {Artifact} from "../src/Artifact"
-import { GithubArtifactUploader } from "../src/ArtifactUploader"
+import {GithubArtifactUploader} from "../src/ArtifactUploader"
-import { Releases } from "../src/Releases";
+import {Releases} from "../src/Releases";
-import { RequestError } from '@octokit/request-error'
+import {RequestError} from '@octokit/request-error'
 
 const artifacts = [
     new Artifact('a/art1'),
@@ -19,7 +19,9 @@ const uploadMock = jest.fn()
 jest.mock('fs', () => {
     return {
         readFileSync: () => fileContents,
-        statSync: () => { return { size: contentLength } }
+        statSync: () => {
+            return {size: contentLength}
+        }
     };
 })
 
@@ -124,6 +126,19 @@ describe('ArtifactUploader', () => {
         expect(deleteMock).toBeCalledTimes(0)
     })
 
+    it('throws upload error when replacesExistingArtifacts is true', async () => {
+        mockListWithoutAssets()
+        mockUploadError()
+        const uploader = createUploader(true, true)
+
+        expect.hasAssertions()
+        try {
+            await uploader.uploadArtifacts(artifacts, releaseId, url)
+        } catch (error) {
+            expect(error).toEqual(Error("Failed to upload artifact art1. error."))
+        }
+    })
+
     it('throws error from replace', async () => {
         mockDeleteError()
         mockListWithAssets()
@@ -155,7 +170,7 @@ describe('ArtifactUploader', () => {
         expect(deleteMock).toBeCalledTimes(0)
     })
 
-    function createUploader(replaces: boolean): GithubArtifactUploader {
+    function createUploader(replaces: boolean, throws: boolean = false): GithubArtifactUploader {
         const MockReleases = jest.fn<Releases, any>(() => {
             return {
                 create: jest.fn(),
@@ -167,7 +182,7 @@ describe('ArtifactUploader', () => {
                 uploadArtifact: uploadMock
             }
         })
-        return new GithubArtifactUploader(new MockReleases(), replaces)
+        return new GithubArtifactUploader(new MockReleases(), replaces, throws)
     }
 
     function mockDeleteError(): any {
@@ -194,14 +209,24 @@ describe('ArtifactUploader', () => {
     }
 
     function mockListWithoutAssets() {
-        listArtifactsMock.mockResolvedValue({ data: [] })
+        listArtifactsMock.mockResolvedValue({data: []})
     }
 
     function mockUploadArtifact(status: number = 200, failures: number = 0) {
-        const error = new RequestError(`HTTP ${status}`, status, { headers: {}, request: { method: 'GET', url: '', headers: {} } })
+        const error = new RequestError(`HTTP ${status}`, status, {
+            headers: {},
+            request: {method: 'GET', url: '', headers: {}}
+        })
         for (let index = 0; index < failures; index++) {
             uploadMock.mockRejectedValueOnce(error)
         }
         uploadMock.mockResolvedValue({})
     }
+
+    function mockUploadError() {
+        uploadMock.mockRejectedValue({
+            message: "error",
+            status: 502
+        })
+    }
 });

__tests__/Inputs.test.ts

@@ -59,7 +59,28 @@ describe('Inputs', () => {
         })
     })
 
+    describe('artifactErrorsFailBuild', () => {
+        it('returns false', () => {
+            expect(inputs.artifactErrorsFailBuild).toBe(false)
+        })
+
+        it('returns true', () => {
+            mockGetInput.mockReturnValue('true')
+            expect(inputs.artifactErrorsFailBuild).toBe(true)
+        })
+    })
+
     describe('artifacts', () => {
+        it('globber told to throw errors', () => {
+            mockGetInput.mockReturnValueOnce('art1')
+                .mockReturnValueOnce('contentType')
+                .mockReturnValueOnce('true')
+
+            expect(inputs.artifacts).toEqual(artifacts)
+            expect(mockGlob).toBeCalledTimes(1)
+            expect(mockGlob).toBeCalledWith('art1', 'contentType', true)
+        })
+        
         it('returns empty artifacts', () => {
             mockGetInput.mockReturnValueOnce('')
                 .mockReturnValueOnce('')
@@ -71,28 +92,32 @@ describe('Inputs', () => {
         it('returns input.artifacts', () => {
             mockGetInput.mockReturnValueOnce('art1')
                 .mockReturnValueOnce('contentType')
+                .mockReturnValueOnce('false')
 
             expect(inputs.artifacts).toEqual(artifacts)
             expect(mockGlob).toBeCalledTimes(1)
-            expect(mockGlob).toBeCalledWith('art1', 'contentType')
+            expect(mockGlob).toBeCalledWith('art1', 'contentType', false)
         })
 
         it('returns input.artifacts with default contentType', () => {
             mockGetInput.mockReturnValueOnce('art1')
+                .mockReturnValueOnce('raw')
+                .mockReturnValueOnce('false')
 
             expect(inputs.artifacts).toEqual(artifacts)
             expect(mockGlob).toBeCalledTimes(1)
-            expect(mockGlob).toBeCalledWith('art1', 'raw')
+            expect(mockGlob).toBeCalledWith('art1', 'raw', false)
         })
 
         it('returns input.artifact', () => {
             mockGetInput.mockReturnValueOnce('')
                 .mockReturnValueOnce('art2')
                 .mockReturnValueOnce('contentType')
+                .mockReturnValueOnce('false')
 
             expect(inputs.artifacts).toEqual(artifacts)
             expect(mockGlob).toBeCalledTimes(1)
-            expect(mockGlob).toBeCalledWith('art2', 'contentType')
+            expect(mockGlob).toBeCalledWith('art2', 'contentType', false)
         })
     })
 
@@ -164,7 +189,7 @@ describe('Inputs', () => {
             expect(inputs.draft).toBe(true)
         })
     })
-    
+
     describe('owner', () => {
         it('returns owner from context', function () {
             process.env.GITHUB_REPOSITORY = "owner/repo"
@@ -175,7 +200,7 @@ describe('Inputs', () => {
             mockGetInput.mockReturnValue("owner")
             expect(inputs.owner).toBe("owner")
         });
-    })    
+    })
 
     describe('prerelase', () => {
         it('returns false', () => {

__tests__/Integration.test.ts

@@ -18,7 +18,11 @@ describe.skip('Integration Test', () => {
 
         const inputs = getInputs()
         const releases = new GithubReleases(inputs, git)
-        const uploader = new GithubArtifactUploader(releases, inputs.replacesArtifacts)
+        const uploader = new GithubArtifactUploader(
+            releases,
+            inputs.artifactErrorsFailBuild,
+            inputs.replacesArtifacts
+        )
         action = new Action(inputs, releases, uploader)
     })
 
@@ -30,6 +34,7 @@ describe.skip('Integration Test', () => {
         const MockInputs = jest.fn<Inputs, any>(() => {
             return {
                 allowUpdates: true,
+                artifactErrorsFailBuild: false,
                 artifacts: artifacts(),
                 createdReleaseBody: "This release was generated by release-action's integration test",
                 createdReleaseName: "Releases Action Integration Test",
@@ -47,12 +52,12 @@ describe.skip('Integration Test', () => {
         })
         return new MockInputs();
     }
-    
+
     function artifacts() {
         const globber = new FileArtifactGlobber()
         const artifactPath = path.join(__dirname, 'Integration.test.ts')
         const artifactString = `~/Desktop/test.txt,blarg.tx, ${artifactPath}`
-        return globber.globArtifactString(artifactString, "raw")
+        return globber.globArtifactString(artifactString, "raw", false)
     }
 
     function getToken(): string {

action.yml

@@ -6,6 +6,10 @@ inputs:
     description: 'An optional flag which indicates if we should update a release if it already exists. Defaults to false.'
     required: false
     default: ''
+  artifactErrorsFailBuild:
+    description: 'An optional flag which indicates if artifact read or upload errors should fail the build.'
+    required: false
+    default: ''
   artifact:
     deprecationMessage: Use 'artifacts' instead.
     description: 'An optional set of paths representing artifacts to upload to the release. This may be a single path or a comma delimited list of paths (or globs)'

lib/Action.js

@@ -80,8 +80,7 @@ class Action {
     }
     createRelease() {
         return __awaiter(this, void 0, void 0, function* () {
-            const response = yield this.releases.create(this.inputs.tag, this.inputs.createdReleaseBody, this.inputs.commit, this.inputs.draft, this.inputs.createdReleaseName, this.inputs.prerelease);
+            return yield this.releases.create(this.inputs.tag, this.inputs.createdReleaseBody, this.inputs.commit, this.inputs.draft, this.inputs.createdReleaseName, this.inputs.prerelease);
-            return response;
         });
     }
 }

lib/ArtifactGlobber.js

@@ -31,23 +31,31 @@ class FileArtifactGlobber {
     constructor(globber = new Globber_1.FileGlobber()) {
         this.globber = globber;
     }
-    globArtifactString(artifact, contentType) {
+    globArtifactString(artifact, contentType, throwsWhenNoFiles) {
         return artifact.split(',')
             .map(path => FileArtifactGlobber.expandPath(path))
-            .map(pattern => this.globPattern(pattern))
+            .map(pattern => this.globPattern(pattern, throwsWhenNoFiles))
             .reduce((accumulated, current) => accumulated.concat(current))
             .map(path => new Artifact_1.Artifact(path, contentType));
     }
-    globPattern(pattern) {
+    globPattern(pattern, throwsWhenNoFiles) {
         const paths = this.globber.glob(pattern);
         if (paths.length == 0) {
-            FileArtifactGlobber.reportGlobWarning(pattern);
+            if (throwsWhenNoFiles) {
+                FileArtifactGlobber.throwGlobError(pattern);
+            }
+            else {
+                FileArtifactGlobber.reportGlobWarning(pattern);
+            }
         }
         return paths;
     }
     static reportGlobWarning(pattern) {
         core.warning(`Artifact pattern :${pattern} did not match any files`);
     }
+    static throwGlobError(pattern) {
+        throw Error(`Artifact pattern :${pattern} did not match any files`);
+    }
     static expandPath(path) {
         return untildify_1.default(path);
     }

lib/ArtifactUploader.js

@@ -31,9 +31,10 @@ Object.defineProperty(exports, "__esModule", { value: true });
 exports.GithubArtifactUploader = void 0;
 const core = __importStar(require("@actions/core"));
 class GithubArtifactUploader {
-    constructor(releases, replacesExistingArtifacts = true) {
+    constructor(releases, replacesExistingArtifacts = true, throwsUploadErrors = false) {
         this.releases = releases;
         this.replacesExistingArtifacts = replacesExistingArtifacts;
+        this.throwsUploadErrors = throwsUploadErrors;
     }
     uploadArtifacts(artifacts, releaseId, uploadUrl) {
         return __awaiter(this, void 0, void 0, function* () {
@@ -57,7 +58,12 @@ class GithubArtifactUploader {
                     yield this.uploadArtifact(artifact, releaseId, uploadUrl, retry - 1);
                 }
                 else {
-                    core.warning(`Failed to upload artifact ${artifact.name}. ${error.message}.`);
+                    if (this.throwsUploadErrors) {
+                        throw Error(`Failed to upload artifact ${artifact.name}. ${error.message}.`);
+                    }
+                    else {
+                        core.warning(`Failed to upload artifact ${artifact.name}. ${error.message}.`);
+                    }
                 }
             }
         });

lib/Inputs.js

@@ -42,10 +42,14 @@ class CoreInputs {
                 contentType = 'raw';
             }
             return this.artifactGlobber
-                .globArtifactString(artifacts, contentType);
+                .globArtifactString(artifacts, contentType, this.artifactErrorsFailBuild);
         }
         return [];
     }
+    get artifactErrorsFailBuild() {
+        const allow = core.getInput('artifactErrorsFailBuild');
+        return allow == 'true';
+    }
     get createdReleaseBody() {
         if (CoreInputs.omitBody)
             return undefined;

src/Action.ts

@@ -78,7 +78,7 @@ export class Action {
     }
 
     private async createRelease(): Promise<CreateReleaseResponse> {
-        const response = await this.releases.create(
+        return await this.releases.create(
             this.inputs.tag,
             this.inputs.createdReleaseBody,
             this.inputs.commit,
@@ -86,7 +86,5 @@ export class Action {
             this.inputs.createdReleaseName,
             this.inputs.prerelease
         )
-
-        return response
     }
 }

src/ArtifactGlobber.ts

@@ -4,7 +4,7 @@ import {Artifact} from "./Artifact";
 import untildify from "untildify";
 
 export interface ArtifactGlobber {
-    globArtifactString(artifact: string, contentType: string): Artifact[]
+    globArtifactString(artifact: string, contentType: string, throwsWhenNoFiles: boolean): Artifact[]
 }
 
 export class FileArtifactGlobber implements ArtifactGlobber {
@@ -14,18 +14,22 @@ export class FileArtifactGlobber implements ArtifactGlobber {
         this.globber = globber
     }
 
-    globArtifactString(artifact: string, contentType: string): Artifact[] {
+    globArtifactString(artifact: string, contentType: string, throwsWhenNoFiles: boolean): Artifact[] {
         return artifact.split(',')
             .map(path => FileArtifactGlobber.expandPath(path))
-            .map(pattern => this.globPattern(pattern))
+            .map(pattern => this.globPattern(pattern, throwsWhenNoFiles))
             .reduce((accumulated, current) => accumulated.concat(current))
             .map(path => new Artifact(path, contentType))
     }
 
-    private globPattern(pattern: string): string[] {
+    private globPattern(pattern: string, throwsWhenNoFiles: boolean): string[] {
         const paths = this.globber.glob(pattern)
         if (paths.length == 0) {
-            FileArtifactGlobber.reportGlobWarning(pattern)
+            if (throwsWhenNoFiles) {
+                FileArtifactGlobber.throwGlobError(pattern)
+            } else {
+                FileArtifactGlobber.reportGlobWarning(pattern)
+            }
         }
         return paths
     }
@@ -34,6 +38,10 @@ export class FileArtifactGlobber implements ArtifactGlobber {
         core.warning(`Artifact pattern :${pattern} did not match any files`)
     }
 
+    private static throwGlobError(pattern: string) {
+        throw Error(`Artifact pattern :${pattern} did not match any files`)
+    }
+
     private static expandPath(path: string): string {
         return untildify(path)
     }

src/ArtifactUploader.ts

@@ -10,6 +10,7 @@ export class GithubArtifactUploader implements ArtifactUploader {
     constructor(
         private releases: Releases,
         private replacesExistingArtifacts: boolean = true,
+        private throwsUploadErrors: boolean = false,
     ) {
     }
 
@@ -41,7 +42,11 @@ export class GithubArtifactUploader implements ArtifactUploader {
                 core.warning(`Failed to upload artifact ${artifact.name}. ${error.message}. Retrying...`)
                 await this.uploadArtifact(artifact, releaseId, uploadUrl, retry - 1)
             } else {
-                core.warning(`Failed to upload artifact ${artifact.name}. ${error.message}.`)
+                if (this.throwsUploadErrors) {
+                    throw Error(`Failed to upload artifact ${artifact.name}. ${error.message}.`)
+                } else {
+                    core.warning(`Failed to upload artifact ${artifact.name}. ${error.message}.`)
+                }
             }
         }
     }

src/Inputs.ts

@@ -6,6 +6,7 @@ import {Artifact} from './Artifact';
 
 export interface Inputs {
     readonly allowUpdates: boolean
+    readonly artifactErrorsFailBuild: boolean
     readonly artifacts: Artifact[]
     readonly commit: string
     readonly createdReleaseBody?: string
@@ -46,11 +47,16 @@ export class CoreInputs implements Inputs {
                 contentType = 'raw'
             }
             return this.artifactGlobber
-                .globArtifactString(artifacts, contentType)
+                .globArtifactString(artifacts, contentType, this.artifactErrorsFailBuild)
         }
         return []
     }
 
+    get artifactErrorsFailBuild(): boolean {
+        const allow = core.getInput('artifactErrorsFailBuild')
+        return allow == 'true'
+    }
+
     get createdReleaseBody(): string | undefined {
         if (CoreInputs.omitBody) return undefined
         return this.body