Stack trace api differences when in strict mode causing TypeErrors

Question

Stack trace api differences when in strict mode causing TypeErrors

jasisk opened this issue 9 years ago · 25 comments

Can be realized by setting strict mode in test/fixtures/my-lib.

To maintain restrictions imposed on strict mode functions, frames that have a strict mode function and all frames below (its caller etc.) are not allow to access their receiver and function objects.

Failing test:

  1) deprecate(message) when message omitted should generate message for function call on named function:
     TypeError: Cannot read property 'constructor' of undefined
      at CallSiteGetTypeName (native)
      at defaultMessage (/Users/jsisk/src/node/nodejs-depd/index.js:283:27)
      at Function.log (/Users/jsisk/src/node/nodejs-depd/index.js:234:9)
      at deprecate (/Users/jsisk/src/node/nodejs-depd/index.js:125:9)
      at automsgnamed (/Users/jsisk/src/node/nodejs-depd/test/fixtures/my-lib.js:59:3)
      at callold (/Users/jsisk/src/node/nodejs-depd/test/test.js:116:9)
      at captureStderr (/Users/jsisk/src/node/nodejs-depd/test/test.js:664:5)
      at Context.<anonymous> (/Users/jsisk/src/node/nodejs-depd/test/test.js:118:20)
      at callFn (/Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runnable.js:250:21)
      at Test.Runnable.run (/Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runnable.js:243:7)
      at Runner.runTest (/Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runner.js:373:10)
      at /Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runner.js:451:12
      at next (/Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runner.js:298:14)
      at /Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runner.js:308:7
      at next (/Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runner.js:246:23)
      at Immediate._onImmediate (/Users/jsisk/src/node/nodejs-depd/node_modules/mocha/lib/runner.js:275:5)
      at processImmediate [as _immediateCallback] (timers.js:358:17)

Answer 1 · 2015-04-06T19:48:20.000Z

I'll take a look into this. Any suggestions are welcome :)

Answer 2 · 2015-04-06T19:50:38.000Z

Actually, I assume this just make a TypeError from our getThis() call?

Answer 3 · 2015-04-06T19:52:00.000Z

Not super familiar with the call stack api or your normalization stuff, figured I'd capture the issue before diving in to your caller resolution stuff.

getThis fails but so does getTypeName because TypeError: Cannot read property 'constructor' of undefined.

Answer 4 · 2015-04-06T19:53:58.000Z

That's fine. Really, I just need a definition of what "causing problems" from your title means. Are there Errors being thrown? If so what are they? If not, what's happening?

Answer 5 · 2015-04-06T19:54:45.000Z

Hah. Sorry. Those are the only two TypeErrors I've come across so far.

Answer 6 · 2015-04-06T19:55:43.000Z

Cool. This is officially a bug at this point :)

Answer 7 · 2015-04-06T19:57:31.000Z

I'll write a test case for realizing a getThis() failure. The getTypeName() can be seen in the existing test by adding 'use strict'; to the my-lib fixture.

Answer 8 · 2015-04-06T20:22:54.000Z

'use strict';
var dep = require('depd')('dep');
var inc = dep.function(function deprecated() {});
inc.apply(inc);

/Users/jsisk/tmp/depd/node_modules/depd/index.js:292
    typeName = callSite.getThis().name || typeName
                                 ^
TypeError: Cannot read property 'name' of undefined
    at defaultMessage (/Users/jsisk/tmp/depd/node_modules/depd/index.js:292:34)
    at Function.log (/Users/jsisk/tmp/depd/node_modules/depd/index.js:235:9)
    at Function.eval (eval at wrapfunction (/Users/jsisk/tmp/depd/node_modules/depd/index.js:414:5), <anonymous>:3:5)
    at Object.<anonymous> (/Users/jsisk/tmp/depd/index.js:4:5)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
    at startup (node.js:129:16)

Answer 9 · 2015-04-06T20:23:31.000Z

Nice. I've found a bunch of issues playing with it already :) Hopefully I want to get a new patch version out tonight for this issue :D

Answer 10 · 2015-04-06T20:24:47.000Z

Awesome! Thanks for your attention to this, Doug. 😀

Answer 11 · 2015-04-06T20:25:56.000Z

No problem at all! The TypeError: Cannot read property 'constructor' of undefined is the most odd-ball error of the bunch :) I'm looking through the native V8 code to determine the best way without adding a slow try, haha

Answer 12 · 2015-04-06T20:26:17.000Z

👍 looking forward to seeing what you come up with.

Answer 13 · 2015-04-06T20:37:59.000Z

haha. and lol, this isn't super fun; seems V8 didn't completely think of all the different methods in their error API when they implemented use strict, haha. Seems I have to replicate some of those in my code instead of calling the error API methods.

Answer 14 · 2015-04-06T20:47:21.000Z

I think I have it mostly working so far :)

Answer 15 · 2015-04-06T22:09:54.000Z

The worse part is that from those API docs:

For those frames, getFunction() and getThis() will return undefined.

This is true... but not for Node.js < 0.12, which throw on getTypeName() from a bug, but Node.js 0.12+ correctly gets the type name.

Answer 16 · 2015-04-06T22:14:01.000Z

Hah. Sorry for opening up this can of worms. 😉

Answer 17 · 2015-04-06T22:15:45.000Z

lol. It's no problem. We'll get this fixed :D!

Answer 18 · 2015-04-06T22:36:26.000Z

Basically it seems like this part of V8 that is in Node.js 0.10 is buggy, lol

Answer 19 · 2015-04-07T00:27:11.000Z

Ok, so hopefully it's all fixed on master. If you'd be so kind to test using what you saw the issues with, that would be awesome :)

Answer 20 · 2015-04-07T02:12:01.000Z

Tried in my application and in my test cases. Everything works! Thanks, Doug!

Answer 21 · 2015-04-07T02:13:18.000Z

Sweet :D I plan to publish tonight (US Eastern time) as version 1.0.1

Answer 22 · 2015-04-07T02:13:55.000Z

Sounds good! Thanks again!

Answer 23 · 2015-04-07T17:59:28.000Z

I see it was released. Thanks again, Doug!

Answer 24 · 2015-04-07T19:55:57.000Z

No problem! Sorry, I got too busy last night to make the release :(

Answer 25 · 2015-04-07T20:16:05.000Z

Haha. Not a problem at all. I appreciate it!