Skip to content

Commit

Permalink
fix(pulumi): fix process spawn machinery in Pulumi plugin (#6377)
Browse files Browse the repository at this point in the history
* fix(pulumi): attempt to fix unhandled I/O error

* fix(tools): restore log streaming in ext tool invocation

* chore(tools): remove dead code

* chore: cleanup

---------

Co-authored-by: Vladimir Vagaytsev <[email protected]>
  • Loading branch information
thsig and vvagaytsev authored Aug 15, 2024
1 parent 883df51 commit 76bdbec
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 52 deletions.
23 changes: 11 additions & 12 deletions core/src/util/ext-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ import AsyncLock from "async-lock"
import type { PluginContext } from "../plugin-context.js"
import { LogLevel } from "../logger/logger.js"
import { uuidv4 } from "./random.js"
import { streamLogs, waitForProcess } from "./process.js"
import { pipeline } from "node:stream/promises"
import type { MaybeSecret } from "./secrets.js"
import split2 from "split2"

const { pathExists, createWriteStream, ensureDir, chmod, remove, move, createReadStream } = fsExtra

Expand Down Expand Up @@ -122,7 +122,7 @@ export abstract class CliWrapper {
}

/**
* Helper for using spawn with live log streaming. Waits for the command to finish before returning.
* Helper for using exec with live log streaming. Waits for the command to finish before returning.
*
* If an error occurs and no output has been written to stderr, we use stdout for the error message instead.
*
Expand All @@ -134,20 +134,19 @@ export abstract class CliWrapper {
env,
log,
ctx,
errorPrefix,
}: SpawnParams & { errorPrefix: string; ctx: PluginContext; statusLine?: Log }) {
const proc = await this.spawn({ args, cwd, env, log })
const logEventContext = {
origin: this.name,
level: "verbose" as const,
}

streamLogs({
proc,
name: this.name,
ctx,
const logStream = split2()
logStream.on("data", (line: Buffer) => {
const logLine = line.toString()
ctx.events.emit("log", { timestamp: new Date().toISOString(), msg: logLine, ...logEventContext })
})

await waitForProcess({
proc,
errorPrefix,
})
return await this.spawnAndWait({ args, cwd, env, log, stdout: logStream, stderr: logStream })
}

/**
Expand Down
40 changes: 0 additions & 40 deletions core/src/util/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import type { ChildProcess } from "child_process"
import split2 from "split2"
import { RuntimeError } from "../exceptions.js"
import type { PluginContext } from "../plugin-context.js"
import type { StringLogLevel } from "../logger/logger.js"

Expand Down Expand Up @@ -57,45 +56,6 @@ export function waitForProcessExit({ proc }: { proc: ChildProcess }): Promise<vo
})
}

export function waitForProcess({ proc, errorPrefix }: { proc: ChildProcess; errorPrefix: string }): Promise<void> {
const logStream = split2()

let stdout = ""
let stderr = ""

if (proc.stderr) {
proc.stderr.pipe(logStream)
proc.stderr.on("data", (data) => {
stderr += data
})
}

if (proc.stdout) {
proc.stdout.pipe(logStream)
proc.stdout.on("data", (data) => {
stdout += data
})
}

return new Promise<void>((resolve, reject) => {
proc.on("error", reject)
proc.on("close", (code) => {
if (code === 0) {
resolve()
} else {
// Some commands (e.g. the pulumi CLI) don't log anything to stderr when an error occurs. To handle that,
// we use `stdout` for the error output instead (in case information relevant to the user is included there).
const errOutput = stderr.length > 0 ? stderr : stdout
reject(
new RuntimeError({
message: `${errorPrefix}:\n${errOutput}\n\nExit code: ${code}`,
})
)
}
})
})
}

export class LogLineTimeoutError extends Error {
private stdout: string
private stderr: string
Expand Down

0 comments on commit 76bdbec

Please sign in to comment.