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

EventEmitter gets TypeError if EventEmitter.prototype.addListener is called directly. #523

Closed
yosuke-furukawa opened this issue Jan 20, 2015 · 4 comments
Closed

EventEmitter gets TypeError if EventEmitter.prototype.addListener is called directly. #523

yosuke-furukawa opened this issue Jan 20, 2015 · 4 comments

Comments

@yosuke-furukawa
Copy link
Member

I got following TypeError. the detail is here (sequelize/sequelize#2966).

TypeError: undefined is not a function
     at Promise.addListener (events.js:154:18)
     at Promise.on (/Users/yosuke/go/src/github.com/yosuke-furukawa/Hexi/node_modules/sequelize/lib/promise.js:118:31)

I think Sequelize module is wrong, EventEmitter.prototype.addListener should not be called directly. However, I don't think this error message is easy to understand. And io.js seems to break node.js compatible on events.

I have 4 options, I would like to discuss about the behavior.

  1. If this.getMaxListeners is undefined, events module prints helpful message like "EventEmitter.prototype.addListener should not called directly."
  2. If this.getMaxListeners is undefined, return the default EventEmitter.defaultMaxListeners.
    1. + 2.
  4. Keep this behavior.
@Fishrock123
Copy link
Contributor

Seems like it's mostly Sequalize abusing EventEmitter.

@bnoordhuis bnoordhuis mentioned this issue Jan 20, 2015
Merged
@bnoordhuis
Copy link
Member

Agreed, but we can't welch on our promise of joyent/node compatibility with little things like this. First one to review #527 gets a cookie.

bnoordhuis added a commit to bnoordhuis/io.js that referenced this issue Jan 20, 2015
@bnoordhuis
lib: fix TypeError with EventEmitter#on() abuse
ee9cd00 
Commit 2931348 added EventEmitter#getMaxListeners() but introduced a
regression when people abuse EventEmitter.prototype.on.call() to call
EventEmitter#on() on a non-EE object.  Add a workaround for that.

Fixes: nodejs#523
PR-URL: nodejs#527
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@yosuke-furukawa
Copy link
Member Author

LGTM.

@bnoordhuis
Copy link
Member

Cheers, fixed in ee9cd00. Thanks for the bug report, Yosuke.

@yosuke-furukawa yosuke-furukawa mentioned this issue Jan 21, 2015
Closed
@mscdex mscdex mentioned this issue Feb 21, 2015
Closed
@bodyslam bodyslam mentioned this issue Feb 15, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@bnoordhuis @yosuke-furukawa @Fishrock123