lib: require a callback for end-of-stream

Make the callback mandatory as mostly done in all other Node.js callback APIs so users explicitly have to decide what to do in error cases. This also documents the options for `Stream.finished()`. When originally implemented it was missed that Stream.finished() has an optional options object. PR-URL: #21058 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Mathias Buus <[email protected]>
nodejs · Aug 21, 2018 · 36468ca · 36468ca
1 parent 9dae0ae
commit 36468ca
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 9 deletions.
diff --git a/doc/api/stream.md b/doc/api/stream.md
@@ -1294,12 +1294,21 @@ implementors should not override this method, but instead implement
 [`readable._destroy()`][readable-_destroy].
 The default implementation of `_destroy()` for `Transform` also emit `'close'`.
 
-### stream.finished(stream, callback)
+### stream.finished(stream[, options], callback)
 <!-- YAML
 added: v10.0.0
 -->
 
 * `stream` {Stream} A readable and/or writable stream.
+* `options` {Object}
+  * `error` {boolean} If set to `false`, then a call to `emit('error', err)` is
+    not treated as finished. **Default**: `true`.
+  * `readable` {boolean} When set to `false`, the callback will be called when
+    the stream ends even though the stream might still be readable.
+    **Default**: `true`.
+  * `writable` {boolean} When set to `false`, the callback will be called when
+    the stream ends even though the stream might still be writable.
+    **Default**: `true`.
 * `callback` {Function} A callback function that takes an optional error
   argument.
 
@@ -2438,7 +2447,7 @@ contain multi-byte characters.
 [zlib]: zlib.html
 [hwm-gotcha]: #stream_highwatermark_discrepancy_after_calling_readable_setencoding
 [pipeline]: #stream_stream_pipeline_streams_callback
-[finished]: #stream_stream_finished_stream_callback
+[finished]: #stream_stream_finished_stream_options_callback
 [stream-_flush]: #stream_transform_flush_callback
 [stream-_read]: #stream_readable_read_size_1
 [stream-_transform]: #stream_transform_transform_chunk_encoding_callback

diff --git a/lib/internal/streams/end-of-stream.js b/lib/internal/streams/end-of-stream.js
@@ -4,11 +4,10 @@
 'use strict';
 
 const {
+  ERR_INVALID_ARG_TYPE,
   ERR_STREAM_PREMATURE_CLOSE
 } = require('internal/errors').codes;
 
-function noop() {}
-
 function isRequest(stream) {
   return stream.setHeader && typeof stream.abort === 'function';
 }
@@ -23,10 +22,19 @@ function once(callback) {
 }
 
 function eos(stream, opts, callback) {
-  if (typeof opts === 'function') return eos(stream, null, opts);
-  if (!opts) opts = {};
+  if (arguments.length === 2) {
+    callback = opts;
+    opts = {};
+  } else if (opts == null) {
+    opts = {};
+  } else if (typeof opts !== 'object') {
+    throw new ERR_INVALID_ARG_TYPE('opts', 'object', opts);
+  }
+  if (typeof callback !== 'function') {
+    throw new ERR_INVALID_ARG_TYPE('callback', 'function', callback);
+  }
 
-  callback = once(callback || noop);
+  callback = once(callback);
 
   const ws = stream._writableState;
   const rs = stream._readableState;

diff --git a/test/parallel/test-stream-finished.js b/test/parallel/test-stream-finished.js
@@ -91,8 +91,8 @@ const { promisify } = require('util');
 {
   const rs = fs.createReadStream('file-does-not-exist');
 
-  finished(rs, common.mustCall((err) => {
-    assert.strictEqual(err.code, 'ENOENT');
+  finished(rs, common.expectsError({
+    code: 'ENOENT'
   }));
 }
 
@@ -119,3 +119,37 @@ const { promisify } = require('util');
   rs.push(null);
   rs.resume();
 }
+
+// Test faulty input values and options.
+{
+  const rs = new Readable({
+    read() {}
+  });
+
+  assert.throws(
+    () => finished(rs, 'foo'),
+    {
+      name: /ERR_INVALID_ARG_TYPE/,
+      message: /callback/
+    }
+  );
+  assert.throws(
+    () => finished(rs, 'foo', () => {}),
+    {
+      name: /ERR_INVALID_ARG_TYPE/,
+      message: /opts/
+    }
+  );
+  assert.throws(
+    () => finished(rs, {}, 'foo'),
+    {
+      name: /ERR_INVALID_ARG_TYPE/,
+      message: /callback/
+    }
+  );
+
+  finished(rs, null, common.mustCall());
+
+  rs.push(null);
+  rs.resume();
+}