process,tty: allow reading/writing from duplex sockets

doc/api/errors.md

@@ -1550,18 +1550,6 @@ An attempt was made to operate on an already closed socket.
 
 A call was made and the UDP subsystem was not running.
 
-<a id="ERR_STDERR_CLOSE"></a>
-### ERR_STDERR_CLOSE
-
-An attempt was made to close the `process.stderr` stream. By design, Node.js
-does not allow `stdout` or `stderr` streams to be closed by user code.
-
-<a id="ERR_STDOUT_CLOSE"></a>
-### ERR_STDOUT_CLOSE
-
-An attempt was made to close the `process.stdout` stream. By design, Node.js
-does not allow `stdout` or `stderr` streams to be closed by user code.
-
 <a id="ERR_STREAM_CANNOT_PIPE"></a>
 ### ERR_STREAM_CANNOT_PIPE
 
@@ -1713,12 +1701,6 @@ A `Transform` stream finished with data still in the write buffer.
 
 The initialization of a TTY failed due to a system error.
 
-<a id="ERR_TTY_WRITABLE_NOT_READABLE"></a>
-### ERR_TTY_WRITABLE_NOT_READABLE
-
-This `Error` is thrown when a read is attempted on a TTY `WriteStream`,
-such as `process.stdout.on('data')`.
-
 <a id="ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET"></a>
 ### ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET
 
@@ -1958,6 +1940,37 @@ removed: v10.0.0
 
 The `repl` module was unable to parse data from the REPL history file.
 
+
+<a id="ERR_STDERR_CLOSE"></a>
+### ERR_STDERR_CLOSE
+<!-- YAML
+removed: REPLACEME
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/23053
+    description: Rather than emitting an error, `process.stderr.end()` now
+                 only closes the stream side but not the underlying resource,
+                 making this error obsolete.
+-->
+
+An attempt was made to close the `process.stderr` stream. By design, Node.js
+does not allow `stdout` or `stderr` streams to be closed by user code.
+
+<a id="ERR_STDOUT_CLOSE"></a>
+### ERR_STDOUT_CLOSE
+<!-- YAML
+removed: REPLACEME
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/23053
+    description: Rather than emitting an error, `process.stderr.end()` now
+                 only closes the stream side but not the underlying resource,
+                 making this error obsolete.
+-->
+
+An attempt was made to close the `process.stdout` stream. By design, Node.js
+does not allow `stdout` or `stderr` streams to be closed by user code.
+
 <a id="ERR_STREAM_READ_NOT_IMPLEMENTED"></a>
 ### ERR_STREAM_READ_NOT_IMPLEMENTED
 <!-- YAML
@@ -2065,6 +2078,13 @@ instance.setEncoding('utf8');
 An attempt has been made to create a string larger than the maximum allowed
 size.
 
+<a id="ERR_TTY_WRITABLE_NOT_READABLE"></a>
+### ERR_TTY_WRITABLE_NOT_READABLE
+
+This `Error` is thrown when a read is attempted on a TTY `WriteStream`,
+such as `process.stdout.on('data')`.
+
+
 [`--force-fips`]: cli.html#cli_force_fips
 [`'uncaughtException'`]: process.html#process_event_uncaughtexception
 [`child_process`]: child_process.html

doc/api/process.md

@@ -1860,9 +1860,7 @@ important ways:
 
 1. They are used internally by [`console.log()`][] and [`console.error()`][],
    respectively.
-2. They cannot be closed ([`end()`][] will throw).
-3. They will never emit the [`'finish'`][] event.
-4. Writes may be synchronous depending on what the stream is connected to
+2. Writes may be synchronous depending on what the stream is connected to
    and whether the system is Windows or POSIX:
    - Files: *synchronous* on Windows and POSIX
    - TTYs (Terminals): *asynchronous* on Windows, *synchronous* on POSIX
@@ -2087,7 +2085,6 @@ cases:
   code will be `128` + `6`, or `134`.
 
 [`'exit'`]: #process_event_exit
-[`'finish'`]: stream.html#stream_event_finish
 [`'message'`]: child_process.html#child_process_event_message
 [`'rejectionHandled'`]: #process_event_rejectionhandled
 [`'uncaughtException'`]: #process_event_uncaughtexception
