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

Cannot create property 'domainEmitter' on string #501

Closed
grantcarthew opened this issue Feb 17, 2017 · 1 comment
Closed

Cannot create property 'domainEmitter' on string #501

grantcarthew opened this issue Feb 17, 2017 · 1 comment

Comments

@grantcarthew
Copy link

Code: https://github.com/grantcarthew/node-rethinkdb-job-queue/blob/master/src/queue.js

Line 57 of the above module reads as follows:

  _raiseQueueError (name) {
    const self = this
    return function raiseQueueErrorInternal (errObj) {
      const message = `Event: ${name} error`
      logger(message, self.id, errObj)
      self.emit(enums.status.error, self.id, errObj)
      return Promise.reject(errObj)
    }
  }

It is a helper function that gets used throughout the queue.js file.

I was writing the following test code to test the _raiseQueueError and _raiseQueueErrorInternal functions:

        const testErr = new Error('test')
        const raiseErr = qReady._raiseQueueError('error function name')
        return raiseErr(testErr).catch((err) => {
          t.equal(err.message, 'test', 'raiseQueueErrorInternal returns error object')
        })

This causes the following error in the SSH console:

    Module: queue.spec
    Name: TypeError
    Message: Cannot create property 'domainEmitter' on string 'WebDev:rjqJobQueueTests:rjqJobQueueTestJobs:27160:a06abda5-2b8c-40d1-80fe-984697a089b8'
    TypeError: Cannot create property 'domainEmitter' on string 'WebDev:rjqJobQueueTests:rjqJobQueueTestJobs:27160:a06abda5-2b8c-40d1-80fe-984697a089b8'
    at Queue.emit (events.js:156:24)
    at raiseQueueErrorInternal (/home/grant/node-rethinkdb-job-queue/src/queue.js:67:12)
    at qReady.ready.then.delay.then (/home/grant/node-rethinkdb-job-queue/tests/queue.spec.js:83:16)
    at bound (domain.js:280:14)
    at runBound (domain.js:293:12)
    at tryCatcher (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/util.js:16:23)
    at Promise._settlePromiseFromHandler (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:510:31)
    at Promise._settlePromise (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:567:18)
    at Promise._settlePromise0 (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:612:10)
    at Promise._settlePromises (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:691:18)
    at Promise._fulfill (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:636:18)
    at Promise._resolveCallback (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:431:57)
    at Promise._settlePromiseFromHandler (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:522:17)
    at Promise._settlePromise (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:567:18)
    at Promise._settlePromise0 (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:612:10)
    at Promise._settlePromises (/home/grant/node-rethinkdb-job-queue/node_modules/bluebird/js/release/promise.js:691:18)

So reading on the Domain - Additions to Error Objects page it is obvious that the domainEmitter property is trying to be added to the string value above.

When I wrote the API for the job queue module I decided to go with a standard of always returning the Queue.id as the first argument for all events.

If I change it now it will require a major version bump due to the API change. If that is the case so be it however it seems like node.js should be looking at the type of object the first argument is and preventing this exception.

If I am just doing everything wrong then I'll change it.

Thanks.

@bnoordhuis
Copy link
Member

It's not completely clear to me what your code is doing but I infer it emits an 'error' event with a string instead of an Error object as the argument.

Yes, maybe node should check the type before trying to extend the argument, but the convention is to use Error objects with 'error' events and you shouldn't deviate from that unless you have a really good reason.

Consider that if node itself doesn't get it right - a project that is relentlessly scrutinized by a ton of people - there's no chance whatsoever users of your module will.

I've filed nodejs/node#11438 to fix the bug in core.

addaleax pushed a commit to nodejs/node that referenced this issue Feb 24, 2017
Fix a TypeError when emitting an 'error' argument with a non-object
argument (like a string) when domains are active.

Fixes: nodejs/help#501
PR-URL: #11438
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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

No branches or pull requests

2 participants