Skip to content

Commit

Permalink
fix(exec): show error on failed commands
Browse files Browse the repository at this point in the history
* fix: show error on failed exec commands

* chore: fix error message templating
  • Loading branch information
Orzelius authored Jul 6, 2023
1 parent c16bff6 commit 8d39e22
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 27 deletions.
8 changes: 5 additions & 3 deletions core/src/plugins/exec/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export const execBuildHandler = execBuild.addHandler("build", async ({ action, l
const command = action.getSpec("command")

const { chalk } = sdk.util
let success = true

if (command?.length) {
const result = await execRunCommand({ command, action, ctx, log })
Expand All @@ -59,7 +60,8 @@ export const execBuildHandler = execBuild.addHandler("build", async ({ action, l
}

output.detail.fresh = true
output.detail.buildLog = result.all || result.stdout + result.stderr
output.detail.buildLog = result.outputLog
success = result.success
}

if (output.detail?.buildLog) {
Expand All @@ -70,11 +72,11 @@ export const execBuildHandler = execBuild.addHandler("build", async ({ action, l
renderMessageWithDivider({
prefix,
msg: output.detail?.buildLog,
isError: false,
isError: !success,
color: chalk.gray,
})
)
}

return output
return { ...output, state: success ? "ready" : "failed" }
})
17 changes: 15 additions & 2 deletions core/src/plugins/exec/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ export async function execRunCommand({
}

const outputStream = split2()
outputStream.on("error", () => {})
outputStream.on("error", (line: Buffer) => {
log.error(line.toString())
ctx.events.emit("log", { timestamp: new Date().toISOString(), msg: line.toString(), ...logEventContext })
})
outputStream.on("data", (line: Buffer) => {
log.verbose(line.toString())
ctx.events.emit("log", { timestamp: new Date().toISOString(), msg: line.toString(), ...logEventContext })
Expand All @@ -72,14 +75,24 @@ export async function execRunCommand({

log.debug(`Running command: ${cmd}`)

return exec(cmd, args, {
const result = await exec(cmd, args, {
...opts,
shell,
cwd: action.getBuildPath(),
env: envVars,
stdout: outputStream,
stderr: outputStream,
})

// Comes from error object
const shortMessage = (result as any).shortMessage || ""

return {
...result,
outputLog: ((result.stdout || "") + "\n" + (result.stderr || "") + "\n" + shortMessage).trim(),
completedAt: new Date(),
success: result.exitCode === 0,
}
}

export async function copyArtifacts(
Expand Down
17 changes: 10 additions & 7 deletions core/src/plugins/exec/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ import {
execStaticOutputsSchema,
} from "./config"
import { deployStateToActionState, DeployStatus } from "../../plugin/handlers/Deploy/get-status"
import { Resolved } from "../../actions/types"
import { ActionState, Resolved } from "../../actions/types"
import { convertCommandSpec, execRunCommand, getDefaultEnvVars } from "./common"
import { isRunning, killRecursive } from "../../process"
import { sdk } from "../../plugin/sdk"
import { execProvider } from "./exec"
import { getTracePropagationEnvVars } from "../../util/tracing/propagation"
import { DeployState } from "../../types/service"

const persistentLocalProcRetryIntervalMs = 2500

Expand Down Expand Up @@ -193,22 +194,24 @@ execDeploy.addHandler("deploy", async (params) => {
opts: { reject: true },
})

const outputLog = (result.stdout + result.stderr).trim()
if (outputLog) {
if (result.outputLog) {
const prefix = `Finished deploying service ${chalk.white(action.name)}. Here is the output:`
log.verbose(
renderMessageWithDivider({
prefix,
msg: outputLog,
isError: false,
msg: result.outputLog,
isError: !result.success,
color: chalk.gray,
})
)
}

const deployState: DeployState = result.success ? "ready" : "unhealthy"
const actionState: ActionState = result.success ? "ready" : "failed"

return {
state: "ready",
detail: { state: "ready", detail: { deployCommandOutput: result.all } },
state: actionState,
detail: { state: deployState, detail: { deployCommandOutput: result.all } },
outputs: {},
}
}
Expand Down
8 changes: 4 additions & 4 deletions core/src/plugins/exec/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ execRun.addHandler("run", async ({ artifactsPath, log, action, ctx }) => {
if (command && command.length) {
const commandResult = await execRunCommand({ command, action, ctx, log, env, opts: { reject: false } })

completedAt = new Date()
outputLog = commandResult.all?.trim() || ""
success = commandResult.exitCode === 0
completedAt = commandResult.completedAt
outputLog = commandResult.outputLog
success = commandResult.success
} else {
completedAt = startedAt
outputLog = ""
Expand All @@ -54,7 +54,7 @@ execRun.addHandler("run", async ({ artifactsPath, log, action, ctx }) => {
renderMessageWithDivider({
prefix,
msg: outputLog,
isError: false,
isError: !success,
color: chalk.gray,
})
)
Expand Down
15 changes: 7 additions & 8 deletions core/src/plugins/exec/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,13 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => {

const { chalk } = sdk.util

const outputLog = result.all?.trim() || ""
if (outputLog) {
if (result.outputLog) {
const prefix = `Finished executing ${chalk.white(action.key())}. Here is the full output:`
log.verbose(
renderMessageWithDivider({
prefix,
msg: outputLog,
isError: false,
msg: result.outputLog,
isError: !result.success,
color: chalk.gray,
})
)
Expand All @@ -60,17 +59,17 @@ execTest.addHandler("run", async ({ log, action, artifactsPath, ctx }) => {
command,
testName: action.name,
version: action.versionString(),
success: result.exitCode === 0,
success: result.success,
startedAt,
completedAt: new Date(),
log: outputLog,
completedAt: result.completedAt,
log: result.outputLog,
}

return {
state: runResultToActionState(detail),
detail,
outputs: {
log: outputLog,
log: result.outputLog,
},
}
})
5 changes: 2 additions & 3 deletions core/src/tasks/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,8 @@ export abstract class BaseActionTask<T extends Action, O extends ValidResultType
if (disabled && action.kind !== "Build") {
// TODO-0.13.1: Need to handle conditional references, over in dependenciesFromAction()
throw new GraphError({
message: `${this.action.longDescription()} depends on one or more runtime outputs from action ${
action.key
}, which is disabled. Please either remove the reference or enable the action.`,
message: `${this.action.longDescription()} depends on one or more runtime outputs from action
${action.key}, which is disabled. Please either remove the reference or enable the action.`,
detail: { dependant: this.action.key(), dependency: action.key() },
})
}
Expand Down

0 comments on commit 8d39e22

Please sign in to comment.