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

SyntaxError objects should contain information about where the syntax error happened #3411

Closed
fresheneesz opened this issue Oct 16, 2015 · 9 comments
Labels
vm Issues and PRs related to the vm subsystem.

Comments

@fresheneesz
Copy link

Consider these files:

test.js:

try {
    require("./testmodule")
} catch(e) {
    console.log(e.stack)
}

testmodule.js:

}

In this case, the result is the following text:

SyntaxError: Unexpected token }
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:413:25)
    at Object.Module._extensions..js (module.js:452:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/home/vagrant/temp/test.js:2:5)
    at Module._compile (module.js:434:26)
    at Object.Module._extensions..js (module.js:452:10)

Notice that it doesn't give you any information about what module that unexpected token is in nor what line in the module that unexpected token was found at. This makes it pretty hard to debug in cases where you're requiring a module somewhere other than at the very top of your source.

You can usually infer the module its in by looking at the line in test.js that it indicates, but if you have more than one module required in the same line, that won't tell you which one.

If you don't catch the SyntaxError, you get the following additional info:

/home/vagrant/temp/testmodule.js:3
});
^

This is super helpful (although it incorrectly contains some boilerplate - the ); that doesn't actually exist in the file), and should be contained in the error's message and stack properties. At very least, the error object should contain those two pieces of information somewhere.

@vkurchatkin vkurchatkin added the vm Issues and PRs related to the vm subsystem. label Oct 16, 2015
@ChuckLangford
Copy link
Contributor

This appears to be related to the following issue and pull request:
#2104
#2108

@tflanagan
Copy link
Contributor

That ); is actually the code your module is wrapped with before being executed.

@fresheneesz
Copy link
Author

@tflanagan Yes I expected that to be the case, but it still ideally shouldn't appear in the exception.

@nouex
Copy link

nouex commented Oct 19, 2015

Also related to issue #2762

@setthase
Copy link

+1 for fixing this.

I spent half day trying to fix things in a wrong file...

cjihrig added a commit to cjihrig/node that referenced this issue Nov 25, 2015
Currently, when a file with a syntax error is imported in the
REPL, no information is provided on the error's location. This
commit adds the error's location to the stack trace.

Refs: nodejs#2762
Refs: nodejs#3411
Refs: nodejs#3784
PR-URL: nodejs#4013
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
cjihrig added a commit that referenced this issue Dec 5, 2015
Currently, when a file with a syntax error is imported in the
REPL, no information is provided on the error's location. This
commit adds the error's location to the stack trace.

Refs: #2762
Refs: #3411
Refs: #3784
PR-URL: #4013
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
cjihrig added a commit to cjihrig/node that referenced this issue Jan 7, 2016
Currently, when a file with a syntax error is imported in the
REPL, no information is provided on the error's location. This
commit adds the error's location to the stack trace.

Refs: nodejs#2762
Refs: nodejs#3411
Refs: nodejs#3784
PR-URL: nodejs#4013
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
MylesBorins pushed a commit that referenced this issue Jan 19, 2016
Currently, when a file with a syntax error is imported in the
REPL, no information is provided on the error's location. This
commit adds the error's location to the stack trace.

Refs: #2762
Refs: #3411
Refs: #3784
PR-URL: #4013
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
scovetta pushed a commit to scovetta/node that referenced this issue Apr 2, 2016
Currently, when a file with a syntax error is imported in the
REPL, no information is provided on the error's location. This
commit adds the error's location to the stack trace.

Refs: nodejs#2762
Refs: nodejs#3411
Refs: nodejs#3784
PR-URL: nodejs#4013
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
@matthewloring
Copy link

@fresheneesz This should be fixed by #4874. Can you confirm this issue is fixed on master?

@cjihrig
Copy link
Contributor

cjihrig commented Jun 16, 2016

This was fixed in v6, and the module wrapper was documented. Closing.

@cjihrig cjihrig closed this as completed Jun 16, 2016
@ghost
Copy link

ghost commented Jul 19, 2017

I don't understand why this is closed.. This is still an issue in Node v8.x

@gibfahn
Copy link
Member

gibfahn commented Jul 22, 2017

I don't understand why this is closed.. This is still an issue in Node v8.x

This was closed because #4874 landed as 5700352. That commit has been in every Node release since 6.0.0 (including all 8.x releases).

If you're still seeing this issue could you provide more details? Ideally let us know why 5700352 didn't fix your problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests

9 participants