fix(ext/node): enable test-stdio-closed tests#32237
Merged
fraidev merged 11 commits intodenoland:mainfrom Feb 20, 2026
Merged
Conversation
c2ae4a0 to
9d3b7c8
Compare
This reverts commit 215dae1.
When a non-deno process (e.g. Python) is spawned with Deno's execPath in its args, set DENO_NODE_COMPAT=1 in the child's environment. This env var is then read by cli/args/flags.rs to enable allow_all, bare_node_builtins, and detect_cjs for bare-run scripts, allowing the child deno process to use require() and other Node.js compat features even when invoked directly without the usual node compat flags. This fixes test-stdio-closed.js on Windows, where the test spawns Python which closes fd 0/1/2 and then calls deno directly (bypassing child_process arg translation), causing require() to be undefined and exit code 1.
This reverts commit 9d3b7c8.
284ea4b to
032511e
Compare
bartlomieju
reviewed
Feb 20, 2026
Comment on lines
+43
to
+51
| writer.writeSync( | ||
| ObjectPrototypeIsPrototypeOf(Uint8ArrayPrototype, buf) | ||
| ? buf | ||
| : Buffer.from(buf, enc), | ||
| ); | ||
| } catch (e) { | ||
| // BrokenPipe errors should be emitted as async 'error' events, | ||
| // not thrown synchronously, matching Node.js behavior. | ||
| if ( |
Contributor
Author
There was a problem hiding this comment.
Yeah, this is a workaround to replicate the BrokenPipe error, but if we had FD access in net modules like new Socket({ fd: 1 }) we would not need it.
Without the workaround, that exception escapes the stream machinery entirely and becomes an uncaught error, the 'error' event on process.stdout never fires.
I'll add TODO to fix it when/if we have raw fd access. Or should we skip/ignore this test?
bartlomieju
reviewed
Feb 20, 2026
Comment on lines
+262
to
+281
| // Provide a minimal _handle so code that checks process.stdin._handle | ||
| // (e.g. test-stdout-close-unref.js) works. We intentionally omit | ||
| // readStart/readStop/reading so the onpause handler takes the simple | ||
| // io.stdin UNREF path - adding those methods causes _readableState.reading | ||
| // to be reset, which triggers duplicate _read() calls and orphaned | ||
| // reffed promises that prevent process exit. | ||
| stdin._handle = { | ||
| close(cb) { | ||
| io.stdin?.close(); | ||
| if (typeof cb === "function") cb(); | ||
| }, | ||
| ref() { | ||
| io.stdin?.[io.REF](); | ||
| }, | ||
| unref() { | ||
| io.stdin?.[io.UNREF](); | ||
| }, | ||
| getAsyncId() { | ||
| return -1; | ||
| }, |
Member
There was a problem hiding this comment.
I guess we could provide a proper implementation once we have equivalent of libuv stdio handle?
Contributor
Author
There was a problem hiding this comment.
Yeah, that would help here. Should I also add a todo?
bartlomieju
approved these changes
Feb 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
child_processshell command translation to handle shell redirections (1>&-,< file), piped commands (cmd1 | cmd2), and shell variable prefixes ("$NODE",${NODE}). Previously shell operators were passed to the CLI arg translator, causing parse failures or incorrect output.transformDenoShellCommandcall fromnormalizeSpawnArguments— it was already called inbuildCommand, and the double transformation corrupted POSIX'\''quoting.wrap_evalparameter toop_node_translate_cli_argsso shell command contexts skip eval code wrapping (which introduces metacharacters unsafe for shell embedding).BrokenPipetoEPIPE(was incorrectly mapped toEBADF) instream_wrap, and catchBrokenPipeinprocess.stdout/process.stderrwriteSyncto emit it as an async error event._handlewithref()/unref()/close()to PIPE stdin. Intentionally omitsreadStart/readStop/reading— adding those resets_readableState.reading, which triggers duplicate_read()calls with orphaned reffed promises that prevent process exit.io.stdinin_read()and unref on pause when noreadStopexists, matching Node.js event loop ref-counting behavior.transformDenoShellCommand: POSIX uses single quotes with'\''escaping, Windows uses double quotes with\\doubling before".cmd.exedetection (/d /s /cpattern) inbuildCommandfor shell command transformation.--unstable-*flags infork()between CLI translator and bootstrap args.Enables 3 new node_compat tests:
test-stdio-closed.jstest-stdout-close-catch.jstest-stdout-close-unref.jsNeeds: