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

Cleanup before callback #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ianmetcalf
Copy link

Being a good citizen and cleaning up after oneself.

It seemed that once wasn't adding much since this already had to wrap the callback.

Not sure how to test this that doesn't rely on checking event emitter internals which seems brittle.

@phated
Copy link
Contributor

phated commented Apr 30, 2018

I think this needs to be landed. I've referenced it in nodejs/node#20437

@mafintosh
Copy link
Owner

We cannot cleanup the error handler as that breaks the contract of the module, as streams might emit an error after end, and there is no way of knowing if that happens. Removing that handler would force users to always add their own custom error handling, to avoid the process crashing, which kinda defeats the purpose of eos.

Do you have any numbers on why it's better to remove the handlers instead of letting gc handle it? The streams are usually gc'ed just after the eos cb is called anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants