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

Stack trace api differences when in strict mode causing TypeErrors #17

Closed
jasisk opened this issue Apr 6, 2015 · 25 comments
Closed

Stack trace api differences when in strict mode causing TypeErrors #17

jasisk opened this issue Apr 6, 2015 · 25 comments
Assignees
Labels

Comments

@jasisk
Copy link

jasisk commented Apr 6, 2015

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

From the API docs:

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)
@dougwilson
Copy link
Owner

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

@dougwilson
Copy link
Owner

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

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

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.

@dougwilson
Copy link
Owner

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?

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

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

@jasisk jasisk changed the title Stack trace api differences when in strict mode causing problems Stack trace api differences when in strict mode causing TypeErrors Apr 6, 2015
@dougwilson dougwilson added the bug label Apr 6, 2015
@dougwilson dougwilson self-assigned this Apr 6, 2015
@dougwilson
Copy link
Owner

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

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

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.

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

'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)

@dougwilson
Copy link
Owner

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

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

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

@dougwilson
Copy link
Owner

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

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

👍 looking forward to seeing what you come up with.

@dougwilson
Copy link
Owner

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.

@dougwilson
Copy link
Owner

I think I have it mostly working so far :)

@dougwilson
Copy link
Owner

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.

@jasisk
Copy link
Author

jasisk commented Apr 6, 2015

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

@dougwilson
Copy link
Owner

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

@dougwilson
Copy link
Owner

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

@dougwilson
Copy link
Owner

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 :)

@jasisk
Copy link
Author

jasisk commented Apr 7, 2015

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

@dougwilson
Copy link
Owner

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

@jasisk
Copy link
Author

jasisk commented Apr 7, 2015

Sounds good! Thanks again!

@jasisk
Copy link
Author

jasisk commented Apr 7, 2015

I see it was released. Thanks again, Doug!

@dougwilson
Copy link
Owner

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

@jasisk
Copy link
Author

jasisk commented Apr 7, 2015

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants