Skip to content

fix: address output swallowing on Ubuntu 24.04 (#6726) #6800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AryanKansagara wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: address output swallowing on Ubuntu 24.04 (#6726) #6800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions packages/app/src/cli/services/dev/ui.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {render} from '@shopify/cli-kit/node/ui'
import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system'
import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {isUnitTest} from '@shopify/cli-kit/node/context/local'
import {outputDebug} from '@shopify/cli-kit/node/output'

export async function renderDev({
processes,
Expand All @@ -28,9 +29,19 @@ export async function renderDev({
organizationName?: string
configPath?: string
}) {
if (!terminalSupportsPrompting()) {
const supportsPrompting = terminalSupportsPrompting()
const supportsDevSessions = app.developerPlatformClient.supportsDevSessions

outputDebug(`[renderDev] Terminal supports prompting: ${supportsPrompting}`)
outputDebug(`[renderDev] stdin.isTTY: ${process.stdin.isTTY}, stdout.isTTY: ${process.stdout.isTTY}`)
outputDebug(`[renderDev] Developer platform supports dev sessions: ${supportsDevSessions}`)
outputDebug(`[renderDev] Number of processes: ${processes.length}`)

if (!supportsPrompting) {
outputDebug(`[renderDev] Using NON-INTERACTIVE mode (piping to process.stdout/stderr directly)`)
await renderDevNonInteractive({processes, app, abortController, developerPreview, shopFqdn})
} else if (app.developerPlatformClient.supportsDevSessions) {
} else if (supportsDevSessions) {
outputDebug(`[renderDev] Using DevSessionUI (interactive with dev sessions)`)
return render(
<DevSessionUI
processes={processes}
Expand All @@ -50,6 +61,7 @@ export async function renderDev({
},
)
} else {
outputDebug(`[renderDev] Using Dev component (interactive without dev sessions)`)
return render(
<Dev
processes={processes}
Expand Down
120 changes: 120 additions & 0 deletions packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,124 @@ describe('ConcurrentOutput', () => {

expect(renderInstance.waitUntilExit().isFulfilled()).toBe(false)
})

test('handles delayed/buffered writes correctly (simulates Ubuntu 24.04 issue #6726)', async () => {
// This test simulates the scenario where child process output may be
// delayed or buffered differently on certain Linux distributions.
// The issue manifests as hot reload working but terminal output being silent.

const processSync = new Synchronizer()
const receivedOutput: string[] = []

const delayedProcess = {
prefix: 'web',
action: async (stdout: Writable, _stderr: Writable, _signal: AbortSignal) => {
// Simulate delayed writes like a real dev server would produce
stdout.write('Starting server...\n')

// Small delay to simulate async server startup
await new Promise((resolve) => setTimeout(resolve, 10))
stdout.write('Server listening on port 3000\n')

// Another delay to simulate file change detection
await new Promise((resolve) => setTimeout(resolve, 10))
stdout.write('File changed: index.tsx\n')
stdout.write('Rebuilding...\n')

await new Promise((resolve) => setTimeout(resolve, 10))
stdout.write('Build complete\n')

processSync.resolve()
},
}

// When
const renderInstance = render(
<ConcurrentOutput
processes={[delayedProcess]}
abortSignal={new AbortController().signal}
keepRunningAfterProcessesResolve
/>,
)

await processSync.promise
// Give time for all writes to be processed
await new Promise((resolve) => setTimeout(resolve, 50))

// Then - verify all messages were captured
const output = unstyled(renderInstance.lastFrame()!)
expect(output).toContain('Starting server...')
expect(output).toContain('Server listening on port 3000')
expect(output).toContain('File changed: index.tsx')
expect(output).toContain('Rebuilding...')
expect(output).toContain('Build complete')
})

test('handles rapid consecutive writes without dropping output', async () => {
// Tests for potential race conditions in output handling
const processSync = new Synchronizer()
const messageCount = 100

const rapidWriteProcess = {
prefix: 'rapid',
action: async (stdout: Writable, _stderr: Writable, _signal: AbortSignal) => {
// Rapidly write many messages without any delay
for (let i = 0; i < messageCount; i++) {
stdout.write(`message ${i}\n`)
}
processSync.resolve()
},
}

// When
const renderInstance = render(
<ConcurrentOutput
processes={[rapidWriteProcess]}
abortSignal={new AbortController().signal}
keepRunningAfterProcessesResolve
/>,
)

await processSync.promise
await new Promise((resolve) => setTimeout(resolve, 100))

// Then - verify all messages were captured
const output = unstyled(renderInstance.lastFrame()!)
const lines = output.split('\n').filter((line) => line.includes('message'))
expect(lines.length).toBe(messageCount)
})

test('handles stderr output alongside stdout', async () => {
const processSync = new Synchronizer()

const mixedOutputProcess = {
prefix: 'mixed',
action: async (stdout: Writable, stderr: Writable, _signal: AbortSignal) => {
stdout.write('stdout: normal output\n')
stderr.write('stderr: error output\n')
stdout.write('stdout: more output\n')
stderr.write('stderr: warning\n')
processSync.resolve()
},
}

// When
const renderInstance = render(
<ConcurrentOutput
processes={[mixedOutputProcess]}
abortSignal={new AbortController().signal}
keepRunningAfterProcessesResolve
/>,
)

await processSync.promise
await new Promise((resolve) => setTimeout(resolve, 50))

// Then - both stdout and stderr should be captured
const output = unstyled(renderInstance.lastFrame()!)
expect(output).toContain('stdout: normal output')
expect(output).toContain('stderr: error output')
expect(output).toContain('stdout: more output')
expect(output).toContain('stderr: warning')
})
})
54 changes: 35 additions & 19 deletions packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {OutputProcess} from '../../../../public/node/output.js'
import {OutputProcess, outputDebug} from '../../../../public/node/output.js'
import {AbortSignal} from '../../../../public/node/abort.js'
import React, {FunctionComponent, useCallback, useEffect, useMemo, useState} from 'react'
import {Box, Static, Text, TextProps, useApp} from 'ink'
Expand Down Expand Up @@ -132,24 +132,40 @@ const ConcurrentOutput: FunctionComponent<ConcurrentOutputProps> = ({
const writableStream = useCallback(
(process: OutputProcess, prefixes: string[]) => {
return new Writable({
write(chunk, _encoding, next) {
const context = outputContextStore.getStore()
const prefix = context?.outputPrefix ?? process.prefix
const shouldStripAnsi = context?.stripAnsi ?? true
const log = chunk.toString('utf8').replace(/(\n)$/, '')

const index = addPrefix(prefix, prefixes)

const lines = shouldStripAnsi ? stripAnsi(log).split(/\n/) : log.split(/\n/)
setProcessOutput((previousProcessOutput) => [
...previousProcessOutput,
{
color: lineColor(index),
prefix,
lines,
},
])
next()
// Explicitly set options for cross-platform compatibility
// This addresses potential buffering issues on Ubuntu 24.04 (#6726)
decodeStrings: false,
defaultEncoding: 'utf8',
// Use a smaller high water mark to ensure data flows through quickly
highWaterMark: 16,
write(chunk, encoding, next) {
try {
const context = outputContextStore.getStore()
const prefix = context?.outputPrefix ?? process.prefix
const shouldStripAnsi = context?.stripAnsi ?? true
// Handle both Buffer and string chunks
const log = (typeof chunk === 'string' ? chunk : chunk.toString('utf8')).replace(/(\n)$/, '')

outputDebug(`[ConcurrentOutput] Received chunk for prefix "${prefix}": ${log.substring(0, 100)}${log.length > 100 ? '...' : ''}`)

const index = addPrefix(prefix, prefixes)

const lines = shouldStripAnsi ? stripAnsi(log).split(/\n/) : log.split(/\n/)
outputDebug(`[ConcurrentOutput] Processing ${lines.length} line(s) for prefix "${prefix}"`)

setProcessOutput((previousProcessOutput) => [
...previousProcessOutput,
{
color: lineColor(index),
prefix,
lines,
},
])
next()
} catch (error) {
outputDebug(`[ConcurrentOutput] Error processing chunk: ${error}`)
next(error as Error)
}
},
})
},
Expand Down
13 changes: 13 additions & 0 deletions packages/cli-kit/src/public/node/system.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,20 @@ export async function exec(command: string, args: string[], options?: ExecOption
}

if (options?.stderr && options.stderr !== 'inherit') {
outputDebug(`[exec] Piping stderr for command: ${command}`)
commandProcess.stderr?.pipe(options.stderr, {end: false})
// Add debug listener to track data flow
commandProcess.stderr?.on('data', (chunk) => {
outputDebug(`[exec] stderr data received (${chunk.length} bytes) for: ${command}`)
})
}
if (options?.stdout && options.stdout !== 'inherit') {
outputDebug(`[exec] Piping stdout for command: ${command}`)
commandProcess.stdout?.pipe(options.stdout, {end: false})
// Add debug listener to track data flow
commandProcess.stdout?.on('data', (chunk) => {
outputDebug(`[exec] stdout data received (${chunk.length} bytes) for: ${command}`)
})
}
let aborted = false
options?.signal?.addEventListener('abort', () => {
Expand Down Expand Up @@ -137,6 +147,9 @@ function buildExec(command: string, args: string[], options?: ExecOptions): Exec
windowsHide: false,
detached: options?.background,
cleanup: !options?.background,
// Disable buffering for stdout/stderr to ensure real-time streaming
// This helps address output swallowing issues on Ubuntu 24.04 (#6726)
buffer: false,
})
outputDebug(`Running system process${options?.background ? ' in background' : ''}:
· Command: ${command} ${args.join(' ')}
Expand Down