Skip to content

Commit 2efe4ab

Browse files
committed
stream: start old-mode read in a next tick
Calling `.read()` in the same tick with `.on('data', ...)` may cause users missing `error` events, because no `error` listeners were set yet. fix #7618
1 parent a7dd0e5 commit 2efe4ab

File tree

4 files changed

+57
-4
lines changed

4 files changed

+57
-4
lines changed

lib/_stream_readable.js

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ function ReadableState(options, stream) {
8787
// if true, a maybeReadMore has been scheduled
8888
this.readingMore = false;
8989

90+
// if true, stream is in old mode
91+
this.oldMode = false;
92+
9093
this.decoder = null;
9194
this.encoding = null;
9295
if (options.encoding) {
@@ -766,8 +769,15 @@ function emitDataEvents(stream, startPaused) {
766769
this.emit('resume');
767770
};
768771

769-
// now make it start, just in case it hadn't already.
770-
stream.emit('readable');
772+
// Start reading in next tick to allow caller to set event listeners on
773+
// the stream object (like 'error')
774+
process.nextTick(function() {
775+
// now make it start, just in case it hadn't already.
776+
stream.emit('readable');
777+
});
778+
779+
// Let others know about our mode
780+
state.oldMode = true;
771781
}
772782

773783
// wrap an old-style stream as the async data source.

lib/http.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2078,7 +2078,7 @@ function connectionListener(socket) {
20782078
// if the user never called req.read(), and didn't pipe() or
20792079
// .resume() or .on('data'), then we call req._dump() so that the
20802080
// bytes will be pulled off the wire.
2081-
if (!req._consuming)
2081+
if (!req._consuming && !req._readableState.oldMode)
20822082
req._dump();
20832083

20842084
res.detachSocket(socket);
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// Copyright Joyent, Inc. and other Node contributors.
2+
//
3+
// Permission is hereby granted, free of charge, to any person obtaining a
4+
// copy of this software and associated documentation files (the
5+
// "Software"), to deal in the Software without restriction, including
6+
// without limitation the rights to use, copy, modify, merge, publish,
7+
// distribute, sublicense, and/or sell copies of the Software, and to permit
8+
// persons to whom the Software is furnished to do so, subject to the
9+
// following conditions:
10+
//
11+
// The above copyright notice and this permission notice shall be included
12+
// in all copies or substantial portions of the Software.
13+
//
14+
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
15+
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
16+
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
17+
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
18+
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
19+
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
20+
// USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
22+
var common = require('../common');
23+
var assert = require('assert');
24+
25+
var Readable = require('stream').Readable;
26+
27+
var r = new Readable();
28+
var errors = 0;
29+
30+
// Setting `data` listener should not trigger `_read()` calls before we will
31+
// set the `error` listener below
32+
r.on('data', function() {
33+
});
34+
35+
r.on('error', function() {
36+
errors++;
37+
});
38+
39+
process.on('exit', function() {
40+
assert.equal(errors, 1);
41+
});

test/simple/test-stream2-compatibility.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,6 @@ TestReader.prototype._read = function(n) {
4747
};
4848

4949
var reader = new TestReader();
50-
assert.equal(ondataCalled, 1);
50+
process.nextTick(function() {
51+
assert.equal(ondataCalled, 1);
52+
});

0 commit comments

Comments
 (0)