fix(exec): fix error handling in exec run and test actions (#6319)

* fix(exec): fix crash on artifacts copying errors Add missing handler for `CpyError`. Do not crash, throw a `RuntimeError` instead. * fix(exec): print command output after its execution Otherwise, it won't appear if case of artifacts copying error * refactor: rename local variable * 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. * refactor: extract local variable * refactor: rename local variable * refactor: reorder fields declaration To have the same order as for run actions. * fix(exec): align the error handling of exec run and test commands * chore(exec): log action key instead of name in exec run * chore(exec): remove duplicate logs from result object * fix: propagate original error of unknown type * chore: linting
garden-io · Jul 25, 2024 · 6a03430 · 6a03430
1 parent 4157472
commit 6a03430
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 64 deletions.
diff --git 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,24 @@ 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 })
+        }
+
+        throw err
+      }
     })
   )
 }
diff --git a/core/src/plugins/exec/run.ts b/core/src/plugins/exec/run.ts
@@ -30,57 +30,47 @@ export type ExecRunConfig = GardenSdkActionDefinitionConfigType<typeof execRun>
 export type ExecRun = GardenSdkActionDefinitionActionType<typeof execRun>
 
 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,
-    outputs: {
-      log: outputLog,
-    },
+    success: commandResult.success,
+    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 executing ${styles.highlight(action.key())}. 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
 })
diff --git a/core/src/plugins/exec/test.ts b/core/src/plugins/exec/test.ts
@@ -33,41 +33,46 @@ export type ExecTest = GardenSdkActionDefinitionActionType<typeof execTest>
 
 execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => {
   const startedAt = new Date()
-  const { command, env } = action.getSpec()
+  const { command, env, artifacts } = action.getSpec()
 
-  const result = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } })
-
-  const artifacts = action.getSpec("artifacts")
-  await copyArtifacts(log, artifacts, action.getBuildPath(), artifactsPath)
-
-  if (result.outputLog) {
-    const prefix = `Finished executing ${styles.highlight(action.key())}. Here is the full output:`
-    log.info(
-      renderMessageWithDivider({
-        prefix,
-        msg: result.outputLog,
-        isError: !result.success,
-        color: styles.primary,
-      })
-    )
-  }
+  const commandResult = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } })
 
   const detail = {
     moduleName: action.moduleName(),
-    command,
     testName: action.name,
+    command,
     version: action.versionString(),
-    success: result.success,
+    success: commandResult.success,
+    log: commandResult.outputLog,
     startedAt,
-    completedAt: result.completedAt,
-    log: result.outputLog,
+    completedAt: commandResult.completedAt,
   }
 
-  return {
+  const result = {
     state: runResultToActionState(detail),
     detail,
     outputs: {
-      log: result.outputLog,
+      log: commandResult.outputLog,
     },
+  } as const
+
+  if (!commandResult.success) {
+    return result
   }
+
+  if (commandResult.outputLog) {
+    const prefix = `Finished executing ${styles.highlight(action.key())}. 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
 })