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.

Sign up for GitHub

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

Jump to bottom

Propagating events from EventEmmiter's that generate secondary events #9

Closed
rbrtribeiro opened this issue Apr 8, 2019 · 2 comments · Fixed by #17
Closed

Propagating events from EventEmmiter's that generate secondary events #9

rbrtribeiro opened this issue Apr 8, 2019 · 2 comments · Fixed by #17
Labels
released

Comments

@rbrtribeiro
Copy link

What is the expected behavior?
When propagating events from emitters such as http.ClientRequest (.e.g response), we should be able to listen for the secondary events (e.g. data event).

What is the actual behavior?
The response object seems to be empty, so, for instance, data event is not emitted.

Possible solution
In the propagate function, the new source.emit function should propagate the return value of emiting the event in the destination emitter. If not, the node http module will dump the object. As stated in the node _http_client.js source code:

If the user did not listen for the 'response' event, then they can't possibly read the data, so we ._dump() it into the void so that the socket doesn't hang there in a paused state.

If this interpretation is correct, PR submitted with a possible solution.

How to reproduce the issue
PR submitted with a new test case that reflects the issue.

Does the bug have a test case?
Submitted with PR
@rbrtribeiro rbrtribeiro mentioned this issue Apr 8, 2019
Closed
@rbrtribeiro
Copy link
Author

Hi @pgte, any feedback on this issue?

Thanks

paulmelnikow added a commit that referenced this issue Apr 12, 2019
@paulmelnikow
fix: Provide correct return value from source.emit()
9020be7 
Thanks to @rbrtribeiro for the fix! This rebases the work from #8 and tests the return values directly (rather than adding a wide-bracket integration test).

Close #8 Close #9
paulmelnikow added a commit that referenced this issue Apr 12, 2019
@paulmelnikow
fix: Provide correct return value from source.emit()
1e3f622 
Thanks to @rbrtribeiro for the fix! This rebases the work from #8 with minor modifications, and tests the return values directly (rather than adding a wide-bracket integration test).

Close #8 Close #9
paulmelnikow added a commit that referenced this issue Apr 12, 2019
@paulmelnikow
fix: Provide correct return value from source.emit()
00a2e63 
Thanks to @rbrtribeiro for the fix! This rebases the work from #8 with minor modifications, and tests the return values directly (rather than adding a wide-bracket integration test).

Close #8 Close #9
@paulmelnikow paulmelnikow mentioned this issue Apr 12, 2019
Merged
paulmelnikow added a commit that referenced this issue Apr 12, 2019
@paulmelnikow
fix: Provide correct return value from source.emit() (#17)
0d1418e 
This rebases the work from #8 with minor modifications, and tests the return values directly (rather than adding a wide-bracket integration test).

Adopts arrow functions in the test file.

This was identified as the root cause of nock/nock#1485.

Thanks to @rbrtribeiro for the fix! 

Close #8 Close #9
@nockbot
Copy link
Collaborator

nockbot commented Apr 12, 2019

🎉 This issue has been resolved in version 2.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: Provide correct return value from source.emit() nock/propagate
2 participants
@rbrtribeiro @nockbot