neumino / thinky

JavaScript ORM for RethinkDB
http://justonepixel.com/thinky/
Other
1.12k stars 128 forks source link

Broken in node v.0.11.x #33

Closed chrisvariety closed 10 years ago

chrisvariety commented 10 years ago

Hi there,

First off: :heart: thinky.

I'm running into an error trying to use a secondary index in a getAll query, as seen below:

var thinky = require('thinky');
thinky.init({db: 'ti_dev'});

var Company = thinky.createModel('Company', {name: String, subdomain: String});

Company.getAll('isis', { index: 'subdomain' }, function(err, result) {
  console.log("callback", err, result);
});
TypeError: Cannot call method 'bind' of undefined
    at new Document (/Users/chrismcc/workspace/ti-node/node_modules/thinky/lib/document.js:14:49)
    at new model (/Users/chrismcc/workspace/ti-node/node_modules/thinky/lib/model.js:34:28)
    at /Users/chrismcc/workspace/ti-node/node_modules/thinky/lib/model.js:434:39
    at /Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/cursor.js:80:16
    at /Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/cursor.js:53:17
    at Cursor._handleRow (/Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/cursor.js:152:12)
    at Object.pb.ResponseTypeSwitch.SUCCESS_SEQUENCE (/Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/cursor.js:178:28)
    at Object.module.exports.ResponseTypeSwitch (/Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/protobuf.js:61:29)
    at Cursor._promptNext (/Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/cursor.js:170:12)
    at Cursor.<anonymous> (/Users/chrismcc/workspace/ti-node/node_modules/rethinkdb/cursor.js:220:17)

Debugging tells me that the key it is blowing up on is domain from eventEmitter.prototype.

Any thoughts?

I'm running node v0.11.9 which may be the reason?

A dump of my database, if it's useful (super tiny, two test records): http://ec1.s3.amazonaws.com/rethinkdb_dump_2013-12-28T07:07:15.tar.gz

Thanks so much!

neumino commented 10 years ago

Hum, that's really strange.

The API for eventEmitter is frozen

Do you get the same error with get?

chrisvariety commented 10 years ago

@neumino Yeah, same error with get: https://gist.github.com/speedmanly/59b765e658d47d99b538

I agree it is strange.

> var eventEmitter = require('events').EventEmitter;
undefined
> Object.keys(eventEmitter.prototype);
[ 'domain',
  '_events',
  '_maxListeners',
  'setMaxListeners',
  'emit',
  'addListener',
  'on',
  'once',
  'removeListener',
  'removeAllListeners',
  'listeners' ]

Seems like it was added in this commit, along with two others: https://github.com/joyent/node/commit/7ce5a310612bfcfc153836e718fe3c6309369fb4

chrisvariety commented 10 years ago

@neumino This patch fixes it FYI: https://github.com/speedmanly/thinky/commit/23d348f184a303ff995553627fe538ae2c49c9bb

(I'm using node v0.11.x for koa: http://koajs.com/)

neumino commented 10 years ago

That prevents the bug, but does a document still have the eventEmitter properties? (emit, on, off, addListener etc.)

chrisvariety commented 10 years ago

@neumino Yep!

All tests pass, e.g. https://github.com/neumino/thinky/blob/master/test/document.js#L219

$ mocha

  ․․․․․․․․․․․.get() is deprecated. Access using array indexes instead.
.set() is deprecated. Set using array indexes instead.
․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․
  ․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․․

  161 passing (3s)
neumino commented 10 years ago

Thanks @speedmanly!

The offending keys were domain, _events, _maxListeners. They are by default undefined.

I pushed a similar fix as what you did, with a slightly stronger check (typeof ... === 'function'). Release in 0.2.20 : )

chrisvariety commented 10 years ago

Awesome! Thanks so much & happy new year!

On Tuesday, December 31, 2013, Michel wrote:

Thanks @speedmanly https://github.com/speedmanly!

The offending key were domain, _events, _maxListeners. They are by default undefined.

I pushed a similar fix as what you did, with a slightly stronger check (typeof ... === 'function'). Release in 0.2.20 : )

— Reply to this email directly or view it on GitHubhttps://github.com/neumino/thinky/issues/33#issuecomment-31394776 .