Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't pipe to a stream without listenerCount #2655

Closed
calvinmetcalf opened this issue Sep 2, 2015 · 8 comments
Closed

can't pipe to a stream without listenerCount #2655

calvinmetcalf opened this issue Sep 2, 2015 · 8 comments
Assignees
Labels
stream Issues and PRs related to the stream subsystem.
Milestone

Comments

@calvinmetcalf
Copy link
Contributor

just noticed this when looking into upgrading readable stream to v3.3.0

@Fishrock123 Fishrock123 added the stream Issues and PRs related to the stream subsystem. label Sep 2, 2015
@Fishrock123 Fishrock123 added this to the 4.0.0 milestone Sep 2, 2015
@Fishrock123 Fishrock123 self-assigned this Sep 2, 2015
@Fishrock123
Copy link
Contributor

Was introduced in 8f58fb9. I'm on it.

@thefourtheye
Copy link
Contributor

@calvinmetcalf Can you please explain with a small program to reproduce the problem?

@Fishrock123
Copy link
Contributor

@calvinmetcalf I understand the gist of the issue, but I can't seem to reproduce it with this?

'use strict';
var common = require('../common');
var stream = require('stream');

const r = new stream.Stream();
r.listenerCount = undefined;

const w = new stream.Stream();
w.listenerCount = undefined;

w.on('pipe', function() {
  r.emit('error', new Error('Readable Error'));
  w.emit('error', new Error('Writable Error'));
});
r.pipe(w);

To my understanding that should cause this error?

EDIT: fixed.

@Fishrock123
Copy link
Contributor

Does this only show up when using custom event-emitter solutions?

@thefourtheye
Copy link
Contributor

@Fishrock123 You had to pipe it after attaching the handler ;-)

'use strict';
var stream = require('stream');

const r = new stream.Stream();
r.listenerCount = undefined;

const w = new stream.Stream();
w.listenerCount = undefined;

w.on('pipe', function() {
  r.emit('error', new Error('Readable Error'));
});

r.pipe(w);

Also, thanks for helping me understand the problem :-)

@rvagg rvagg mentioned this issue Sep 3, 2015
10 tasks
Fishrock123 added a commit to Fishrock123/node that referenced this issue Sep 3, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: nodejs#2655
Refs: 8f58fb9

PR-URL: nodejs#2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@Fishrock123
Copy link
Contributor

Fixed in b513a33

@calvinmetcalf if you could confirm, that'd be 💯

Fishrock123 added a commit that referenced this issue Sep 3, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: #2655
Refs: 8f58fb9

PR-URL: #2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 added a commit to Fishrock123/node that referenced this issue Sep 3, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: nodejs#2655
Refs: 8f58fb9

PR-URL: nodejs#2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@calvinmetcalf
Copy link
Contributor Author

will look in the morning thanks!

@calvinmetcalf
Copy link
Contributor Author

👍

Fishrock123 added a commit that referenced this issue Sep 6, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: #2655
Refs: 8f58fb9

PR-URL: #2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 added a commit that referenced this issue Sep 6, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: #2655
Refs: 8f58fb9

PR-URL: #2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 added a commit that referenced this issue Sep 6, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: #2655
Refs: 8f58fb9

PR-URL: #2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Fishrock123 added a commit that referenced this issue Sep 6, 2015
Now parts of our public and public-ish APIs fall back to old-style
listenerCount() if the emitter does not have a listenerCount function.

Fixes: #2655
Refs: 8f58fb9

PR-URL: #2661
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stream Issues and PRs related to the stream subsystem.
Projects
None yet
Development

No branches or pull requests

3 participants