Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[node-compat] process.stdin.unref should warn instead of throwing unreachable #21796

Closed
brc-dd opened this issue Jan 5, 2024 · 1 comment · Fixed by #22865
Closed

[node-compat] process.stdin.unref should warn instead of throwing unreachable #21796

brc-dd opened this issue Jan 5, 2024 · 1 comment · Fixed by #22865
Assignees
Labels
node API Related to various "node:*" modules APIs node compat suggestion suggestions for new features (yet to be agreed)

Comments

@brc-dd
Copy link

brc-dd commented Jan 5, 2024

Example code:

import process from 'node:process'
import readline from 'node:readline'

const input = readline.createInterface({
  input: process.stdin
})

process.stdin.unref()

In node, this immediately exits the process. In deno, it throws:

error: Uncaught (in promise) NodeCompatAssertionError: unreachable
    at unreachable (ext:deno_node/_util/asserts.ts:16:9)
    at TTY.unref (ext:deno_node/internal_binding/handle_wrap.ts:41:5)
    at ReadStream.unref (node:net:643:20)

I believe this is currently not supported, but it should just warnNotImplemented instead of throwing?

Corresponding Node docs - https://nodejs.org/api/readline.html#readlinecreateinterfaceoptions (Also here in polyfill comments:)

* When creating a `readline.Interface` using `stdin` as input, the program
* will not terminate until it receives `EOF` (Ctrl+D on
* Linux/macOS, Ctrl+Z followed by Return on
* Windows).
* If you want your application to exit without waiting for user input, you can `unref()` the standard input stream:
*
* ```js
* process.stdin.unref();
* ```

Version:

deno 1.39.1 (release, aarch64-apple-darwin)
v8 12.0.267.8
typescript 5.3.3
@brc-dd brc-dd changed the title [node-compat] process.stdin.unref should throw not implemented instead of unreachable [node-compat] process.stdin.unref should warn instead of throwing unreachable Jan 5, 2024
@kt3k kt3k added suggestion suggestions for new features (yet to be agreed) node compat labels Jan 8, 2024
@kt3k
Copy link
Member

kt3k commented Jan 8, 2024

Sounds reasonable to me. Can you send a PR?

@bartlomieju bartlomieju added the node API Related to various "node:*" modules APIs label Mar 4, 2024
kt3k added a commit that referenced this issue Apr 27, 2024
This PR adds private `[REF]()` and `[UNREF]()` methods to Stdin class,
and call them from Node.js polyfill layer (`TTY` class). This enables
`process.stdin.unref()` and `process.stdin.ref()` for the case when
stdin is terminal.

closes #21796
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node API Related to various "node:*" modules APIs node compat suggestion suggestions for new features (yet to be agreed)
Projects
None yet
3 participants