@@ -2101,7 +2098,6 @@ cases:
 [`console.error()`]: console.html#console_console_error_data_args
 [`console.log()`]: console.html#console_console_log_data_args
 [`domain`]: domain.html
-[`end()`]: stream.html#stream_writable_end_chunk_encoding_callback
 [`net.Server`]: net.html#net_class_net_server
 [`net.Socket`]: net.html#net_class_net_socket
 [`NODE_OPTIONS`]: cli.html#cli_node_options_options

lib/internal/errors.js

@@ -810,8 +810,6 @@ E('ERR_SOCKET_BUFFER_SIZE',
 E('ERR_SOCKET_CANNOT_SEND', 'Unable to send data', Error);
 E('ERR_SOCKET_CLOSED', 'Socket is closed', Error);
 E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running', Error);
-E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed', Error);
-E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed', Error);
 E('ERR_STREAM_CANNOT_PIPE', 'Cannot pipe, not readable', Error);
 E('ERR_STREAM_DESTROYED', 'Cannot call %s after a stream was destroyed', Error);
 E('ERR_STREAM_NULL_VALUES', 'May not write null values to stream', TypeError);
@@ -846,9 +844,6 @@ E('ERR_TRANSFORM_ALREADY_TRANSFORMING',
 E('ERR_TRANSFORM_WITH_LENGTH_0',
   'Calling transform done when writableState.length != 0', Error);
 E('ERR_TTY_INIT_FAILED', 'TTY initialization failed', SystemError);
-E('ERR_TTY_WRITABLE_NOT_READABLE',
-  'The Writable side of a TTY is not Readable',
-  Error);
 E('ERR_UNCAUGHT_EXCEPTION_CAPTURE_ALREADY_SET',
   '`process.setupUncaughtExceptionCapture()` was called while a capture ' +
     'callback was already active',

lib/internal/process/stdio.js

@@ -1,15 +1,15 @@
 'use strict';
 
 const {
-  ERR_STDERR_CLOSE,
-  ERR_STDOUT_CLOSE,
   ERR_UNKNOWN_STDIN_TYPE,
   ERR_UNKNOWN_STREAM_TYPE
 } = require('internal/errors').codes;
 
 exports.setupProcessStdio = setupProcessStdio;
 exports.getMainThreadStdio = getMainThreadStdio;
 
+function dummyDestroy(err, cb) { cb(err); }
+
 function getMainThreadStdio() {
   var stdin;
   var stdout;
@@ -19,11 +19,8 @@ function getMainThreadStdio() {
     if (stdout) return stdout;
     stdout = createWritableStdioStream(1);
     stdout.destroySoon = stdout.destroy;
-    stdout._destroy = function(er, cb) {
-      // Avoid errors if we already emitted
-      er = er || new ERR_STDOUT_CLOSE();
-      cb(er);
-    };
+    // Override _destroy so that the fd is never actually closed.
+    stdout._destroy = dummyDestroy;
     if (stdout.isTTY) {
       process.on('SIGWINCH', () => stdout._refreshSize());
     }
@@ -34,11 +31,8 @@ function getMainThreadStdio() {
     if (stderr) return stderr;
     stderr = createWritableStdioStream(2);
     stderr.destroySoon = stderr.destroy;
-    stderr._destroy = function(er, cb) {
-      // Avoid errors if we already emitted
-      er = er || new ERR_STDERR_CLOSE();
-      cb(er);
-    };
+    // Override _destroy so that the fd is never actually closed.
+    stdout._destroy = dummyDestroy;
     if (stderr.isTTY) {
       process.on('SIGWINCH', () => stderr._refreshSize());
     }

lib/tty.js

@@ -26,11 +26,7 @@ const net = require('net');
 const { internalBinding } = require('internal/bootstrap/loaders');
 const { TTY, isTTY } = internalBinding('tty_wrap');
 const errors = require('internal/errors');
-const {
-  ERR_INVALID_FD,
-  ERR_TTY_INIT_FAILED,
-  ERR_TTY_WRITABLE_NOT_READABLE
-} = errors.codes;
+const { ERR_INVALID_FD, ERR_TTY_INIT_FAILED } = errors.codes;
 const { getColorDepth } = require('internal/tty');
 
 // Lazy loaded for startup performance.
@@ -132,13 +128,6 @@ WriteStream.prototype._refreshSize = function() {
   }
 };
 
-// A WriteStream is not readable from, so _read become a no-op.
-// this method could still be called because net.Socket()
-// is a duplex
-WriteStream.prototype._read = function() {
-  this.destroy(new ERR_TTY_WRITABLE_NOT_READABLE());
-};
-
 // Backwards-compat
 WriteStream.prototype.cursorTo = function(x, y) {
   if (readline === undefined) readline = require('readline');

test/parallel/test-stdout-cannot-be-closed-child-process-pipe.js

@@ -24,9 +24,9 @@ function parent() {
   });
 
   child.on('close', function(code, signal) {
-    assert(code);
+    assert.strictEqual(code, 0);
+    assert.strictEqual(err, '');
     assert.strictEqual(out, 'foo');
-    assert(/process\.stdout cannot be closed/.test(err));
     console.log('ok');
   });
 }

test/pseudo-tty/test-stdin-write.js

@@ -0,0 +1,3 @@
+'use strict';
+require('../common');
+process.stdin.end('foobar\n');

test/pseudo-tty/test-stdin-write.out

@@ -0,0 +1 @@
+foobar

test/pseudo-tty/test-stdout-read.in

@@ -0,0 +1 @@
+Hello!

test/pseudo-tty/test-stdout-read.js

@@ -0,0 +1,3 @@
+'use strict';
+const common = require('../common');
+process.stderr.on('data', common.mustCall(console.log));

test/pseudo-tty/test-stdout-read.out

@@ -0,0 +1 @@
+<Buffer 48 65 6c 6c 6f 21 0a>

test/pseudo-tty/testcfg.py

@@ -35,10 +35,11 @@
 
 class TTYTestCase(test.TestCase):
 
-  def __init__(self, path, file, expected, arch, mode, context, config):
+  def __init__(self, path, file, expected, input, arch, mode, context, config):
     super(TTYTestCase, self).__init__(context, path, arch, mode)
     self.file = file
     self.expected = expected
+    self.input = input
     self.config = config
     self.arch = arch
     self.mode = mode
@@ -104,12 +105,16 @@ def GetSource(self):
           + open(self.expected).read())
 
   def RunCommand(self, command, env):
+    input = None
+    if self.input is not None and exists(self.input):
+      input = open(self.input).read()
     full_command = self.context.processor(command)
     output = test.Execute(full_command,
                      self.context,
                      self.context.GetTimeout(self.mode),
                      env,
-                     True)
+                     faketty=True,
+                     input=input)
     return test.TestOutput(self,
                       full_command,
                       output,
@@ -139,11 +144,12 @@ def ListTests(self, current_path, path, arch, mode):
       if self.Contains(path, test):
         file_prefix = join(self.root, reduce(join, test[1:], ""))
         file_path = file_prefix + ".js"
+        input_path = file_prefix + ".in"
         output_path = file_prefix + ".out"
         if not exists(output_path):
           raise Exception("Could not find %s" % output_path)
         result.append(TTYTestCase(test, file_path, output_path,
-                                      arch, mode, self.context, self))
+                                  input_path, arch, mode, self.context, self))
     return result
 
   def GetBuildRequirements(self):

tools/test.py

@@ -705,12 +705,23 @@ def CheckedUnlink(name):
       PrintError("os.unlink() " + str(e))
     break
 
-def Execute(args, context, timeout=None, env={}, faketty=False, disable_core_files=False):
+def Execute(args, context, timeout=None, env={}, faketty=False, disable_core_files=False, input=None):
   if faketty:
     import pty
     (out_master, fd_out) = pty.openpty()
     fd_in = fd_err = fd_out
     pty_out = out_master
+
+    if input is not None:
+      # Before writing input data, disable echo so the input doesn't show
+      # up as part of the output.
+      import termios
+      attr = termios.tcgetattr(fd_in)
+      attr[3] = attr[3] & ~termios.ECHO
+      termios.tcsetattr(fd_in, termios.TCSADRAIN, attr)
+
+      os.write(pty_out, input)
+      os.write(pty_out, '\x04') # End-of-file marker (Ctrl+D)
   else:
     (fd_out, outname) = tempfile.mkstemp()
     (fd_err, errname) = tempfile.mkstemp()