Node: Setting callback for on 'data' event of stderr causes error on v10.0.0

[jesobreira]

• Version: 10.0.0
• Platform: OS X 10.13.2
• Subsystem: Process

I am unable to set a callback on the 'data' event (since process.stderr is a net.Socket stream, it should emit the data event).

The following snippet works on v8.0.0 but throws an error on v10.0.0:

process.stderr.on('data', function(data) {
    // not relevant, error is threw anyway
});

Error threw:

```
Error: read ENOTCONN
at WriteStream.Socket._read (net.js:530:20)
at WriteStream.Readable.read (_stream_readable.js:458:10)
at resume_ (_stream_readable.js:897:12)
at process._tickCallback (internal/process/next_tick.js:174:19)
at Function.Module.runMain (internal/modules/cjs/loader.js:721:11)
at startup (internal/bootstrap/node.js:228:19)
at bootstrapNodeJSCore (internal/bootstrap/node.js:575:3)
Emitted 'error' event at:
at emitErrorNT (internal/streams/destroy.js:92:8)
at emitErrorAndCloseNT (internal/streams/destroy.js:59:3)
at process._tickCallback (internal/process/next_tick.js:174:19)
[... lines matching original stack trace ...]
at bootstrapNodeJSCore (internal/bootstrap/node.js:575:3)
````

[ryzokuken]

Okay, it doesn't throw on Node 8 and throws on 10.

/cc @nodejs/process @nodejs/streams

[ryzokuken]

Even if it's a valid error, we should try throwing something descriptive.

[mcollina]

This is likely a bug on stdout/stderr and net, and not on stream.

[jrasanen]

I git bisected when the error was introduced, if that helps (this commit):

ae2b5bcb7c17a2d2a488f234c736201eed8200db is the first bad commit
commit ae2b5bcb7c17a2d2a488f234c736201eed8200db
Author: cjihrig <[email protected]>
Date:   Mon Apr 2 13:33:48 2018 -0400

    deps: upgrade libuv to 1.20.0

    Notable changes:
    - uv_fs_copyfile() adds support for copy-on-write behavior.
    - uv_relative_path() now uses the long directory name
      for handle->dirw.
    - File operations on files > 2 GB on 32-bit platforms are
      working again.
    - uv_fs_fchmod() on Windows works on files with the
      Archive flag cleared.

    Fixes: https://github.com/nodejs/node/issues/19170
    Fixes: https://github.com/nodejs/node/issues/19455
    Fixes: https://github.com/nodejs/node/issues/12803
    PR-URL: https://github.com/nodejs/node/pull/19758
    Reviewed-By: Ben Noordhuis <[email protected]>
    Reviewed-By: Santiago Gimeno <[email protected]>
    Reviewed-By: James M Snell <[email protected]>

:040000 040000 b5f3cc6178caf6ee1a46ee21fc89c83b2b32a69c c093efd2a818669a8aa2d3641905d6cf2e432c30 M  deps

[cjihrig]

The specific commit in libuv responsible for this is https://github.com/libuv/libuv/commit/8f9ba2a59709a36502fff13ffcb6a36b04bd8506. Specifically, the return -ENOTCONN; when uv_read_start() is called on a stream that is not readable.

This breakage was not intended, but this is arguably a bug in Node (or the application code) as the socket's readable property is false.

[thatshailesh]

Hi All, would like to work on this need some guidance, as per comments i think this hasn't been fixed yet please let me know if this still need to fixed for me to proceed :)

[AyushG3112]

Same issue exists with v10.5.0 and stdout

process.stdout.on('data', function(data) {

});

[AyushG3112]

Resolving issue is more complicated than it seemed at first glance(atleast to me), but the gist of the issue is is the stdout or stderrs fd handle type corresponds to TTY, PIPE or TCP, the stream is created as a WriteStream instead of a DuplexStream.

Relevant code is at: https://github.com/nodejs/node/blob/cdb59854ee6c13712d17af729a42d5221d6af8f2/lib/internal/process/stdio.js#L164-L197

[IamManchanda]

V

On Sun, Jun 24, 2018, 5:08 PM Ayush Gupta [e-mail] wrote:

Found the issue, PR coming in a bit if the fix works.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/nodejs/node/issues/21203#issuecomment-399750045, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AEvYgA23bcuvV_aHIdWKVgiYpePFDMAvks5t_3o_gaJpZM4UfUp1
.

[gireeshpunathil]

I guess this is the relevant code, for a top level Node process (stdout / err is a TTY):

https://github.com/nodejs/node/blob/831821bcf50e722072b0aeac93aacd2c09caae57/lib/tty.js#L82-L86

While I think this is a design decision rather than a bug, the ask in the first comment looks like valid requirement to me, at least allowing stdio to be readable - absorbing stdio logs without allowing the write onto the actual device (don't know if it is possible though) can be useful for custom redirections without code modifications.

[mcollina]

I'm proposing https://github.com/nodejs/node/pull/21654 as a fix for this to be landed in master and Node 10. Then we might want to change this behavior in Node 11 to produce an explicative error.

[addaleax]

I’m reopening this since https://github.com/nodejs/node/pull/22997 should be a good step towards making this work again

[ZissisT]

I am a bit confused on what was the solution for this one (fixed or it still throws error but in another way?).
I notice a very similar behavior in v12.16.1
No error was thrown in v8.11.4 but when I try to upgrade to v12.16.1 I get same error.

In my project these lines of code exist for redirecting stderr to syslogger

        process.addListener('uncaughtException', err => {
            // Log this in syslog
            .
            .
        });

        process.stderr.on('data', function(data) {
            // Redirect data to syslogger
        });

This triggers uncaughtException listener always with

Error: read ENOTCONN
at tryReadStart (net.js:567:20)
at Socket._read (net.js:578:5)
at Socket.Readable.read (_stream_readable.js:478:10)
at Socket.read (net.js:618:39)
at resume_ (_stream_readable.js:965:12)
at processTicksAndRejections (internal/process/task_queues.js:84:21)

Whatever the callback is, exception is thrown.
Is this expected? If yes, what causes this to be thrown now and not before (so I know how to change my code)
Thank you