From 7817d5c0f74b212dd65aa84f4feb8af5ecd674fb Mon Sep 17 00:00:00 2001 From: Josh Date: Sat, 25 Apr 2026 11:03:50 -0400 Subject: [PATCH 1/3] fix(docker): split exec stdout and stderr handling and drop workaround Signed-off-by: Josh --- lib/docker.ts | 126 ++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 97 insertions(+), 29 deletions(-) diff --git a/lib/docker.ts b/lib/docker.ts index 0fbc5314..ad0c3e92 100644 --- a/lib/docker.ts +++ b/lib/docker.ts @@ -251,9 +251,8 @@ export const configureNextcloud = async function(apps = ['viewer'], vendoredBran console.log('│ └─ OK !') // Build app list - const json = await runOcc(['app:list', '--output', 'json'], { container }) - // fix dockerode bug returning invalid leading characters - const applist = JSON.parse(json.substring(json.indexOf('{'))) + const json = await runOcc(['app:list', '--output', 'json'], { container, verbose: true }) + const applist = JSON.parse(json) // Enable apps and give status for (const app of apps) { @@ -263,7 +262,8 @@ export const configureNextcloud = async function(apps = ['viewer'], vendoredBran // built in or mounted already as the app under development await runOcc(['app:enable', '--force', app], { container, verbose: true }) } else { - const { shippedApps } = JSON.parse(await runExec(['cat', 'core/shipped.json'], { container })) + const { stdout: jsonOutput } = await runExec(['cat', 'core/shipped.json'], { container }) + const { shippedApps } = JSON.parse(jsonOutput) if (shippedApps.includes(app)) { const branchOption = ['main', 'master'].includes(vendoredBranch) ? [] : [`--branch=${vendoredBranch}`] // apps that are vendored but still missing (i.e. not build in or mounted already) @@ -408,13 +408,19 @@ interface RunExecOptions { verbose: boolean } +type RunExecResult = { + stdout: string + stderr: string + exitCode: number +} + /** * Execute a command in the container */ -export const runExec = async function( +export async function runExec( command: string | string[], { container, user='www-data', verbose=false, env=[], failOnError=true }: Partial = {}, -) { +): Promise { container = container || getContainer() const exec = await container.exec({ Cmd: typeof command === 'string' ? [command] : command, @@ -424,35 +430,96 @@ export const runExec = async function( Env: env, }) - return new Promise((resolve, reject) => { - const dataStream = new PassThrough() + return new Promise((resolve, reject) => { + const stdoutStream = new PassThrough() + const stderrStream = new PassThrough() - exec.start({}, (err, stream) => { - if (stream) { - // Pass stdout and stderr to dataStream - exec.modem.demuxStream(stream, dataStream, dataStream) - stream.on('end', () => dataStream.end()) - } else { - reject(err) + const stdout: string[] = [] + const stderr: string[] = [] + + let settled = false + let finishedStreams = 0 + + const cleanup = () => { + stdoutStream.removeAllListeners() + stderrStream.removeAllListeners() + } + + const settleResolve = (result: RunExecResult) => { + if (settled) { + return + } + settled = true + cleanup() + resolve(result) + } + + const settleReject = (err: unknown) => { + if (settled) { + return + } + settled = true + cleanup() + reject(err) + } + + const maybeResolve = async () => { + finishedStreams++ + if (finishedStreams === 2) { + const inspectionResult = await exec.inspect() + const result = { + stdout: stdout.join(''), + stderr: stderr.join(''), + exitCode: inspectionResult.ExitCode ?? 0, + } + + if (result.exitCode && failOnError) { + settleReject(new Error('command exited with non-zero exit code', { cause: result })) + return + } + settleResolve(result) + } + } + + stdoutStream.on('data', (chunk) => { + const text = chunk.toString('utf8') + stdout.push(text) + if (verbose && text.trim()) { + console.log(`├─ stdout: ${text.trim().replace(/\n/gi, '\n├─ stdout: ')}`) } }) - const data: string[] = [] - dataStream.on('data', (chunk) => { - data.push(chunk.toString('utf8')) - const printable = data.at(-1)?.trim() - if (verbose && printable) { - console.log(`├─ ${printable.replace(/\n/gi, '\n├─ ')}`) + stderrStream.on('data', (chunk) => { + const text = chunk.toString('utf8') + stderr.push(text) + if (verbose && text.trim()) { + console.log(`├─ stderr: ${text.trim().replace(/\n/gi, '\n├─ stderr: ')}`) } }) - dataStream.on('error', (err) => reject(err)) - dataStream.on('end', async () => { - const result = await exec.inspect() - if (result.ExitCode && failOnError) { - reject(new Error(data.join(''), { cause: result.ExitCode })) + + stdoutStream.on('error', settleReject) + stderrStream.on('error', settleReject) + + stdoutStream.on('end', maybeResolve) + stderrStream.on('end', maybeResolve) + + exec.start({}, (err, stream) => { + if (err) { + settleReject(err) return } - resolve(data.join('')) + if (!stream) { + settleReject(new Error('No exec stream returned')) + return + } + + stream.on('error', settleReject) + stream.on('end', () => { + stdoutStream.end() + stderrStream.end() + }) + + exec.modem.demuxStream(stream, stdoutStream, stderrStream) }) }) } @@ -460,12 +527,13 @@ export const runExec = async function( /** * Execute an occ command in the container */ -export const runOcc = function( +export async function runOcc( command: string | string[], { container, env=[], verbose=false }: Partial> = {}, ) { const cmdArray = typeof command === 'string' ? [command] : command - return runExec(['php', 'occ', ...cmdArray], { container, verbose, env }) + const { stdout } = await runExec(['php', 'occ', ...cmdArray], { container, verbose, env }) + return stdout } /** From 55bc08ca8b704a073c73ebc4e69c7e634f482f3d Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 2 Jul 2026 11:21:35 +0200 Subject: [PATCH 2/3] chore: adjust usage of runExec to new signature Signed-off-by: Ferdinand Thiessen --- lib/docker.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/docker.ts b/lib/docker.ts index ad0c3e92..6143c4cf 100644 --- a/lib/docker.ts +++ b/lib/docker.ts @@ -251,7 +251,7 @@ export const configureNextcloud = async function(apps = ['viewer'], vendoredBran console.log('│ └─ OK !') // Build app list - const json = await runOcc(['app:list', '--output', 'json'], { container, verbose: true }) + const { stdout: json } = await runOcc(['app:list', '--output', 'json'], { container, verbose: true }) const applist = JSON.parse(json) // Enable apps and give status @@ -415,7 +415,7 @@ type RunExecResult = { } /** - * Execute a command in the container + * Execute a command in the container and return stdout/stderr separately. */ export async function runExec( command: string | string[], @@ -529,11 +529,10 @@ export async function runExec( */ export async function runOcc( command: string | string[], - { container, env=[], verbose=false }: Partial> = {}, + { container, env=[], verbose=false, ...rest }: Partial> = {}, ) { const cmdArray = typeof command === 'string' ? [command] : command - const { stdout } = await runExec(['php', 'occ', ...cmdArray], { container, verbose, env }) - return stdout + return runExec(['php', 'occ', ...cmdArray], { ...rest, container, verbose, env }) } /** @@ -550,11 +549,12 @@ export const setSystemConfig = function( /** * Get a Nextcloud system config value from the container. */ -export const getSystemConfig = function( +export async function getSystemConfig( key: string, { container }: { container?: Docker.Container } = {}, ) { - return runOcc(['config:system:get', key], { container }) + const { stdout } = await runOcc(['config:system:get', key], { container }) + return stdout.trim() } From 5da9ed2dd694541695bc33218f1581821d9ea5ef Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 2 Jul 2026 11:26:50 +0200 Subject: [PATCH 3/3] test(docker): add tests for runExec Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Ferdinand Thiessen --- tests/runExec.spec.ts | 98 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 tests/runExec.spec.ts diff --git a/tests/runExec.spec.ts b/tests/runExec.spec.ts new file mode 100644 index 00000000..838c0bcc --- /dev/null +++ b/tests/runExec.spec.ts @@ -0,0 +1,98 @@ +/* + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { Container } from 'dockerode' + +import assert from 'node:assert/strict' +import { after, before, describe, test } from 'node:test' + +import { getContainer, runExec, startNextcloud, stopNextcloud, waitOnNextcloud } from '../lib/docker.ts' + +describe('Docker: runExec', async () => { + let container: Container + + before(async () => { + const ip = await startNextcloud('master', false) + await waitOnNextcloud(ip) + container = getContainer() + }) + + after(async () => await stopNextcloud()) + + await test('captures stdout of a command', async () => { + const { stdout, stderr, exitCode } = await runExec(['echo', 'hello world'], { container }) + assert.equal(stdout.trim(), 'hello world') + assert.equal(stderr, '') + assert.equal(exitCode, 0) + }) + + await test('accepts a plain string command', async () => { + const { stdout, exitCode } = await runExec('hostname', { container }) + assert.ok(stdout.trim().length > 0) + assert.equal(exitCode, 0) + }) + + await test('captures stderr separately from stdout', async () => { + const { stdout, stderr, exitCode } = await runExec( + ['sh', '-c', 'echo out; echo err >&2'], + { container }, + ) + assert.equal(stdout.trim(), 'out') + assert.equal(stderr.trim(), 'err') + assert.equal(exitCode, 0) + }) + + await test('rejects on a non-zero exit code by default', async () => { + await assert.rejects( + runExec(['sh', '-c', 'echo boom >&2; exit 3'], { container }), + (err: Error & { cause?: { stderr: string, exitCode: number } }) => { + assert.match(err.message, /non-zero exit code/) + // the collected result is attached as the error cause + assert.equal(err.cause?.exitCode, 3) + assert.equal(err.cause?.stderr.trim(), 'boom') + return true + }, + ) + }) + + await test('does not reject on a non-zero exit code when failOnError is false', async () => { + const { stdout, stderr, exitCode } = await runExec( + ['sh', '-c', 'echo out; echo err >&2; exit 5'], + { container, failOnError: false }, + ) + assert.equal(exitCode, 5) + assert.equal(stdout.trim(), 'out') + assert.equal(stderr.trim(), 'err') + }) + + await test('runs as the www-data user by default', async () => { + const { stdout } = await runExec('whoami', { container }) + assert.equal(stdout.trim(), 'www-data') + }) + + await test('runs as the requested user', async () => { + const { stdout } = await runExec('whoami', { container, user: 'root' }) + assert.equal(stdout.trim(), 'root') + }) + + await test('forwards environment variables to the command', async () => { + const { stdout } = await runExec( + ['sh', '-c', 'echo "$MY_TEST_VAR"'], + { container, env: ['MY_TEST_VAR=from-env'] }, + ) + assert.equal(stdout.trim(), 'from-env') + }) + + await test('handles large multi-chunk output without truncation', async () => { + // seq produces 100000 lines, forcing the stream to be delivered in + // multiple chunks that runExec must concatenate in order + const { stdout, exitCode } = await runExec(['seq', '1', '100000'], { container }) + const lines = stdout.trim().split('\n') + assert.equal(exitCode, 0) + assert.equal(lines.length, 100000) + assert.equal(lines[0], '1') + assert.equal(lines.at(-1), '100000') + }) +})