stream: make virtual methods errors consistent

Use the same error code and always emit the error instead of throwing it. PR-URL: #18813 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michaë Zasso <targos@protonmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
nodejs · Mar 12, 2018 · c979488 · c979488
1 parent 8403f00
commit c979488
Show file tree

Hide file tree

Showing 9 changed files with 58 additions and 61 deletions.
diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
@@ -616,7 +616,7 @@ OutgoingMessage.prototype.removeHeader = function removeHeader(name) {
 
 
 OutgoingMessage.prototype._implicitHeader = function _implicitHeader() {
-  throw new ERR_METHOD_NOT_IMPLEMENTED('_implicitHeader()');
+  this.emit('error', new ERR_METHOD_NOT_IMPLEMENTED('_implicitHeader()'));
 };
 
 Object.defineProperty(OutgoingMessage.prototype, 'headersSent', {

diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js
@@ -35,7 +35,7 @@ const { getHighWaterMark } = require('internal/streams/state');
 const {
   ERR_INVALID_ARG_TYPE,
   ERR_STREAM_PUSH_AFTER_EOF,
-  ERR_STREAM_READ_NOT_IMPLEMENTED,
+  ERR_METHOD_NOT_IMPLEMENTED,
   ERR_STREAM_UNSHIFT_AFTER_END_EVENT
 } = require('internal/errors').codes;
 const ReadableAsyncIterator = require('internal/streams/async_iterator');
@@ -568,7 +568,7 @@ function maybeReadMore_(stream, state) {
 // for virtual (non-string, non-buffer) streams, "length" is somewhat
 // arbitrary, and perhaps not very meaningful.
 Readable.prototype._read = function(n) {
-  this.emit('error', new ERR_STREAM_READ_NOT_IMPLEMENTED());
+  this.emit('error', new ERR_METHOD_NOT_IMPLEMENTED('_read()'));
 };
 
 Readable.prototype.pipe = function(dest, pipeOpts) {

diff --git a/lib/_stream_transform.js b/lib/_stream_transform.js
@@ -162,7 +162,7 @@ Transform.prototype.push = function(chunk, encoding) {
 // an error, then that'll put the hurt on the whole operation.  If you
 // never call cb(), then you'll never get another chunk.
 Transform.prototype._transform = function(chunk, encoding, cb) {
-  throw new ERR_METHOD_NOT_IMPLEMENTED('_transform');
+  cb(new ERR_METHOD_NOT_IMPLEMENTED('_transform()'));
 };
 
 Transform.prototype._write = function(chunk, encoding, cb) {

diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
@@ -553,7 +553,7 @@ function clearBuffer(stream, state) {
 }
 
 Writable.prototype._write = function(chunk, encoding, cb) {
-  cb(new ERR_METHOD_NOT_IMPLEMENTED('_write'));
+  cb(new ERR_METHOD_NOT_IMPLEMENTED('_write()'));
 };
 
 Writable.prototype._writev = null;

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -849,7 +849,6 @@ 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);
 E('ERR_STREAM_PUSH_AFTER_EOF', 'stream.push() after EOF', Error);
-E('ERR_STREAM_READ_NOT_IMPLEMENTED', '_read() is not implemented', Error);
 E('ERR_STREAM_UNSHIFT_AFTER_END_EVENT',
   'stream.unshift() after end event', Error);
 E('ERR_STREAM_WRAP', 'Stream has StringDecoder set or is in objectMode', Error);

diff --git a/test/parallel/test-http-outgoing-proto.js b/test/parallel/test-http-outgoing-proto.js
@@ -7,14 +7,6 @@ const OutgoingMessage = http.OutgoingMessage;
 const ClientRequest = http.ClientRequest;
 const ServerResponse = http.ServerResponse;
 
-common.expectsError(
-  OutgoingMessage.prototype._implicitHeader,
-  {
-    code: 'ERR_METHOD_NOT_IMPLEMENTED',
-    type: Error,
-    message: 'The _implicitHeader() method is not implemented'
-  }
-);
 assert.strictEqual(
   typeof ClientRequest.prototype._implicitHeader, 'function');
 assert.strictEqual(
@@ -67,14 +59,17 @@ common.expectsError(() => {
 });
 
 // write
-common.expectsError(() => {
+{
   const outgoingMessage = new OutgoingMessage();
-  outgoingMessage.write();
-}, {
-  code: 'ERR_METHOD_NOT_IMPLEMENTED',
-  type: Error,
-  message: 'The _implicitHeader() method is not implemented'
-});
+
+  outgoingMessage.on('error', common.expectsError({
+    code: 'ERR_METHOD_NOT_IMPLEMENTED',
+    type: Error,
+    message: 'The _implicitHeader() method is not implemented'
+  }));
+
+  outgoingMessage.write('');
+}
 
 assert(OutgoingMessage.prototype.write.call({ _header: 'test' }));
 

diff --git a/test/parallel/test-stream-readable-with-unimplemented-_read.js b/test/parallel/test-stream-readable-with-unimplemented-_read.js
@@ -1,11 +1,14 @@
 'use strict';
 const common = require('../common');
-const stream = require('stream');
 
-const readable = new stream.Readable();
+const { Readable } = require('stream');
 
-common.expectsError(() => readable.read(), {
-  code: 'ERR_STREAM_READ_NOT_IMPLEMENTED',
+const readable = new Readable();
+
+readable.on('error', common.expectsError({
+  code: 'ERR_METHOD_NOT_IMPLEMENTED',
   type: Error,
-  message: '_read() is not implemented'
-});
+  message: 'The _read() method is not implemented'
+}));
+
+readable.read();
diff --git a/test/parallel/test-stream-transform-constructor-set-methods.js b/test/parallel/test-stream-transform-constructor-set-methods.js
@@ -4,6 +4,16 @@ const common = require('../common');
 const { strictEqual } = require('assert');
 const { Transform } = require('stream');
 
+const t = new Transform();
+
+t.on('error', common.expectsError({
+  type: Error,
+  code: 'ERR_METHOD_NOT_IMPLEMENTED',
+  message: 'The _transform() method is not implemented'
+}));
+
+t.end(Buffer.from('blerg'));
+
 const _transform = common.mustCall((chunk, _, next) => {
   next();
 });
@@ -16,25 +26,15 @@ const _flush = common.mustCall((next) => {
   next();
 });
 
-const t = new Transform({
+const t2 = new Transform({
   transform: _transform,
   flush: _flush,
   final: _final
 });
 
-strictEqual(t._transform, _transform);
-strictEqual(t._flush, _flush);
-strictEqual(t._final, _final);
+strictEqual(t2._transform, _transform);
+strictEqual(t2._flush, _flush);
+strictEqual(t2._final, _final);
 
-t.end(Buffer.from('blerg'));
-t.resume();
-
-const t2 = new Transform({});
-
-common.expectsError(() => {
-  t2.end(Buffer.from('blerg'));
-}, {
-  type: Error,
-  code: 'ERR_METHOD_NOT_IMPLEMENTED',
-  message: 'The _transform method is not implemented'
-});
+t2.end(Buffer.from('blerg'));
+t2.resume();
diff --git a/test/parallel/test-stream-writable-constructor-set-methods.js b/test/parallel/test-stream-writable-constructor-set-methods.js
@@ -4,6 +4,16 @@ const common = require('../common');
 const { strictEqual } = require('assert');
 const { Writable } = require('stream');
 
+const w = new Writable();
+
+w.on('error', common.expectsError({
+  type: Error,
+  code: 'ERR_METHOD_NOT_IMPLEMENTED',
+  message: 'The _write() method is not implemented'
+}));
+
+w.end(Buffer.from('blerg'));
+
 const _write = common.mustCall((chunk, _, next) => {
   next();
 });
@@ -13,25 +23,15 @@ const _writev = common.mustCall((chunks, next) => {
   next();
 });
 
-const w = new Writable({ write: _write, writev: _writev });
+const w2 = new Writable({ write: _write, writev: _writev });
 
-strictEqual(w._write, _write);
-strictEqual(w._writev, _writev);
+strictEqual(w2._write, _write);
+strictEqual(w2._writev, _writev);
 
-w.write(Buffer.from('blerg'));
+w2.write(Buffer.from('blerg'));
 
-w.cork();
-w.write(Buffer.from('blerg'));
-w.write(Buffer.from('blerg'));
-
-w.end();
-
-const w2 = new Writable();
-
-w2.on('error', common.expectsError({
-  type: Error,
-  code: 'ERR_METHOD_NOT_IMPLEMENTED',
-  message: 'The _write method is not implemented'
-}));
+w2.cork();
+w2.write(Buffer.from('blerg'));
+w2.write(Buffer.from('blerg'));
 
-w2.end(Buffer.from('blerg'));
+w2.end();