From c3d6a9ee2e9cc7fb577a8bac55226b10469fd3ab Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 10:56:30 +0200 Subject: [PATCH 01/12] fix(exec): fix crash on artifacts copying errors Add missing handler for `CpyError`. Do not crash, throw a `RuntimeError` instead. --- core/src/plugins/exec/common.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/core/src/plugins/exec/common.ts b/core/src/plugins/exec/common.ts index f98c311005..8e648a9fc4 100644 --- a/core/src/plugins/exec/common.ts +++ b/core/src/plugins/exec/common.ts @@ -16,6 +16,7 @@ import { exec } from "../../util/util.js" import type { Log } from "../../logger/log-entry.js" import type { PluginContext } from "../../plugin-context.js" import type { ResolvedExecAction } from "./config.js" +import { RuntimeError } from "../../exceptions.js" export function getDefaultEnvVars(action: ResolvedExecAction) { return { @@ -99,12 +100,22 @@ export async function copyArtifacts( ) { return Promise.all( (artifacts || []).map(async (spec) => { - log.verbose(`→ Copying artifacts ${spec.source}`) + try { + log.verbose(`→ Copying artifacts ${spec.source}`) - // Note: lazy-loading for startup performance - const { default: cpy } = await import("cpy") + // Note: lazy-loading for startup performance + const { default: cpy } = await import("cpy") - await cpy(`./${spec.source}`, join(artifactsPath, spec.target || "."), { cwd: from }) + await cpy(`./${spec.source}`, join(artifactsPath, spec.target || "."), { cwd: from }) + } catch (err: unknown) { + if (!(err instanceof Error)) { + throw err + } + + if (err.name === "CpyError") { + throw new RuntimeError({ message: err.message }) + } + } }) ) } From 7075d8bd56f46f5b0b2b5f8fb46ae130b12a3472 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:10:24 +0200 Subject: [PATCH 02/12] fix(exec): print command output after its execution Otherwise, it won't appear if case of artifacts copying error --- core/src/plugins/exec/test.ts | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts index 0315a0593e..3d7a775aa5 100644 --- a/core/src/plugins/exec/test.ts +++ b/core/src/plugins/exec/test.ts @@ -37,8 +37,16 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { const result = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) - const artifacts = action.getSpec("artifacts") - await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath) + const detail = { + moduleName: action.moduleName(), + command, + testName: action.name, + version: action.versionString(), + success: result.success, + startedAt, + completedAt: result.completedAt, + log: result.outputLog, + } if (result.outputLog) { const prefix = `Finished executing ${styles.highlight(action.key())}. Here is the full output:` @@ -52,16 +60,8 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { ) } - const detail = { - moduleName: action.moduleName(), - command, - testName: action.name, - version: action.versionString(), - success: result.success, - startedAt, - completedAt: result.completedAt, - log: result.outputLog, - } + const artifacts = action.getSpec("artifacts") + await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath) return { state: runResultToActionState(detail), From ad5dd3c9ac7fa47186e60201a489d8dfa0f27cc6 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:25:00 +0200 Subject: [PATCH 03/12] refactor: rename local variable --- core/src/plugins/exec/test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts index 3d7a775aa5..e01ae61559 100644 --- a/core/src/plugins/exec/test.ts +++ b/core/src/plugins/exec/test.ts @@ -35,26 +35,26 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { const startedAt = new Date() const { command, env } = action.getSpec() - const result = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) + const execCommandOutputs = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) const detail = { moduleName: action.moduleName(), command, testName: action.name, version: action.versionString(), - success: result.success, + success: execCommandOutputs.success, startedAt, - completedAt: result.completedAt, - log: result.outputLog, + completedAt: execCommandOutputs.completedAt, + log: execCommandOutputs.outputLog, } - if (result.outputLog) { + if (execCommandOutputs.outputLog) { const prefix = `Finished executing ${styles.highlight(action.key())}. Here is the full output:` log.info( renderMessageWithDivider({ prefix, - msg: result.outputLog, - isError: !result.success, + msg: execCommandOutputs.outputLog, + isError: !execCommandOutputs.success, color: styles.primary, }) ) @@ -67,7 +67,7 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { state: runResultToActionState(detail), detail, outputs: { - log: result.outputLog, + log: execCommandOutputs.outputLog, }, } }) From 4d2995850784ef8cca8d984d93f686f019cf3819 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:26:19 +0200 Subject: [PATCH 04/12] fix(exec): return immediately if the test command fail Otherwise, a confusing error message on missing artifacts may appear. This also prevents duplicate logging of command output. --- core/src/plugins/exec/test.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts index e01ae61559..035415b672 100644 --- a/core/src/plugins/exec/test.ts +++ b/core/src/plugins/exec/test.ts @@ -48,6 +48,16 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { log: execCommandOutputs.outputLog, } + if (!execCommandOutputs.success) { + return { + state: runResultToActionState(detail), + detail, + outputs: { + log: execCommandOutputs.outputLog, + }, + } + } + if (execCommandOutputs.outputLog) { const prefix = `Finished executing ${styles.highlight(action.key())}. Here is the full output:` log.info( From 43b88a8bde6cd07a9d1f941b608bc3656d277c3c Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:30:00 +0200 Subject: [PATCH 05/12] refactor: extract local variable --- core/src/plugins/exec/test.ts | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts index 035415b672..6b4bc256f2 100644 --- a/core/src/plugins/exec/test.ts +++ b/core/src/plugins/exec/test.ts @@ -48,14 +48,16 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { log: execCommandOutputs.outputLog, } + const result = { + state: runResultToActionState(detail), + detail, + outputs: { + log: execCommandOutputs.outputLog, + }, + } as const + if (!execCommandOutputs.success) { - return { - state: runResultToActionState(detail), - detail, - outputs: { - log: execCommandOutputs.outputLog, - }, - } + return result } if (execCommandOutputs.outputLog) { @@ -73,11 +75,5 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { const artifacts = action.getSpec("artifacts") await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath) - return { - state: runResultToActionState(detail), - detail, - outputs: { - log: execCommandOutputs.outputLog, - }, - } + return result }) From 41b360477b7bbc93f2121ac17b4e0e79e2a94a3a Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:46:16 +0200 Subject: [PATCH 06/12] refactor: rename local variable --- core/src/plugins/exec/test.ts | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts index 6b4bc256f2..61d74194fa 100644 --- a/core/src/plugins/exec/test.ts +++ b/core/src/plugins/exec/test.ts @@ -33,46 +33,45 @@ export type ExecTest = GardenSdkActionDefinitionActionType execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { const startedAt = new Date() - const { command, env } = action.getSpec() + const { command, env, artifacts } = action.getSpec() - const execCommandOutputs = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) + const commandResult = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) const detail = { moduleName: action.moduleName(), command, testName: action.name, version: action.versionString(), - success: execCommandOutputs.success, + success: commandResult.success, startedAt, - completedAt: execCommandOutputs.completedAt, - log: execCommandOutputs.outputLog, + completedAt: commandResult.completedAt, + log: commandResult.outputLog, } const result = { state: runResultToActionState(detail), detail, outputs: { - log: execCommandOutputs.outputLog, + log: commandResult.outputLog, }, } as const - if (!execCommandOutputs.success) { + if (!commandResult.success) { return result } - if (execCommandOutputs.outputLog) { + if (commandResult.outputLog) { const prefix = `Finished executing ${styles.highlight(action.key())}. Here is the full output:` log.info( renderMessageWithDivider({ prefix, - msg: execCommandOutputs.outputLog, - isError: !execCommandOutputs.success, + msg: commandResult.outputLog, + isError: !commandResult.success, color: styles.primary, }) ) } - const artifacts = action.getSpec("artifacts") await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath) return result From d232610e73e9a137a4226e92c30e6b6976aa1e79 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:50:31 +0200 Subject: [PATCH 07/12] refactor: reorder fields declaration To have the same order as for run actions. --- core/src/plugins/exec/test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts index 61d74194fa..269d58dec3 100644 --- a/core/src/plugins/exec/test.ts +++ b/core/src/plugins/exec/test.ts @@ -39,13 +39,13 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => { const detail = { moduleName: action.moduleName(), - command, testName: action.name, + command, version: action.versionString(), success: commandResult.success, + log: commandResult.outputLog, startedAt, completedAt: commandResult.completedAt, - log: commandResult.outputLog, } const result = { From 0a4ad58015b41759c341d8e5b4131113a520a4a2 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:51:21 +0200 Subject: [PATCH 08/12] fix(exec): align the error handling of exec run and test commands --- core/src/plugins/exec/run.ts | 63 ++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 35 deletions(-) diff --git a/core/src/plugins/exec/run.ts b/core/src/plugins/exec/run.ts index 2d9c46fab7..6e4f547278 100644 --- a/core/src/plugins/exec/run.ts +++ b/core/src/plugins/exec/run.ts @@ -30,57 +30,50 @@ export type ExecRunConfig = GardenSdkActionDefinitionConfigType export type ExecRun = GardenSdkActionDefinitionActionType execRun.addHandler("run", async ({ artifactsPath, log, action, ctx }) => { - const { command, env, artifacts } = action.getSpec() const startedAt = new Date() + const { command, env, artifacts } = action.getSpec() - let completedAt: Date - let outputLog: string - let success = true - - if (command && command.length) { - const commandResult = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) - - completedAt = commandResult.completedAt - outputLog = commandResult.outputLog - success = commandResult.success - } else { - completedAt = startedAt - outputLog = "" - } - - if (outputLog) { - const prefix = `Finished running ${styles.highlight(action.name)}. Here is the full output:` - log.info( - renderMessageWithDivider({ - prefix, - msg: outputLog, - isError: !success, - color: styles.primary, - }) - ) - } - - await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath) + const commandResult = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } }) const detail = { moduleName: action.moduleName(), taskName: action.name, command, version: action.versionString(), - success, - log: outputLog, + success: commandResult.success, + log: commandResult.outputLog, outputs: { - log: outputLog, + log: commandResult.outputLog, }, startedAt, - completedAt, + completedAt: commandResult.completedAt, } - return { + const result = { state: runResultToActionState(detail), detail, outputs: { - log: outputLog, + log: commandResult.outputLog, }, + } as const + + if (!commandResult.success) { + return result + } + + if (commandResult.outputLog) { + const prefix = `Finished running ${styles.highlight(action.name)}. Here is the full output:` + log.info( + renderMessageWithDivider({ + prefix, + msg: commandResult.outputLog, + isError: !commandResult.success, + color: styles.primary, + }) + ) } + + await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath) + + return result }) From f99e2bda168f25b3c1bbb94587bd252086f4bb22 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:52:06 +0200 Subject: [PATCH 09/12] chore(exec): log action key instead of name in exec run --- core/src/plugins/exec/run.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/plugins/exec/run.ts b/core/src/plugins/exec/run.ts index 6e4f547278..f9c8688fc2 100644 --- a/core/src/plugins/exec/run.ts +++ b/core/src/plugins/exec/run.ts @@ -62,7 +62,7 @@ execRun.addHandler("run", async ({ artifactsPath, log, action, ctx }) => { } if (commandResult.outputLog) { - const prefix = `Finished running ${styles.highlight(action.name)}. Here is the full output:` + const prefix = `Finished executing ${styles.highlight(action.key())}. Here is the full output:` log.info( renderMessageWithDivider({ prefix, From 157a7cface6354a76c2d7a3110afada15f3e2073 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 11:57:35 +0200 Subject: [PATCH 10/12] chore(exec): remove duplicate logs from result object --- core/src/plugins/exec/run.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/core/src/plugins/exec/run.ts b/core/src/plugins/exec/run.ts index f9c8688fc2..edfe3a01cd 100644 --- a/core/src/plugins/exec/run.ts +++ b/core/src/plugins/exec/run.ts @@ -42,9 +42,6 @@ execRun.addHandler("run", async ({ artifactsPath, log, action, ctx }) => { version: action.versionString(), success: commandResult.success, log: commandResult.outputLog, - outputs: { - log: commandResult.outputLog, - }, startedAt, completedAt: commandResult.completedAt, } From 64d2dde73f20bb9faab1cd4e34bfd66bb54c2b3a Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 17:03:12 +0200 Subject: [PATCH 11/12] fix: propagate original error of unknown type --- core/src/plugins/exec/common.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/plugins/exec/common.ts b/core/src/plugins/exec/common.ts index 8e648a9fc4..6d403e3a56 100644 --- a/core/src/plugins/exec/common.ts +++ b/core/src/plugins/exec/common.ts @@ -16,7 +16,7 @@ import { exec } from "../../util/util.js" import type { Log } from "../../logger/log-entry.js" import type { PluginContext } from "../../plugin-context.js" import type { ResolvedExecAction } from "./config.js" -import { RuntimeError } from "../../exceptions.js" +import { RuntimeError, toGardenError } from "../../exceptions.js" export function getDefaultEnvVars(action: ResolvedExecAction) { return { @@ -115,6 +115,8 @@ export async function copyArtifacts( if (err.name === "CpyError") { throw new RuntimeError({ message: err.message }) } + + throw err } }) ) From 5322dcea402cb219165199ad1c21d990e683d743 Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev <10628074+vvagaytsev@users.noreply.github.com> Date: Thu, 25 Jul 2024 17:17:35 +0200 Subject: [PATCH 12/12] chore: linting --- core/src/plugins/exec/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/plugins/exec/common.ts b/core/src/plugins/exec/common.ts index 6d403e3a56..b0a1086051 100644 --- a/core/src/plugins/exec/common.ts +++ b/core/src/plugins/exec/common.ts @@ -16,7 +16,7 @@ import { exec } from "../../util/util.js" import type { Log } from "../../logger/log-entry.js" import type { PluginContext } from "../../plugin-context.js" import type { ResolvedExecAction } from "./config.js" -import { RuntimeError, toGardenError } from "../../exceptions.js" +import { RuntimeError } from "../../exceptions.js" export function getDefaultEnvVars(action: ResolvedExecAction) { return {