fix(rpc): surface drained stderr when startup fails under load
- RpcClient: when stdout closes before ready, race child.exited (which settles only after ptree drains the stderr tail for nonzero exits) for 250ms before rejecting, so the earlier-registered exit watcher rejects with the real stderr text instead of an empty 'Stderr:'. - mock-rpc-agent fixture: await the stderr pipe write callback before process.exit so failure text cannot be dropped unflushed. - sdk-tool-activation: restore the default extension-handler budget once the intentionally stalled activation times out, so full-suite machine load cannot also time out the genuine recovery registration. Both tests flaked only under concurrent full-suite chunk load; 10 concurrent stress runs pass post-fix.
This commit is contained in:
@@ -353,6 +353,13 @@ export class RpcClient {
|
||||
// failures are reaped by the readyPromise catch below; established
|
||||
// workers are reaped here so pending requests cannot hang indefinitely.
|
||||
if (!readySettled) {
|
||||
// Stdout can close before the exit reaper finishes draining stderr.
|
||||
// child.exited settles only after the stderr tail is complete (for
|
||||
// nonzero exits), so give it a bounded head start: the exit watcher
|
||||
// below was registered first and rejects with the real stderr text
|
||||
// instead of an empty "Stderr:" (flaked under full-suite load).
|
||||
await Promise.race([child.exited.catch(() => {}), Bun.sleep(250)]);
|
||||
if (readySettled) return;
|
||||
readySettled = true;
|
||||
readyReject(new Error(`Agent output stream ended before ready. Stderr: ${child.peekStderr()}`));
|
||||
return;
|
||||
|
||||
+8
-1
@@ -30,7 +30,14 @@ const legacyState = {
|
||||
};
|
||||
|
||||
if (Bun.env.MOCK_RPC_EXIT_BEFORE_READY) {
|
||||
process.stderr.write(Bun.env.MOCK_RPC_EXIT_STDERR ?? "");
|
||||
const message = Bun.env.MOCK_RPC_EXIT_STDERR ?? "";
|
||||
if (message) {
|
||||
// Await the pipe write: exiting immediately can drop unflushed stderr
|
||||
// bytes, leaving the client's startup error without the failure text.
|
||||
const { promise, resolve } = Promise.withResolvers<void>();
|
||||
process.stderr.write(message, () => resolve());
|
||||
await promise;
|
||||
}
|
||||
process.exit(Number(Bun.env.MOCK_RPC_EXIT_BEFORE_READY));
|
||||
}
|
||||
|
||||
|
||||
@@ -1247,6 +1247,11 @@ describe("createAgentSession defaultInactive tool activation", () => {
|
||||
const errors: string[] = [];
|
||||
const unsubscribe = runner.onError(error => {
|
||||
errors.push(error.error);
|
||||
// The 10ms budget exists only to reap the stalled first handler
|
||||
// quickly; handlers run sequentially and the budget is read per
|
||||
// handler, so restoring it here keeps machine load from timing out
|
||||
// the genuine recovery registration too (flaked in full-suite runs).
|
||||
testSetExtensionHandlerTimeoutMs(EXTENSION_HANDLER_TIMEOUT_MS);
|
||||
});
|
||||
testSetExtensionHandlerTimeoutMs(10);
|
||||
|
||||
@@ -1454,6 +1459,10 @@ describe("createAgentSession defaultInactive tool activation", () => {
|
||||
|
||||
releaseStalledRegistration.resolve();
|
||||
const failure = await detachedFailure.promise;
|
||||
// Restore the default budget before the recovered registration flush:
|
||||
// the 10ms budget was only for reaping the stalled activation, and the
|
||||
// real presentation pass can exceed it under full-suite load.
|
||||
testSetExtensionHandlerTimeoutMs(EXTENSION_HANDLER_TIMEOUT_MS);
|
||||
releaseRecoveredRegistration.resolve();
|
||||
await recoveredActivation.promise;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user