diff --git a/apps/cli/lib/tests/wordpress-server-manager.test.ts b/apps/cli/lib/tests/wordpress-server-manager.test.ts index e7bb12d734..bf1ca86acc 100644 --- a/apps/cli/lib/tests/wordpress-server-manager.test.ts +++ b/apps/cli/lib/tests/wordpress-server-manager.test.ts @@ -130,6 +130,66 @@ describe( 'WordPress Server Manager', () => { 'Failed to start process' ); } ); + + it( 'should surface an error when the child process exits before becoming ready', async () => { + // Do not emit `ready`; instead emit an `exit` event to simulate a crash during startup. + setTimeout( () => { + mockBus.emit( 'process-event', { + process: { + name: mockProcessDescription.name, + pm_id: mockProcessDescription.pmId, + }, + event: 'exit', + } ); + }, 10 ); + + await expect( startWordPressServer( mockSiteData, mockLogger ) ).rejects.toThrow( + /exited before becoming ready/ + ); + } ); + + it( 'should include the child stderr tail in the error when the daemon provides it', async () => { + const stderrTail = 'SyntaxError: The requested module did not provide an export named X'; + + setTimeout( () => { + mockBus.emit( 'process-event', { + process: { + name: mockProcessDescription.name, + pm_id: mockProcessDescription.pmId, + }, + event: 'exit', + stderrTail, + } ); + }, 10 ); + + await expect( startWordPressServer( mockSiteData, mockLogger ) ).rejects.toThrow( + new RegExp( stderrTail.replace( /[.*+?^${}()|[\]\\]/g, '\\$&' ) ) + ); + } ); + + it( 'should catch exit events fired before startProcess resolves', async () => { + // Simulate an exit that fires while `startProcess` is still in flight, *before* + // the caller knows the pmId. Listeners must be attached ahead of startProcess for + // this to work. + vi.mocked( daemonClient.startProcess ).mockImplementation( async () => { + setTimeout( () => { + mockBus.emit( 'process-event', { + process: { + name: mockProcessDescription.name, + pm_id: mockProcessDescription.pmId, + }, + event: 'exit', + stderrTail: 'early crash', + } ); + }, 0 ); + await new Promise( ( resolve ) => setTimeout( resolve, 20 ) ); + return mockProcessDescription; + } ); + + await expect( startWordPressServer( mockSiteData, mockLogger ) ).rejects.toThrow( + /early crash/ + ); + } ); } ); describe( 'stopWordPressServer', () => { diff --git a/apps/cli/lib/types/process-manager-ipc.ts b/apps/cli/lib/types/process-manager-ipc.ts index cabd2a439d..de3f0b7f5f 100644 --- a/apps/cli/lib/types/process-manager-ipc.ts +++ b/apps/cli/lib/types/process-manager-ipc.ts @@ -98,6 +98,9 @@ export const processEventSchema = z.object( { z.literal( 'restart' ), z.literal( 'stop' ), ] ), + // Tail of the child's stderr captured during this invocation. Only populated on `exit` + // events; undefined for any other event. + stderrTail: z.string().optional(), } ); const daemonProcessEventSchema = z.object( { diff --git a/apps/cli/lib/wordpress-server-manager.ts b/apps/cli/lib/wordpress-server-manager.ts index 37ce495953..71d8a3544b 100644 --- a/apps/cli/lib/wordpress-server-manager.ts +++ b/apps/cli/lib/wordpress-server-manager.ts @@ -117,48 +117,121 @@ export async function startWordPressServer( serverConfig.enableDebugDisplay = true; } - const processDesc = await startProcess( processName, wordPressServerChildPath ); - await waitForReadyMessage( processDesc.pmId ); - await sendMessage( - processDesc.pmId, - processName, - { - topic: 'start-server', - data: { config: serverConfig }, - }, - { logger } - ); + const readyOrExit = await subscribeForReadyOrExit( processName ); + try { + const processDesc = await startProcess( processName, wordPressServerChildPath ); + await readyOrExit.waitFor( processDesc.pmId ); + await sendMessage( + processDesc.pmId, + processName, + { + topic: 'start-server', + data: { config: serverConfig }, + }, + { logger } + ); - return processDesc; + return processDesc; + } finally { + readyOrExit.dispose(); + } } -async function waitForReadyMessage( pmId: number ): Promise< void > { +function buildChildExitedError( processName: string, stderrTail?: string ): Error { + let message = `WordPress server child process "${ processName }" exited before becoming ready.`; + if ( stderrTail?.trim() ) { + message += `\n${ stderrTail.trimEnd() }`; + } + return new Error( message ); +} + +/** + * Attaches listeners to the daemon bus *before* the child process is started so we cannot miss + * an early `ready` or `exit` event. Events that arrive before the caller knows the pmId are + * buffered (filtered by processName) and replayed once `waitFor(pmId)` is called. + * Must be disposed via `dispose()` when done. + */ +async function subscribeForReadyOrExit( processName: string ): Promise< { + waitFor: ( pmId: number ) => Promise< void >; + dispose: () => void; +} > { const bus = await getDaemonBus(); - let timeoutId: NodeJS.Timeout; - let readyHandler: ( packet: DaemonBusEventMap[ 'process-message' ] ) => void; - let abortListener: () => void; + const pendingReady: Array< DaemonBusEventMap[ 'process-message' ] > = []; + const pendingExits: Array< DaemonBusEventMap[ 'process-event' ] > = []; + let onReady: () => void = () => {}; + let onExit: ( stderrTail?: string ) => void = () => {}; + let waiting = false; + + const messageHandler = ( packet: DaemonBusEventMap[ 'process-message' ] ) => { + if ( packet.process.name !== processName || packet.raw.topic !== 'ready' ) { + return; + } + if ( waiting ) { + onReady(); + } else { + pendingReady.push( packet ); + } + }; + const eventHandler = ( event: DaemonBusEventMap[ 'process-event' ] ) => { + if ( event.process.name !== processName || event.event !== 'exit' ) { + return; + } + if ( waiting ) { + onExit( event.stderrTail ); + } else { + pendingExits.push( event ); + } + }; + + bus.on( 'process-message', messageHandler ); + bus.on( 'process-event', eventHandler ); + + const waitFor = ( pmId: number ): Promise< void > => { + waiting = true; + + let timeoutId: NodeJS.Timeout; + let abortListener: () => void; - return new Promise< void >( ( resolve, reject ) => { - timeoutId = setTimeout( () => { - reject( new Error( 'Timeout waiting for ready message from WordPress server child' ) ); - }, PLAYGROUND_CLI_INACTIVITY_TIMEOUT ); - readyHandler = ( packet ) => { - if ( packet.process.pm_id === pmId && packet.raw.topic === 'ready' ) { - resolve(); + return new Promise< void >( ( resolve, reject ) => { + timeoutId = setTimeout( () => { + reject( new Error( 'Timeout waiting for ready message from WordPress server child' ) ); + }, PLAYGROUND_CLI_INACTIVITY_TIMEOUT ); + abortListener = () => { + reject( new Error( 'Operation aborted' ) ); + }; + + onReady = () => resolve(); + onExit = ( stderrTail ) => reject( buildChildExitedError( processName, stderrTail ) ); + + abortController.signal.addEventListener( 'abort', abortListener ); + + // Replay any events we buffered before pmId was known. + const bufferedExit = pendingExits.find( ( event ) => event.process.pm_id === pmId ); + if ( bufferedExit ) { + onExit( bufferedExit.stderrTail ); + return; } - }; - abortListener = () => { - reject( new Error( 'Operation aborted' ) ); - }; - abortController.signal.addEventListener( 'abort', abortListener ); + const bufferedReady = pendingReady.find( ( packet ) => packet.process.pm_id === pmId ); + if ( bufferedReady ) { + onReady(); + } + } ).finally( () => { + clearTimeout( timeoutId ); + abortController.signal.removeEventListener( 'abort', abortListener ); + // Release per-call handlers; the bus listeners stay until dispose(). + onReady = () => {}; + onExit = () => {}; + waiting = false; + } ); + }; - bus.on( 'process-message', readyHandler ); - } ).finally( () => { - clearTimeout( timeoutId ); - abortController.signal.removeEventListener( 'abort', abortListener ); - bus.off( 'process-message', readyHandler ); - } ); + const dispose = () => { + bus.off( 'process-message', messageHandler ); + bus.off( 'process-event', eventHandler ); + }; + + return { waitFor, dispose }; } const messageActivityTrackers = new Map< @@ -395,21 +468,26 @@ export async function runBlueprint( serverConfig.enableDebugDisplay = true; } - const processDesc = await startProcess( processName, wordPressServerChildPath ); + const readyOrExit = await subscribeForReadyOrExit( processName ); try { - await waitForReadyMessage( processDesc.pmId ); - await sendMessage( - processDesc.pmId, - processName, - { - topic: 'run-blueprint', - data: { config: serverConfig }, - }, - { logger } - ); + const processDesc = await startProcess( processName, wordPressServerChildPath ); + try { + await readyOrExit.waitFor( processDesc.pmId ); + await sendMessage( + processDesc.pmId, + processName, + { + topic: 'run-blueprint', + data: { config: serverConfig }, + }, + { logger } + ); + } finally { + // Always stop the process after blueprint is applied + await stopProcess( processName ); + } } finally { - // Always stop the process after blueprint is applied - await stopProcess( processName ); + readyOrExit.dispose(); } } diff --git a/apps/cli/process-manager-daemon.ts b/apps/cli/process-manager-daemon.ts index 66f4a46b58..d38c39f13f 100644 --- a/apps/cli/process-manager-daemon.ts +++ b/apps/cli/process-manager-daemon.ts @@ -23,6 +23,11 @@ import { ManagerMessage } from 'cli/lib/types/wordpress-server-ipc'; const SOCKET_TIMEOUT_MS = 2_500; const STOP_TIMEOUT_MS = 5_000; +// In-memory tail of stderr kept per child so we can include the current invocation's error +// output in the `exit` event. Bounded to avoid unbounded memory growth on chatty processes. +const STDERR_BUFFER_MAX_LINES = 100; +const STDERR_BUFFER_MAX_BYTES = 16 * 1024; + type ManagedProcessBase = { pmId: number; name: string; @@ -34,6 +39,8 @@ type ManagedProcessBase = { stderrLogPath: string; stdoutStream: WriteStream; stderrStream: WriteStream; + stderrBuffer: string[]; + stderrBufferBytes: number; settled: boolean; }; type ManagedProcessRunning = ManagedProcessBase & { @@ -219,13 +226,17 @@ export class ProcessManagerDaemon { stderrLogPath, stdoutStream, stderrStream, + stderrBuffer: [], + stderrBufferBytes: 0, settled: false, }; this.managedProcesses.set( pmId, managedProcess ); this.pipeOutputWithTimestamp( child.stdout, stdoutStream ); - this.pipeOutputWithTimestamp( child.stderr, stderrStream ); + this.pipeOutputWithTimestamp( child.stderr, stderrStream, ( line ) => { + this.recordStderrLine( managedProcess, line ); + } ); child.on( 'message', ( raw ) => { const event = daemonEventSchema.safeParse( { @@ -242,7 +253,11 @@ export class ProcessManagerDaemon { } ); child.on( 'error', ( error ) => { - writeTimestampedLines( stderrStream, error.stack ?? error.message ); + const errorText = error.stack ?? error.message; + writeTimestampedLines( stderrStream, errorText ); + for ( const line of errorText.split( '\n' ) ) { + this.recordStderrLine( managedProcess, line ); + } void this.handleProcessExit( managedProcess ); } ); @@ -300,11 +315,14 @@ export class ProcessManagerDaemon { managedProcess.stdoutStream.end(); managedProcess.stderrStream.end(); + const stderrTail = managedProcess.stderrBuffer.join( '\n' ); + await this.broadcastEvent( { type: 'process-event', payload: { process: { name: managedProcess.name, pm_id: managedProcess.pmId }, event: 'exit', + ...( stderrTail ? { stderrTail } : {} ), }, } ); } @@ -336,7 +354,8 @@ export class ProcessManagerDaemon { private pipeOutputWithTimestamp( input: NodeJS.ReadableStream | null, - target: WriteStream + target: WriteStream, + onLine?: ( line: string ) => void ): void { if ( ! input ) { return; @@ -349,9 +368,26 @@ export class ProcessManagerDaemon { lineReader.on( 'line', ( line ) => { void target.write( timestampLogLine( line ) ); + onLine?.( line ); } ); } + private recordStderrLine( managedProcess: ManagedProcess, line: string ): void { + managedProcess.stderrBuffer.push( line ); + managedProcess.stderrBufferBytes += Buffer.byteLength( line, 'utf8' ) + 1; // +1 for the joining newline + + while ( + managedProcess.stderrBuffer.length > STDERR_BUFFER_MAX_LINES || + managedProcess.stderrBufferBytes > STDERR_BUFFER_MAX_BYTES + ) { + const dropped = managedProcess.stderrBuffer.shift(); + if ( dropped === undefined ) { + break; + } + managedProcess.stderrBufferBytes -= Buffer.byteLength( dropped, 'utf8' ) + 1; + } + } + private toProcessDescription( managedProcess: ManagedProcess ): ProcessDescription { if ( managedProcess.status === 'stopped' ) { return { diff --git a/apps/cli/tests/daemon.test.ts b/apps/cli/tests/daemon.test.ts index 061ae5369e..b5cc432b98 100644 --- a/apps/cli/tests/daemon.test.ts +++ b/apps/cli/tests/daemon.test.ts @@ -121,6 +121,48 @@ describe( 'ProcessManagerDaemon', () => { ).toContain( 'fixture-stderr' ); } ); + it( 'includes captured stderr in the exit event payload', async () => { + const child = new MockChildProcess(); + spawnMock.mockReturnValue( child ); + const { ProcessManagerDaemon } = await import( '../process-manager-daemon' ); + + const daemon = new ProcessManagerDaemon(); + const daemonInternal = daemon as unknown as { + handleRequest: ( request: unknown ) => Promise< unknown >; + broadcastEvent: ( event: unknown ) => Promise< void >; + }; + const broadcastSpy = vi + .spyOn( daemonInternal, 'broadcastEvent' ) + .mockResolvedValue( undefined ); + + await daemonInternal.handleRequest( { + type: 'start-process', + requestId: '1', + processName: testProcessName, + scriptPath: '/tmp/test-child.js', + env: {}, + args: [], + } ); + + child.stderr.write( 'SyntaxError: boom\n' ); + child.stderr.write( ' at Module._compile\n' ); + // Let readline consume the lines before triggering exit. + await new Promise( ( resolve ) => setTimeout( resolve, 25 ) ); + + child.emit( 'exit', 1 ); + await new Promise( ( resolve ) => setTimeout( resolve, 25 ) ); + + const exitCall = broadcastSpy.mock.calls.find( ( [ event ] ) => { + const payload = ( event as { type: string; payload: { event: string } } ).payload; + return payload.event === 'exit'; + } ); + + expect( exitCall ).toBeDefined(); + const payload = ( exitCall![ 0 ] as { payload: { stderrTail?: string } } ).payload; + expect( payload.stderrTail ).toContain( 'SyntaxError: boom' ); + expect( payload.stderrTail ).toContain( 'at Module._compile' ); + } ); + it( 'reuses duplicate starts, forwards messages, and resolves missing stops', async () => { const child = new MockChildProcess(); spawnMock.mockReturnValue( child );