Improve node-pty spawn diagnostics (#1587)

* Improve node-pty spawn diagnostics

* Preserve original error stack when adding node-pty recovery hint

Mutate the existing Error's message instead of replacing the object so
the original stack trace and custom fields survive into telemetry/logs.

Co-authored-by: Orca <help@stably.ai>

---------

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
Jinjing
2026-05-08 12:53:03 -07:00
committed by GitHub
co-authored by Orca
parent 952ee7a6ec
commit 1fbe7d9dbf
9 changed files with 418 additions and 30 deletions
+214 -5
View File
@@ -1,5 +1,5 @@
diff --git a/binding.gyp b/binding.gyp
index 5f63978b07ab50aaf7523219a2170ec737a6b5db..b3309a07ef99dea7967d7bdd04b9fc3500acacae 100644
index 5f63978b..b3309a07 100644
--- a/binding.gyp
+++ b/binding.gyp
@@ -5,9 +5,6 @@
@@ -13,7 +13,7 @@ index 5f63978b07ab50aaf7523219a2170ec737a6b5db..b3309a07ef99dea7967d7bdd04b9fc35
'VCCLCompilerTool': {
'AdditionalOptions': [
diff --git a/deps/winpty/src/winpty.gyp b/deps/winpty/src/winpty.gyp
index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..e619813759c6f14694838bdfbd0ea5f8360130ef 100644
index 1ac5758b..e6198137 100644
--- a/deps/winpty/src/winpty.gyp
+++ b/deps/winpty/src/winpty.gyp
@@ -10,7 +10,7 @@
@@ -55,7 +55,7 @@ index 1ac5758bedd8cf54f32280dea4e4aeb5afdee30d..e619813759c6f14694838bdfbd0ea5f8
# Specify this setting here to override a setting from somewhere
# else, such as node's common.gypi.
diff --git a/lib/unixTerminal.js b/lib/unixTerminal.js
index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088bf1865877 100644
index 1ec12f79..cec8b67a 100644
--- a/lib/unixTerminal.js
+++ b/lib/unixTerminal.js
@@ -28,8 +28,12 @@ var native = utils_1.loadNativeModule('pty');
@@ -74,10 +74,219 @@ index 1ec12f796a822c78fba9ad7f6448c3987e325c23..cec8b67aef02f8199e5606a0d257088b
var DEFAULT_NAME = 'xterm';
var DESTROY_SOCKET_TIMEOUT_MS = 200;
diff --git a/src/unix/pty.cc b/src/unix/pty.cc
index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..c17decbb7bc06f10d7b5d26317bd68fdc8c4e339 100644
index 7b4b9e1f..0544b86d 100644
--- a/src/unix/pty.cc
+++ b/src/unix/pty.cc
@@ -778,8 +778,8 @@ done:
@@ -23,7 +23,9 @@
#include <errno.h>
#include <string.h>
#include <stdlib.h>
+#include <stdio.h>
#include <unistd.h>
+#include <string>
#include <thread>
#include <sys/types.h>
@@ -237,13 +239,23 @@ pty_getproc(int, char *);
#endif
#if defined(__APPLE__) || defined(__OpenBSD__)
+struct pty_spawn_error {
+ const char* step;
+ int errnum;
+ std::string detail_name;
+ std::string detail_value;
+};
+
+static std::string
+pty_format_spawn_error(const pty_spawn_error&);
+
static void
pty_posix_spawn(char** argv, char** env,
const struct termios *termp,
const struct winsize *winp,
int* master,
pid_t* pid,
- int* err);
+ pty_spawn_error* err);
#endif
struct DelBuf {
@@ -367,10 +379,11 @@ Napi::Value PtyFork(const Napi::CallbackInfo& info) {
argv[i + 3] = strdup(arg.c_str());
}
- int err = -1;
- pty_posix_spawn(argv, env, term, &winp, &master, &pid, &err);
- if (err != 0) {
- throw Napi::Error::New(napiEnv, "posix_spawnp failed.");
+ pty_spawn_error spawn_error = { NULL, 0, "", "" };
+ pty_posix_spawn(argv, env, term, &winp, &master, &pid, &spawn_error);
+ if (spawn_error.errnum != 0) {
+ std::string spawn_message = pty_format_spawn_error(spawn_error);
+ throw Napi::Error::New(napiEnv, spawn_message);
}
if (pty_nonblock(master) == -1) {
throw Napi::Error::New(napiEnv, "Could not set master fd to nonblocking.");
@@ -684,13 +697,65 @@ pty_getproc(int fd, char *tty) {
#endif
#if defined(__APPLE__)
+static const char*
+pty_errno_name(int errnum) {
+ switch (errnum) {
+ case E2BIG: return "E2BIG";
+ case EACCES: return "EACCES";
+ case EAGAIN: return "EAGAIN";
+ case EMFILE: return "EMFILE";
+ case ENFILE: return "ENFILE";
+ case ENOENT: return "ENOENT";
+ case ENOMEM: return "ENOMEM";
+ default: return "errno";
+ }
+}
+
+static void
+pty_set_spawn_error(pty_spawn_error* err,
+ const char* step,
+ int errnum,
+ const char* detail_name = NULL,
+ const char* detail_value = NULL) {
+ err->step = step;
+ err->errnum = errnum;
+ err->detail_name = detail_name ? detail_name : "";
+ err->detail_value = detail_value ? detail_value : "";
+}
+
+static std::string
+pty_format_spawn_error(const pty_spawn_error& err) {
+ char errno_buf[64];
+ snprintf(errno_buf, sizeof(errno_buf), "%d", err.errnum);
+
+ std::string message = "node-pty: ";
+ message += err.step ? err.step : "unknown";
+ message += " failed: ";
+ message += pty_errno_name(err.errnum);
+ message += " (errno ";
+ message += errno_buf;
+ message += ", ";
+ message += strerror(err.errnum);
+ message += ")";
+
+ if (!err.detail_name.empty()) {
+ message += " - ";
+ message += err.detail_name;
+ message += "='";
+ message += err.detail_value;
+ message += "'";
+ }
+
+ return message;
+}
+
static void
pty_posix_spawn(char** argv, char** env,
const struct termios *termp,
const struct winsize *winp,
int* master,
pid_t* pid,
- int* err) {
+ pty_spawn_error* err) {
int low_fds[3];
size_t count = 0;
@@ -706,11 +771,19 @@ pty_posix_spawn(char** argv, char** env,
POSIX_SPAWN_SETSID;
*master = posix_openpt(O_RDWR);
if (*master == -1) {
+ pty_set_spawn_error(err, "posix_openpt", errno);
+ return;
+ }
+
+ int res = grantpt(*master);
+ if (res == -1) {
+ pty_set_spawn_error(err, "grantpt", errno);
return;
}
- int res = grantpt(*master) || unlockpt(*master);
+ res = unlockpt(*master);
if (res == -1) {
+ pty_set_spawn_error(err, "unlockpt", errno);
return;
}
@@ -719,17 +792,20 @@ pty_posix_spawn(char** argv, char** env,
char slave_pty_name[128];
res = ioctl(*master, TIOCPTYGNAME, slave_pty_name);
if (res == -1) {
+ pty_set_spawn_error(err, "ioctl_TIOCPTYGNAME", errno);
return;
}
slave = open(slave_pty_name, O_RDWR | O_NOCTTY);
if (slave == -1) {
+ pty_set_spawn_error(err, "open_slave", errno, "slave", slave_pty_name);
return;
}
if (termp) {
res = tcsetattr(slave, TCSANOW, termp);
if (res == -1) {
+ pty_set_spawn_error(err, "tcsetattr", errno, "slave", slave_pty_name);
return;
};
}
@@ -737,6 +813,7 @@ pty_posix_spawn(char** argv, char** env,
if (winp) {
res = ioctl(slave, TIOCSWINSZ, winp);
if (res == -1) {
+ pty_set_spawn_error(err, "ioctl_TIOCSWINSZ", errno, "slave", slave_pty_name);
return;
}
}
@@ -751,35 +828,41 @@ pty_posix_spawn(char** argv, char** env,
posix_spawnattr_t attrs;
posix_spawnattr_init(&attrs);
- *err = posix_spawnattr_setflags(&attrs, flags);
- if (*err != 0) {
+ res = posix_spawnattr_setflags(&attrs, flags);
+ if (res != 0) {
+ pty_set_spawn_error(err, "posix_spawnattr_setflags", res);
goto done;
}
sigset_t signal_set;
/* Reset all signal the child to their default behavior */
sigfillset(&signal_set);
- *err = posix_spawnattr_setsigdefault(&attrs, &signal_set);
- if (*err != 0) {
+ res = posix_spawnattr_setsigdefault(&attrs, &signal_set);
+ if (res != 0) {
+ pty_set_spawn_error(err, "posix_spawnattr_setsigdefault", res);
goto done;
}
/* Reset the signal mask for all signals */
sigemptyset(&signal_set);
- *err = posix_spawnattr_setsigmask(&attrs, &signal_set);
- if (*err != 0) {
+ res = posix_spawnattr_setsigmask(&attrs, &signal_set);
+ if (res != 0) {
+ pty_set_spawn_error(err, "posix_spawnattr_setsigmask", res);
goto done;
}
do
- *err = posix_spawn(pid, argv[0], &acts, &attrs, argv, env);
- while (*err == EINTR);
+ res = posix_spawn(pid, argv[0], &acts, &attrs, argv, env);
+ while (res == EINTR);
+ if (res != 0) {
+ pty_set_spawn_error(err, "posix_spawn", res, "helper", argv[0]);
+ }
done:
posix_spawn_file_actions_destroy(&acts);
posix_spawnattr_destroy(&attrs);
+3 -3
View File
@@ -9,7 +9,7 @@ patchedDependencies:
hash: df21db050a4d85552cafbe583ece863d7fa19ed634a1219210a0455b986b6634
path: config/patches/@xterm__addon-ligatures@0.11.0-beta.198.patch
node-pty@1.1.0:
hash: d39825bae630ddf0fc73e83e6001057c70957161772ba078988767d8569cee63
hash: de322b71e83f8e3f26a341144dbd1159a10d79d8a492e06d4bc9d10d7aa84bf4
path: config/patches/node-pty@1.1.0.patch
importers:
@@ -159,7 +159,7 @@ importers:
version: 0.55.1
node-pty:
specifier: ^1.1.0
version: 1.1.0(patch_hash=d39825bae630ddf0fc73e83e6001057c70957161772ba078988767d8569cee63)
version: 1.1.0(patch_hash=de322b71e83f8e3f26a341144dbd1159a10d79d8a492e06d4bc9d10d7aa84bf4)
pdfjs-dist:
specifier: ^5.7.284
version: 5.7.284
@@ -11368,7 +11368,7 @@ snapshots:
undici: 6.25.0
which: 6.0.1
node-pty@1.1.0(patch_hash=d39825bae630ddf0fc73e83e6001057c70957161772ba078988767d8569cee63):
node-pty@1.1.0(patch_hash=de322b71e83f8e3f26a341144dbd1159a10d79d8a492e06d4bc9d10d7aa84bf4):
dependencies:
node-addon-api: 7.1.1
+21
View File
@@ -156,6 +156,27 @@ describe('DaemonClient', () => {
'Something went wrong'
)
})
it('adds recovery hints to node-pty daemon diagnostics', async () => {
await startMockDaemon({
onControlMessage: (msg) => {
const req = msg as { id: string; type: string }
return encodeNdjson({
id: req.id,
ok: false,
error:
"node-pty: posix_spawn failed: ENOENT (errno 2, No such file or directory) - helper='/tmp/deleted/spawn-helper'"
})
}
})
client = new DaemonClient({ socketPath, tokenPath })
await client.ensureConnected()
await expect(client.request('listSessions', undefined)).rejects.toThrow(
"Daemon's node-pty install is gone (worktree deleted?). Restart Orca. node-pty: posix_spawn failed: ENOENT"
)
})
})
describe('events', () => {
+5 -2
View File
@@ -4,6 +4,7 @@ import { randomUUID } from 'crypto'
import { encodeNdjson, createNdjsonParser } from './ndjson'
import { PROTOCOL_VERSION, NOTIFY_PREFIX, DaemonProtocolError } from './types'
import type { HelloMessage, HelloResponse, RpcResponse, DaemonEvent } from './types'
import { addNodePtyRecoveryHint } from './node-pty-error-hints'
const CONNECT_TIMEOUT_MS = 5000
const REQUEST_TIMEOUT_MS = 30000
@@ -220,7 +221,9 @@ export class DaemonClient {
if (response.ok) {
resolve()
} else {
reject(new DaemonProtocolError(response.error ?? 'Hello rejected'))
reject(
new DaemonProtocolError(addNodePtyRecoveryHint(response.error ?? 'Hello rejected'))
)
}
} catch {
reject(new DaemonProtocolError('Invalid hello response'))
@@ -248,7 +251,7 @@ export class DaemonClient {
if (response.ok) {
pending.resolve(response.payload)
} else {
pending.reject(new DaemonProtocolError(response.error))
pending.reject(new DaemonProtocolError(addNodePtyRecoveryHint(response.error)))
}
}
}
@@ -0,0 +1,41 @@
import { describe, expect, it } from 'vitest'
import { addNodePtyRecoveryHint, parseNodePtyDiagnostic } from './node-pty-error-hints'
describe('node-pty diagnostic error hints', () => {
it('parses the native step and errno without dropping the original message', () => {
const message =
"node-pty: posix_spawn failed: ENOENT (errno 2, No such file or directory) - helper='/tmp/deleted/node-pty/spawn-helper'"
expect(parseNodePtyDiagnostic(message)).toEqual({ step: 'posix_spawn', errno: 2 })
expect(addNodePtyRecoveryHint(message)).toBe(
`Daemon's node-pty install is gone (worktree deleted?). Restart Orca. ${message}`
)
})
it('hints when the daemon exhausts file descriptors opening the slave pty', () => {
const message =
"node-pty: open_slave failed: EMFILE (errno 24, Too many open files) - slave='/dev/ttys003'"
expect(addNodePtyRecoveryHint(message)).toBe(
`Daemon hit the file-descriptor limit. Restart the daemon. ${message}`
)
})
it('hints when posix_spawn reports the per-user process limit', () => {
const message =
"node-pty: posix_spawn failed: EAGAIN (errno 35, Resource temporarily unavailable) - helper='/tmp/node-pty/spawn-helper'"
expect(addNodePtyRecoveryHint(message)).toBe(
`Per-user process limit reached. Quit some agents and retry. ${message}`
)
})
it('leaves unrelated and unhinted node-pty diagnostics unchanged', () => {
expect(addNodePtyRecoveryHint('plain failure')).toBe('plain failure')
expect(
addNodePtyRecoveryHint(
"node-pty: tcsetattr failed: EIO (errno 5, Input/output error) - slave='/dev/ttys003'"
)
).toBe("node-pty: tcsetattr failed: EIO (errno 5, Input/output error) - slave='/dev/ttys003'")
})
})
+41
View File
@@ -0,0 +1,41 @@
export type NodePtyDiagnostic = {
step: string
errno: number
}
const NODE_PTY_DIAGNOSTIC_RE = /^node-pty: ([A-Za-z0-9_]+) failed: .*?\(errno (\d+)(?:, [^)]*)?\)/
export function parseNodePtyDiagnostic(message: string): NodePtyDiagnostic | null {
const match = NODE_PTY_DIAGNOSTIC_RE.exec(message)
if (!match) {
return null
}
return {
step: match[1],
errno: Number(match[2])
}
}
export function getNodePtyRecoveryHint(diagnostic: NodePtyDiagnostic): string | null {
if (diagnostic.step === 'posix_spawn' && diagnostic.errno === 2) {
return "Daemon's node-pty install is gone (worktree deleted?). Restart Orca."
}
if (diagnostic.step === 'open_slave' && diagnostic.errno === 24) {
return 'Daemon hit the file-descriptor limit. Restart the daemon.'
}
if (diagnostic.step === 'posix_spawn' && diagnostic.errno === 35) {
return 'Per-user process limit reached. Quit some agents and retry.'
}
return null
}
export function addNodePtyRecoveryHint(message: string): string {
const diagnostic = parseNodePtyDiagnostic(message)
if (!diagnostic) {
return message
}
const hint = getNodePtyRecoveryHint(diagnostic)
return hint ? `${hint} ${message}` : message
}
+55 -1
View File
@@ -1,13 +1,18 @@
import * as pty from 'node-pty'
import { statSync } from 'fs'
import { win32 as pathWin32 } from 'path'
import type { SubprocessHandle } from './session'
import { DaemonProtocolError } from './types'
import {
getAttributionShellLaunchConfig,
getShellReadyLaunchConfig,
resolvePtyShellPath
} from './shell-ready'
import { isValidPtySize, normalizePtySize } from './daemon-pty-size'
import { ensureNodePtySpawnHelperExecutable } from '../providers/local-pty-utils'
import {
ensureNodePtySpawnHelperExecutable,
getNodePtySpawnHelperCandidates
} from '../providers/local-pty-utils'
import { resolveWindowsShellLaunchArgs } from '../providers/windows-shell-args'
import { resolveEffectiveWindowsPowerShell } from '../providers/windows-powershell'
import { isPwshAvailable } from '../pwsh'
@@ -43,6 +48,54 @@ function getDefaultCwd(): string {
return 'C:\\'
}
function formatMissingDaemonPathError(kind: 'helper' | 'cwd', path: string): DaemonProtocolError {
const detailName = kind === 'helper' ? 'helper' : 'cwd'
const step = kind === 'helper' ? 'posix_spawn' : 'daemon_cwd'
return new DaemonProtocolError(
`Daemon's ${kind === 'helper' ? 'node-pty install' : 'working directory'} is gone ` +
`(worktree deleted?). Restart Orca. node-pty: ${step} failed: ENOENT ` +
`(errno 2, No such file or directory) - ${detailName}='${path}'`
)
}
function preflightMacNodePtySpawnEnvironment(): void {
if (process.platform !== 'darwin') {
return
}
let daemonCwd: string
try {
daemonCwd = process.cwd()
if (!statSync(daemonCwd).isDirectory()) {
throw formatMissingDaemonPathError('cwd', daemonCwd)
}
} catch (error) {
if (error instanceof DaemonProtocolError) {
throw error
}
throw formatMissingDaemonPathError('cwd', '<unavailable>')
}
let candidates: string[]
try {
candidates = getNodePtySpawnHelperCandidates()
} catch {
throw formatMissingDaemonPathError('helper', '<unresolved>')
}
for (const candidate of candidates) {
try {
if (statSync(candidate).isFile()) {
return
}
} catch {
// Try the next node-pty native location.
}
}
throw formatMissingDaemonPathError('helper', candidates[0] ?? '<unresolved>')
}
export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandle {
const size = normalizePtySize(opts.cols, opts.rows)
const env: Record<string, string> = {
@@ -121,6 +174,7 @@ export function createPtySubprocess(opts: PtySubprocessOptions): SubprocessHandl
// binary. The main process fixes this via LocalPtyProvider, but the daemon
// runs in a separate forked process with its own code path.
ensureNodePtySpawnHelperExecutable()
preflightMacNodePtySpawnEnvironment()
const proc = pty.spawn(shellPath, shellArgs, {
name: 'xterm-256color',
+16 -2
View File
@@ -17,6 +17,7 @@ import { isPwshAvailable } from '../pwsh'
import { LocalPtyProvider } from '../providers/local-pty-provider'
import type { IPtyProvider, PtySpawnOptions, PtySpawnResult } from '../providers/types'
import { mintPtySessionId, isSafePtySessionId } from '../daemon/pty-session-id'
import { addNodePtyRecoveryHint } from '../daemon/node-pty-error-hints'
import type { ClaudeRuntimeAuthPreparation } from '../claude-accounts/runtime-auth-service'
import { CLAUDE_AUTH_ENV_VARS, hasClaudeAuthEnvConflict } from '../claude-accounts/environment'
import {
@@ -914,6 +915,19 @@ export function registerPtyHandlers(
try {
result = await provider.spawn(spawnOptions)
} catch (err) {
const rawMessage = err instanceof Error ? err.message : String(err)
const hintedMessage = addNodePtyRecoveryHint(rawMessage)
let spawnError: Error
if (hintedMessage === rawMessage && err instanceof Error) {
spawnError = err
} else if (err instanceof Error) {
// Why: rewrite the message in place so the original stack trace,
// name, and any custom fields survive into telemetry and logs.
err.message = hintedMessage
spawnError = err
} else {
spawnError = new Error(hintedMessage)
}
if (effectiveSessionId !== undefined) {
ptySizes.delete(effectiveSessionId)
}
@@ -940,14 +954,14 @@ export function registerPtyHandlers(
? ('claude-code' as const)
: null
if (errorAgentKind) {
const classified = classifyError(err)
const classified = classifyError(spawnError)
track('agent_error', {
agent_kind: errorAgentKind,
error_class: classified.error_class,
...getCohortAtEmit()
})
}
throw err
throw spawnError
}
ptyOwnership.set(result.id, args.connectionId ?? null)
if (preAllocatedHandle) {
+22 -17
View File
@@ -1,9 +1,29 @@
import { basename } from 'path'
import { basename, join } from 'path'
import { existsSync, accessSync, statSync, chmodSync, constants as fsConstants } from 'fs'
import type * as pty from 'node-pty'
let didEnsureSpawnHelperExecutable = false
function toUnpackedAsarPath(candidate: string): string {
return candidate
.replace(/app\.asar([/\\])/, 'app.asar.unpacked$1')
.replace(/node_modules\.asar([/\\])/, 'node_modules.asar.unpacked$1')
}
export function getNodePtySpawnHelperCandidates(): string[] {
const unixTerminalPath = require.resolve('node-pty/lib/unixTerminal.js')
const packageRoot =
basename(unixTerminalPath) === 'unixTerminal.js'
? unixTerminalPath.replace(/[/\\]lib[/\\]unixTerminal\.js$/, '')
: unixTerminalPath
return [
join(packageRoot, 'build', 'Release', 'spawn-helper'),
join(packageRoot, 'build', 'Debug', 'spawn-helper'),
join(packageRoot, 'prebuilds', `${process.platform}-${process.arch}`, 'spawn-helper')
].map(toUnpackedAsarPath)
}
/**
* Validate that a shell binary exists and is executable.
* Returns an error message string if invalid, null if valid.
@@ -37,22 +57,7 @@ export function ensureNodePtySpawnHelperExecutable(): void {
didEnsureSpawnHelperExecutable = true
try {
const unixTerminalPath = require.resolve('node-pty/lib/unixTerminal.js')
const packageRoot =
basename(unixTerminalPath) === 'unixTerminal.js'
? unixTerminalPath.replace(/[/\\]lib[/\\]unixTerminal\.js$/, '')
: unixTerminalPath
const candidates = [
`${packageRoot}/build/Release/spawn-helper`,
`${packageRoot}/build/Debug/spawn-helper`,
`${packageRoot}/prebuilds/${process.platform}-${process.arch}/spawn-helper`
].map((candidate) =>
candidate
.replace('app.asar/', 'app.asar.unpacked/')
.replace('node_modules.asar/', 'node_modules.asar.unpacked/')
)
for (const candidate of candidates) {
for (const candidate of getNodePtySpawnHelperCandidates()) {
if (!existsSync(candidate)) {
continue
}