nodejs / node-v0.x-archive

Moved to https://github.com/nodejs/node
34.43k stars 7.31k forks source link

vm.runInNewContext/createScript doesn't throw line numbers on syntax errors #2734

Closed rummik closed 12 years ago

rummik commented 12 years ago

Shouldn't the expected result be to throw line numbers when attempting to execute code with syntax errors?

Here's a gist: https://gist.github.com/1792232

bnoordhuis commented 12 years ago

Thanks for the report. It's a known issue but there's not much we can do about it, there is no way to get the line numbers from the v8::Script object after a SyntaxError. Please open a V8 issue if you would like to see it fixed.

bminer commented 12 years ago

Commented on the issue upstream in V8: http://code.google.com/p/v8/issues/detail?id=1914 Related issue: #229

Anyone requesting this feature can "star" the issue. :)

bminer commented 12 years ago

@bnoordhuis - Would you (or someone else) mind taking a look at issue #1914 on Google V8? The V8 folks added this feature to the "bleeding edge" branch. Not sure when it will hit an official stable release, but I wanted to see if someone from the Node dev team could take a quick peek to make sure we can get this feature added to Node as quickly as possible.

Thanks!!!

bnoordhuis commented 12 years ago

@bminer I've discussed it but the consensus was to wait for it to show up in a stable V8 release, we're already floating more patches than we'd like.

bminer commented 12 years ago

I agree with the consensus. Just wanted to see if someone from the Node team could take a look at the patch to see if it would work nicely with Node and allow Node to report line numbers of syntax errors in eval'ed code.

bminer commented 12 years ago

@bnoordhuis - Sorry to be a bother. Once this patch appears in a stable Google V8 release, do you think it will be easy to modify Node to allows users to access syntax errors in eval'ed code? Any ideas for how this will be made possible? I'd imagine that Node would need to change how it parses error messages?

For example, right now, if eval'ed code contains a syntax error, the thrown Error object only contains the line/column number of the eval() call. That's nice and all, but it would be REALLY cool to get the line/column number in the eval'ed string that contained the error, as well.

Please let me know if there is anything I can do to help. Thanks!

bminer commented 12 years ago

@bnoordhuis - Google V8 version 3.12.0 now allows one to access these error messages. It looks like Node 0.7.11 is just one version of Google V8 behind, based on V8 version 3.11.10.

Please re-open this issue and issue #229. Thanks!

bnoordhuis commented 12 years ago

Google V8 version 3.12.0 now allows one to access these error messages. It looks like Node 0.7.11 is just one version of Google V8 behind, based on V8 version 3.11.10.

The major version of V8 (3.11) is fixed for the lifetime of v0.8, we're only applying patchlevel upgrades. Try again in v0.9. :-)

bminer commented 12 years ago

@bnoordhuis - That's not a big deal; I understand that you guys have to draw the line somewhere. Can we at least get this issue re-opened? That way this ends up on the radar for v0.9. Thanks for your help!

bnoordhuis commented 12 years ago

Sure. Reopened.

bminer commented 12 years ago

@bnoordhuis - Any idea when V8 will be upgraded to 3.12 in Node 0.9?

bminer commented 12 years ago

@bnoordhuis - Google V8 is now on 3.13.7. Any idea when Node 0.9 will be upgraded to the latest version of Google V8? In addition, Google Chrome on my laptop is using Google V8 3.12.19. There is a cool feature in V8 3.12 that allows one to view the line number and column number of an Error thrown when eval()-ing JavaScript code. For template engines like Jade et al, a feature like this would be REALLY nice for developers.

I'd like to incorporate a feature like this into my project node-blade.

bnoordhuis commented 12 years ago

@bminer master ships 3.13.7.1 at the moment (and has for two weeks), I forgot to close this issue.

bminer commented 12 years ago

@bnoordhuis - Thanks! Didn't bother checking master branch... only looked at 0.9.2, I think. Looking forward to 0.9.3!