From f4bd26364a025b9033258793d22f6521c8afdaa0 Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 22 Apr 2025 13:07:54 -0700 Subject: [PATCH 1/5] fix: include cause of parsing errors in action output logs --- src/content.js | 7 ++++--- src/errors.js | 7 ++++++- src/index.js | 16 +++++++++++++--- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/content.js b/src/content.js index 6594a32d..a571a38f 100644 --- a/src/content.js +++ b/src/content.js @@ -55,6 +55,7 @@ export default class Content { * @returns {Content} - the parsed JSON payload to use in requests. */ getContentPayload(config) { + const errors = []; if (!config.inputs.payload) { throw new SlackError( config.core, @@ -71,8 +72,7 @@ export default class Content { return /** @type {Content} */ (content); } catch (error) { if (error instanceof Error) { - config.core.debug("Failed to parse input payload as YAML"); - config.core.debug(error.message); + errors.push(error); } } try { @@ -90,11 +90,12 @@ export default class Content { } return JSON.parse(trimmed); } catch (/** @type {any} */ error) { + errors.unshift(error); throw new SlackError( config.core, "Invalid input! Failed to parse contents of the provided payload", { - cause: error, + cause: { values: errors }, }, ); } diff --git a/src/errors.js b/src/errors.js index b2d4ce0c..3d0d87b1 100644 --- a/src/errors.js +++ b/src/errors.js @@ -1,12 +1,17 @@ import core from "@actions/core"; +/** + * @typedef Cause + * @property {Error[]} [values] - Caught exceptions. + */ + /** * SlackError is a custom error wrapper for known errors of Slack GitHub Action. */ export default class SlackError extends Error { /** * @typedef Options - * @property {Error} [cause] - thrown exception of this error. + * @property {Cause} [cause] - Reason for an error. */ /** diff --git a/src/index.js b/src/index.js index fcdf2277..120af89b 100644 --- a/src/index.js +++ b/src/index.js @@ -6,13 +6,23 @@ import send from "./send.js"; * from the send.js file for testing purposes. */ try { - send(core); + await send(core); } catch (error) { if (error instanceof Error) { + core.startGroup("Error") core.error(error.message); - core.debug(`${error.name} cause: ${error.cause}`); - core.debug(`${error.name} stack: ${error.stack}`); + /** @type {import('./errors.js').Cause} */ + const causes = /** @type {any} */ (error.cause); + if (causes && causes.values) { + for (const cause of causes.values) { + core.info(`${cause.stack}`); + } + } else { + core.info(`${error.stack}`); + } + core.endGroup() } else { core.error(`${error}`); } + throw error; } From cccb0cdc15faf806550fbadd254c2f538fc9707e Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 22 Apr 2025 13:10:07 -0700 Subject: [PATCH 2/5] fix: run the linter for consistent formatting applied --- src/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/index.js b/src/index.js index 120af89b..ae8edfb9 100644 --- a/src/index.js +++ b/src/index.js @@ -9,18 +9,18 @@ try { await send(core); } catch (error) { if (error instanceof Error) { - core.startGroup("Error") + core.startGroup("Error"); core.error(error.message); /** @type {import('./errors.js').Cause} */ const causes = /** @type {any} */ (error.cause); - if (causes && causes.values) { + if (causes?.values) { for (const cause of causes.values) { core.info(`${cause.stack}`); } } else { core.info(`${error.stack}`); } - core.endGroup() + core.endGroup(); } else { core.error(`${error}`); } From 7a98fc3c2078ab83a78a4a7a4a1cdff47e410b3b Mon Sep 17 00:00:00 2001 From: Eden Zimbelman Date: Tue, 22 Apr 2025 13:21:53 -0700 Subject: [PATCH 3/5] feat: remove error groups that hide outputs as a default --- src/index.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/index.js b/src/index.js index ae8edfb9..ff3b701b 100644 --- a/src/index.js +++ b/src/index.js @@ -9,7 +9,6 @@ try { await send(core); } catch (error) { if (error instanceof Error) { - core.startGroup("Error"); core.error(error.message); /** @type {import('./errors.js').Cause} */ const causes = /** @type {any} */ (error.cause); @@ -20,7 +19,6 @@ try { } else { core.info(`${error.stack}`); } - core.endGroup(); } else { core.error(`${error}`); } From 20f425b6c540bf0877cbcf3a2d30d5977bba0a15 Mon Sep 17 00:00:00 2001 From: "@zimeg" Date: Wed, 30 Apr 2025 21:07:36 -0700 Subject: [PATCH 4/5] test: confirm causes of error are thrown in an expected order --- test/content.spec.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/content.spec.js b/test/content.spec.js index be279d44..be78328a 100644 --- a/test/content.spec.js +++ b/test/content.spec.js @@ -1,6 +1,7 @@ import path from "node:path"; import core from "@actions/core"; import { assert } from "chai"; +import { YAMLException } from "js-yaml"; import Config from "../src/config.js"; import Content from "../src/content.js"; import SlackError from "../src/errors.js"; @@ -190,6 +191,11 @@ describe("content", () => { err.message, "Invalid input! Failed to parse contents of the provided payload", ); + assert.isDefined(err.cause?.values); + assert.equal(err.cause.values.length, 2); + const [jsonError, yamlError] = err.cause.values; + assert.isTrue(jsonError instanceof SyntaxError); + assert.isTrue(yamlError instanceof YAMLException); } else { assert.fail("Failed to throw a SlackError", err); } From 5b4b1bce03c094702abdc1e709d5ce6393e9ef37 Mon Sep 17 00:00:00 2001 From: "@zimeg" Date: Wed, 30 Apr 2025 21:53:56 -0700 Subject: [PATCH 5/5] fix: set the cause of file parsing errors to the error value --- src/content.js | 2 +- test/content.spec.js | 51 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/src/content.js b/src/content.js index a571a38f..e9506a09 100644 --- a/src/content.js +++ b/src/content.js @@ -141,7 +141,7 @@ export default class Content { config.core, "Invalid input! Failed to parse contents of the provided payload file", { - cause: error, + cause: { values: [error] }, }, ); } diff --git a/test/content.spec.js b/test/content.spec.js index be78328a..30a37286 100644 --- a/test/content.spec.js +++ b/test/content.spec.js @@ -340,8 +340,10 @@ describe("content", () => { err.message, "Invalid input! Failed to parse contents of the provided payload file", ); + assert.isDefined(err.cause?.values); + assert.equal(err.cause.values.length, 1); assert.include( - err.cause.message, + err.cause.values[0].message, "Invalid input! Failed to parse file extension unknown.md", ); } else { @@ -349,5 +351,52 @@ describe("content", () => { } } }); + + it("fails if invalid JSON exists in the input payload", async () => { + mocks.core.getInput.withArgs("payload-file-path").returns("example.json"); + mocks.fs.readFileSync + .withArgs(path.resolve("example.json"), "utf-8") + .returns(`{ + "message": "a truncated file without an end`); + try { + await send(mocks.core); + assert.fail("Failed to throw for invalid JSON"); + } catch (err) { + if (err instanceof SlackError) { + assert.include( + err.message, + "Invalid input! Failed to parse contents of the provided payload file", + ); + assert.isDefined(err.cause?.values); + assert.equal(err.cause.values.length, 1); + assert.isTrue(err.cause.values[0] instanceof SyntaxError); + } else { + assert.fail("Failed to throw a SlackError", err); + } + } + }); + + it("fails if invalid YAML exists in the input payload", async () => { + mocks.core.getInput.withArgs("payload-file-path").returns("example.yaml"); + mocks.fs.readFileSync + .withArgs(path.resolve("example.yaml"), "utf-8") + .returns(`- "message": "assigned": "values"`); + try { + await send(mocks.core); + assert.fail("Failed to throw for invalid YAML"); + } catch (err) { + if (err instanceof SlackError) { + assert.include( + err.message, + "Invalid input! Failed to parse contents of the provided payload file", + ); + assert.isDefined(err.cause?.values); + assert.equal(err.cause.values.length, 1); + assert.isTrue(err.cause.values[0] instanceof YAMLException); + } else { + assert.fail("Failed to throw a SlackError", err); + } + } + }); }); });