From 1b54371c50cd6a67f22f6ec0db53bac3917387e5 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Thu, 22 Jun 2017 02:38:41 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/13863 Reviewed-By: Matteo Collina --- doc/api/errors.md | 7 ++-- lib/_stream_transform.js | 2 +- lib/_stream_wrap.js | 7 +--- lib/internal/errors.js | 2 +- test/parallel/test-stream-wrap-encoding.js | 45 +++++++++++++++------- 5 files changed, 40 insertions(+), 23 deletions(-) diff --git a/doc/api/errors.md b/doc/api/errors.md index 20f877c92dc966..97c3db76908c46 100644 --- 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. - -### ERR_STREAM_HAS_STRINGDECODER + +### 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 index 34a83f76322807..ba80f5c09cf94f 100644 --- 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 index 4b92fb64536b11..00e6de2fd257a7 100644 --- 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 index 109892f831247d..cf5bdcf4d2e2d6 100644 --- 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 index d23faaab4a65f3..ce6f95fa27d68f 100644 --- 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')); +}