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

MaxListenersExceededWarning fix #1494

Closed
wants to merge 2 commits into from
Closed

MaxListenersExceededWarning fix #1494

wants to merge 2 commits into from

Conversation

devMYC
Copy link

@devMYC devMYC commented Mar 25, 2018

No description provided.

@coveralls
Copy link

coveralls commented Mar 26, 2018

Coverage Status

Coverage increased (+0.01%) to 85.88% when pulling 55b08d3 on devMYC:1.0-fix into f5809e4 on ethereum:1.0.

@frozeman
Copy link
Contributor

I'm declined to merge this, as it would silently fail and not tell the developers that the subscription was not created.
Also this assumes that all providers which support .on() are event emitters with a maxListener.. function.
We should think about a way to return proper warnings in such cases so that the dapp dev can take care of cleaning up subscriptions.

@devMYC
Copy link
Author

devMYC commented Apr 12, 2018

@frozeman yeah totally agree, definitely need a better solution to solve this problem. I'm currently just using this hack to ignore the warning when running tests.

@frozeman
Copy link
Contributor

Then lets think about a real solution, Ill close this.

@frozeman frozeman closed this Apr 12, 2018
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