Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
apps/cli: surface child-process crashes instead of silent 2-min timeout
When the WordPress server child process crashes on startup (e.g. due to a
module import mismatch), `waitForReadyMessage` would block for the full
2-minute inactivity timeout and then throw a generic "Timeout waiting for
ready message" error, burying the real cause.

Listen for the child's exit event and for the pre-listener race, and reject
with the child's stderr log path plus its tail so users see the actual
error immediately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
  • Loading branch information
youknowriad and claude committed Apr 20, 2026
commit dfeda45719fd2cb88f6fc29fb523b30be52f6caa
34 changes: 34 additions & 0 deletions apps/cli/lib/tests/wordpress-server-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ describe( 'WordPress Server Manager', () => {
} );

function setupIpcMocks(): void {
// Pretend the child process is alive so the startup-race guard in
// `waitForReadyMessage` doesn't short-circuit with a "child exited" error.
vi.mocked( daemonClient.isProcessRunning ).mockResolvedValue( mockProcessDescription );

// Emit "ready" repeatedly to avoid races where the listener is attached after one-shot emission.
const readyInterval = setInterval( () => {
mockBus.emit( 'process-message', {
Expand Down Expand Up @@ -130,6 +134,36 @@ describe( 'WordPress Server Manager', () => {
'Failed to start process'
);
} );

it( 'should surface an error when the child process exits before becoming ready', async () => {
// Race guard should not fire — pretend the process is still online when checked.
vi.mocked( daemonClient.isProcessRunning ).mockResolvedValue( mockProcessDescription );

// 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(
/WordPress server child process exited before becoming ready/
);
} );

it( 'should surface an error when the child has already exited before listeners attach', async () => {
// Simulate the race where the child process has already exited by the time
// `startProcess` resolves — `isProcessRunning` returns undefined.
vi.mocked( daemonClient.isProcessRunning ).mockResolvedValue( undefined );

await expect( startWordPressServer( mockSiteData, mockLogger ) ).rejects.toThrow(
/WordPress server child process exited before becoming ready/
);
} );
} );

describe( 'stopWordPressServer', () => {
Expand Down
59 changes: 56 additions & 3 deletions apps/cli/lib/wordpress-server-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* Manages WordPress server processes via process manager daemon. Each site runs in a separate
* process that spawns Playground CLI.
*/
import fs from 'fs';
import path from 'path';
import {
PLAYGROUND_CLI_ACTIVITY_CHECK_INTERVAL,
Expand All @@ -20,6 +21,7 @@ import {
type DaemonBusEventMap,
sendMessageToProcess,
} from 'cli/lib/daemon-client';
import { PROCESS_MANAGER_LOGS_DIR } from 'cli/lib/paths';
import { ProcessDescription } from 'cli/lib/types/process-manager-ipc';
import { ServerConfig, ManagerMessagePayload } from 'cli/lib/types/wordpress-server-ipc';
import { Logger } from 'cli/logger';
Expand Down Expand Up @@ -118,7 +120,7 @@ export async function startWordPressServer(
}

const processDesc = await startProcess( processName, wordPressServerChildPath );
await waitForReadyMessage( processDesc.pmId );
await waitForReadyMessage( processName, processDesc.pmId );
await sendMessage(
processDesc.pmId,
processName,
Expand All @@ -132,11 +134,45 @@ export async function startWordPressServer(
return processDesc;
}

async function waitForReadyMessage( pmId: number ): Promise< void > {
const CHILD_STDERR_TAIL_BYTES = 4096;

function readChildStderrTail( processName: string ): string {
const errorLogPath = path.join( PROCESS_MANAGER_LOGS_DIR, `${ processName }-error.log` );
try {
const { size } = fs.statSync( errorLogPath );
if ( size === 0 ) {
return '';
}
const readBytes = Math.min( size, CHILD_STDERR_TAIL_BYTES );
const buffer = Buffer.alloc( readBytes );
const fd = fs.openSync( errorLogPath, 'r' );
try {
fs.readSync( fd, buffer, 0, readBytes, size - readBytes );
} finally {
fs.closeSync( fd );
}
return buffer.toString( 'utf8' ).trimEnd();
} catch {
return '';
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be helpful, or it can be misleading (because the logs include contents from prior invocations)…

The best thing would probably be for the process manager daemon to forward the stderr output for the current invocation. That's not trivial, but we could probably achieve it by having the process manager daemon keep a rolling buffer of each child's stderr stream and then send the buffer's contents along with the exit event in ProcessManagerDaemon::handleProcessExit.

Not something you have to do in this PR, but an agent can probably do an OK job of it.


function buildChildExitedError( processName: string ): Error {
const errorLogPath = path.join( PROCESS_MANAGER_LOGS_DIR, `${ processName }-error.log` );
const tail = readChildStderrTail( processName );
let message = `WordPress server child process exited before becoming ready. See ${ errorLogPath }`;
if ( tail ) {
message += `\n${ tail }`;
}
return new Error( message );
}

async function waitForReadyMessage( processName: string, pmId: number ): Promise< void > {
const bus = await getDaemonBus();

let timeoutId: NodeJS.Timeout;
let readyHandler: ( packet: DaemonBusEventMap[ 'process-message' ] ) => void;
let exitHandler: ( event: DaemonBusEventMap[ 'process-event' ] ) => void;
let abortListener: () => void;

return new Promise< void >( ( resolve, reject ) => {
Expand All @@ -148,16 +184,33 @@ async function waitForReadyMessage( pmId: number ): Promise< void > {
resolve();
}
};
exitHandler = ( event ) => {
if ( event.process.pm_id === pmId && event.event === 'exit' ) {
reject( buildChildExitedError( processName ) );
}
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

abortListener = () => {
reject( new Error( 'Operation aborted' ) );
};
abortController.signal.addEventListener( 'abort', abortListener );

bus.on( 'process-message', readyHandler );
bus.on( 'process-event', exitHandler );

// Guard against the race where the child exits between `startProcess` resolving and
// our listeners being attached: if the process is no longer running now, surface the
// error immediately rather than waiting for the ready-message timeout.
void ( async () => {
const running = await isProcessRunning( processName );
if ( ! running ) {
reject( buildChildExitedError( processName ) );
}
} )();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void ( async () => {
const running = await isProcessRunning( processName );
if ( ! running ) {
reject( buildChildExitedError( processName ) );
}
} )();
isProcessRunning( processName )
.then( ( running ) => {
if ( ! running ) {
reject( buildChildExitedError( processName ) );
}
} )
.catch( reject );

Part syntax nitpicking, part being more explicit about what happens if we catch an exception here.

} ).finally( () => {
clearTimeout( timeoutId );
abortController.signal.removeEventListener( 'abort', abortListener );
bus.off( 'process-message', readyHandler );
bus.off( 'process-event', exitHandler );
} );
}

Expand Down Expand Up @@ -397,7 +450,7 @@ export async function runBlueprint(

const processDesc = await startProcess( processName, wordPressServerChildPath );
try {
await waitForReadyMessage( processDesc.pmId );
await waitForReadyMessage( processName, processDesc.pmId );
await sendMessage(
processDesc.pmId,
processName,
Expand Down
Loading