stream: use more explicit statements

Using objectMode with stream_wrap has not worked properly before and would end in an error. Therefore prohibit the usage of objectMode alltogether. This also improves the handling performance due to the cheaper chunk check and by using explicit statements as they produce better code from the compiler. PR-URL: #13863 Reviewed-By: Matteo Collina <[email protected]>
nodejs · Jun 29, 2017 · 1b54371 · 1b54371
1 parent a1ecdcf
commit 1b54371
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 23 deletions.
diff --git a/doc/api/errors.md b/doc/api/errors.md
@@ -829,10 +829,11 @@ Node.js does not allow `stdout` or `stderr` Streams to be closed by user code.
 Used when an attempt is 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_HAS_STRINGDECODER"></a>
-### ERR_STREAM_HAS_STRINGDECODER
+<a id="ERR_STREAM_WRAP"></a>
+### ERR_STREAM_WRAP
 
-Used to prevent an abort if a string decoder was set on the Socket.
+Used to prevent an abort if a string decoder was set on the Socket or if in
+`objectMode`.
 
 Example
 ```js

diff --git a/lib/_stream_transform.js b/lib/_stream_transform.js
@@ -76,7 +76,7 @@ function afterTransform(er, data) {
 
   var cb = ts.writecb;
 
-  if (!cb) {
+  if (cb === null) {
     return this.emit('error', new errors.Error('ERR_MULTIPLE_CALLBACK'));
   }
 

diff --git a/lib/_stream_wrap.js b/lib/_stream_wrap.js
@@ -4,9 +4,6 @@ const assert = require('assert');
 const util = require('util');
 const Socket = require('net').Socket;
 const JSStream = process.binding('js_stream').JSStream;
-// TODO(bmeurer): Change this back to const once hole checks are
-// properly optimized away early in Ignition+TurboFan.
-var Buffer = require('buffer').Buffer;
 const uv = process.binding('uv');
 const debug = util.debuglog('stream_wrap');
 const errors = require('internal/errors');
@@ -47,12 +44,12 @@ function StreamWrap(stream) {
     self.emit('error', err);
   });
   this.stream.on('data', function ondata(chunk) {
-    if (!(chunk instanceof Buffer)) {
+    if (typeof chunk === 'string' || this._readableState.objectMode === true) {
       // Make sure that no further `data` events will happen
       this.pause();
       this.removeListener('data', ondata);
 
-      self.emit('error', new errors.Error('ERR_STREAM_HAS_STRINGDECODER'));
+      self.emit('error', new errors.Error('ERR_STREAM_WRAP'));
       return;
     }
 

diff --git a/lib/internal/errors.js b/lib/internal/errors.js
@@ -171,7 +171,7 @@ E('ERR_SOCKET_BAD_PORT', 'Port should be > 0 and < 65536');
 E('ERR_SOCKET_DGRAM_NOT_RUNNING', 'Not running');
 E('ERR_STDERR_CLOSE', 'process.stderr cannot be closed');
 E('ERR_STDOUT_CLOSE', 'process.stdout cannot be closed');
-E('ERR_STREAM_HAS_STRINGDECODER', 'Stream has StringDecoder');
+E('ERR_STREAM_WRAP', 'Stream has StringDecoder set or is in objectMode');
 E('ERR_TRANSFORM_ALREADY_TRANSFORMING',
   'Calling transform done when still transforming');
 E('ERR_TRANSFORM_WITH_LENGTH_0',

diff --git a/test/parallel/test-stream-wrap-encoding.js b/test/parallel/test-stream-wrap-encoding.js
@@ -1,23 +1,42 @@
 'use strict';
 const common = require('../common');
-const assert = require('assert');
 
 const StreamWrap = require('_stream_wrap');
 const Duplex = require('stream').Duplex;
 
-const stream = new Duplex({
-  read: function() {
-  },
-  write: function() {
-  }
-});
+{
+  const stream = new Duplex({
+    read() {},
+    write() {}
+  });
 
-stream.setEncoding('ascii');
+  stream.setEncoding('ascii');
 
-const wrap = new StreamWrap(stream);
+  const wrap = new StreamWrap(stream);
 
-wrap.on('error', common.mustCall(function(err) {
-  assert(/StringDecoder/.test(err.message));
-}));
+  wrap.on('error', common.expectsError({
+    type: Error,
+    code: 'ERR_STREAM_WRAP',
+    message: 'Stream has StringDecoder set or is in objectMode'
+  }));
 
-stream.push('ohai');
+  stream.push('ohai');
+}
+
+{
+  const stream = new Duplex({
+    read() {},
+    write() {},
+    objectMode: true
+  });
+
+  const wrap = new StreamWrap(stream);
+
+  wrap.on('error', common.expectsError({
+    type: Error,
+    code: 'ERR_STREAM_WRAP',
+    message: 'Stream has StringDecoder set or is in objectMode'
+  }));
+
+  stream.push(new Error('foo'));
+}