stream: enable autoDestroy by default

PR-URL: #30623
Refs: #30621
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Author: ronag

doc/api/stream.md

@@ -277,8 +277,8 @@ added: v0.9.4
 The `'error'` event is emitted if an error occurred while writing or piping
 data. The listener callback is passed a single `Error` argument when called.
 
-The stream is not closed when the `'error'` event is emitted unless the
-[`autoDestroy`][writable-new] option was set to `true` when creating the
+The stream is closed when the `'error'` event is emitted unless the
+[`autoDestroy`][writable-new] option was set to `false` when creating the
 stream.
 
 After `'error'`, no further events other than `'close'` *should* be emitted
@@ -1667,11 +1667,7 @@ const { Writable } = require('stream');
 
 class MyWritable extends Writable {
   constructor({ highWaterMark, ...options }) {
-    super({
-      highWaterMark,
-      autoDestroy: true,
-      emitClose: true
-    });
+    super({ highWaterMark });
     // ...
   }
 }
@@ -1745,6 +1741,9 @@ changes:
     pr-url: https://github.com/nodejs/node/pull/22795
     description: Add `autoDestroy` option to automatically `destroy()` the
                  stream when it emits `'finish'` or errors.
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30623
+    description: Change `autoDestroy` option default to `true`.
 -->
 
 * `options` {Object}
@@ -1776,7 +1775,7 @@ changes:
   * `final` {Function} Implementation for the
     [`stream._final()`][stream-_final] method.
   * `autoDestroy` {boolean} Whether this stream should automatically call
-    `.destroy()` on itself after ending. **Default:** `false`.
+    `.destroy()` on itself after ending. **Default:** `true`.
 
 <!-- eslint-disable no-useless-constructor -->
 ```js
@@ -2021,6 +2020,9 @@ changes:
     pr-url: https://github.com/nodejs/node/pull/22795
     description: Add `autoDestroy` option to automatically `destroy()` the
                  stream when it emits `'end'` or errors.
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/30623
+    description: Change `autoDestroy` option default to `true`.
 -->
 
 * `options` {Object}
@@ -2039,7 +2041,7 @@ changes:
   * `destroy` {Function} Implementation for the
     [`stream._destroy()`][readable-_destroy] method.
   * `autoDestroy` {boolean} Whether this stream should automatically call
-    `.destroy()` on itself after ending. **Default:** `false`.
+    `.destroy()` on itself after ending. **Default:** `true`.
 
 <!-- eslint-disable no-useless-constructor -->
 ```js

lib/_http_incoming.js

@@ -48,7 +48,7 @@ function IncomingMessage(socket) {
     };
   }
 
-  Stream.Readable.call(this, streamOptions);
+  Stream.Readable.call(this, { autoDestroy: false, ...streamOptions });
 
   this._readableState.readingMore = true;
 

lib/_stream_readable.js

@@ -138,7 +138,7 @@ function ReadableState(options, stream, isDuplex) {
   this.emitClose = !options || options.emitClose !== false;
 
   // Should .destroy() be called after 'end' (and potentially 'finish')
-  this.autoDestroy = !!(options && options.autoDestroy);
+  this.autoDestroy = !options || options.autoDestroy !== false;
 
   // Has it been destroyed
   this.destroyed = false;
@@ -201,11 +201,7 @@ Readable.prototype._destroy = function(err, cb) {
 };
 
 Readable.prototype[EE.captureRejectionSymbol] = function(err) {
-  // TODO(mcollina): remove the destroyed if once errorEmitted lands in
-  // Readable.
-  if (!this.destroyed) {
-    this.destroy(err);
-  }
+  this.destroy(err);
 };
 
 // Manually shove something into the read() buffer.

lib/_stream_writable.js

@@ -167,7 +167,7 @@ function WritableState(options, stream, isDuplex) {
   this.emitClose = !options || options.emitClose !== false;
 
   // Should .destroy() be called after 'finish' (and potentially 'end')
-  this.autoDestroy = !!(options && options.autoDestroy);
+  this.autoDestroy = !options || options.autoDestroy !== false;
 
   // Indicates whether the stream has errored. When true all write() calls
   // should return false. This is needed since when autoDestroy

lib/internal/fs/streams.js

@@ -77,6 +77,9 @@ function ReadStream(path, options) {
   if (options.emitClose === undefined) {
     options.emitClose = false;
   }
+  if (options.autoDestroy === undefined) {
+    options.autoDestroy = false;
+  }
 
   this[kFs] = options.fs || fs;
 
@@ -298,6 +301,9 @@ function WriteStream(path, options) {
   if (options.emitClose === undefined) {
     options.emitClose = false;
   }
+  if (options.autoDestroy === undefined) {
+    options.autoDestroy = false;
+  }
 
   this[kFs] = options.fs || fs;
   if (typeof this[kFs].open !== 'function') {

lib/internal/http2/compat.js

@@ -282,7 +282,7 @@ function onStreamTimeout(kind) {
 
 class Http2ServerRequest extends Readable {
   constructor(stream, headers, options, rawHeaders) {
-    super(options);
+    super({ autoDestroy: false, ...options });
     this[kState] = {
       closed: false,
       didRead: false,

lib/internal/http2/core.js

@@ -1782,6 +1782,7 @@ class Http2Stream extends Duplex {
   constructor(session, options) {
     options.allowHalfOpen = true;
     options.decodeStrings = false;
+    options.autoDestroy = false;
     super(options);
     this[async_id_symbol] = -1;
 

lib/net.js

@@ -289,6 +289,7 @@ function Socket(options) {
   options.allowHalfOpen = true;
   // For backwards compat do not emit close on destroy.
   options.emitClose = false;
+  options.autoDestroy = false;
   // Handle strings directly.
   options.decodeStrings = false;
   stream.Duplex.call(this, options);

lib/zlib.js

@@ -259,7 +259,7 @@ function ZlibBase(opts, mode, handle, { flush, finishFlush, fullFlush }) {
     }
   }
 
-  Transform.call(this, opts);
+  Transform.call(this, { autoDestroy: false, ...opts });
   this._hadError = false;
   this.bytesWritten = 0;
   this._handle = handle;

test/parallel/test-stream-pipe-error-handling.js

@@ -62,8 +62,8 @@ const Stream = require('stream').Stream;
   const R = Stream.Readable;
   const W = Stream.Writable;
 
-  const r = new R();
-  const w = new W();
+  const r = new R({ autoDestroy: false });
+  const w = new W({ autoDestroy: false });
   let removed = false;
 
   r._read = common.mustCall(function() {