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

require(): no information about where error #3784

Closed
amurchick opened this issue Nov 12, 2015 · 13 comments
Closed

require(): no information about where error #3784

amurchick opened this issue Nov 12, 2015 · 13 comments
Labels
module Issues and PRs related to the module subsystem.

Comments

@amurchick
Copy link

When errors in require()'d file:

$ node -v
v5.0.0
$ node
> require('./auth')
SyntaxError: Unexpected identifier
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:404:25)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at repl:1:5
    at REPLServer.defaultEval (repl.js:248:27)
    at bound (domain.js:280:14)

I am understand - error in auth.js, but no info where error occurred was provided - files sometimes big and no quick way to localize error.

Early - some useful diagnostics was provided:

$ node -v
v0.12.7
$ node
> require('./current')
/path/to/file/current:1
unction (exports, require, module, __filename, __dirname) { 2015-11-11_23:04:3
                                                                    ^^
SyntaxError: Unexpected token ILLEGAL
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    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 Module.require (module.js:365:17)
    at require (module.js:384:17)
    at repl:1:1
    at REPLServer.defaultEval (repl.js:132:27)
    at bound (domain.js:254:14)
> 
@mscdex mscdex added the module Issues and PRs related to the module subsystem. label Nov 12, 2015
@evanlucas
Copy link
Contributor

you should be able to run node -c <file> to do a syntax check.

@thefourtheye
Copy link
Contributor

@amurchick I believe this is specific to REPL. Can you please confirm if my understanding is correct?

@cjihrig
Copy link
Contributor

cjihrig commented Nov 15, 2015

It is REPL specific.

@Zirak
Copy link
Contributor

Zirak commented Nov 15, 2015

Looking into it a bit, the difference between requireing the file and loading it directly is interesting.
The require flow goes through loading the file and then calling node::ContextifyScript::New (vm.js, node_contextify.cc) which propagates the SyntaxError to the javascript-land caller.

On the other hand, towards the end of the regular load's flow node::ReportException is called, which prints the origin of the exception from a hidden value: arrow = err_obj->GetHiddenValue(env->arrow_message_string());

The naive first thought will be to call ReportException in ContextifyScript::New , but that means the error will be printed even when the exception is caught.
The better second thought will be to expose the currently hidden value on the error object, but that'll single out node as the only environment to have this behaviour.
The even better third thought will be to expose ReportException as a binding, and have repl call that on uncaught exceptions. That'll not only solve this issue, but also give more detailed errors on regular errors, which may or may not be desirable.

Thoughts?

@amurchick
Copy link
Author

@thefourtheye @cjihrig - it is NOT REPL specific. Try this:

$ node -v
v5.0.0
$ cat test.js 
try {
    var a = require('./auth');
} catch (e) {
    console.error(e.stack);
}
$ cat auth.js 
module.exports = {
    a: 10
    b: 15
};
$ node test
SyntaxError: Unexpected identifier
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:404:25)
    at Object.Module._extensions..js (module.js:432:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:311:12)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at Object.<anonymous> (./test.js:2:10)
    at Module._compile (module.js:425:26)
    at Object.Module._extensions..js (module.js:432:10)

@cjihrig
Copy link
Contributor

cjihrig commented Nov 16, 2015

Ah, sorry. Misunderstanding. I meant that the legacy behavior is there if you don't catch the error. For example, if you drop the try...catch in your last code sample. It comes down to the ReportException() difference, as explained above.

I would like to work on this, I'm just trying to figure out what the best approach would be. Exposing ReportException() seems like it would require a good bit of work, since that function is pretty much hard coded to use the process stderr, while the REPL can print to arbitrary streams. I think all we really want is the hidden arrow message string, but it feels a little hackish to expose a function just for that.

I'm willing to do the work once it's decided how we want to do this.

@Fishrock123
Copy link
Member

Is this possibly related to #2860?

@cjihrig
Copy link
Contributor

cjihrig commented Nov 16, 2015

I don't think so. #2860 looks to be related to the module wrapper causing incorrect offsets (#2867 appears to be a fix for that one, and I'm going to look into today).

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>
@bnoordhuis
Copy link
Member

Working again in master and I believe 93afc39 is responsible for that. I'll close the issue.

@amurchick
Copy link
Author

Issue still not fixed:

$ uname -a
Linux server 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt11-1+deb8u3 (2015-08-04) x86_64 GNU/Linux

$ node -v
v5.9.0

$ cat test.js 
try {
    var a = require('./auth');
} catch (e) {
    console.error(e.stack);
}

$ cat auth.js
module.exports = {
    a: 10
    b: 15
};

$ node test
SyntaxError: Unexpected identifier
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:387:25)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at Object.<anonymous> (/some-path/test.js:2:13)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)

In REPL all ok:

$ node
> require('./auth')
/some-path/auth.js:3
    b: 15
    ^
SyntaxError: Unexpected identifier
    at exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:387:25)
    at Object.Module._extensions..js (module.js:422:10)
    at Module.load (module.js:357:32)
    at Function.Module._load (module.js:314:12)
    at Module.require (module.js:367:17)
    at require (internal/module.js:16:19)
    at repl:1:1
    at REPLServer.defaultEval (repl.js:260:27)
    at bound (domain.js:287:14)
> 

@bnoordhuis
Copy link
Member

@amurchick It's fixed in master and will be released as v6.0.0 later this month.

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>
@lichenhao
Copy link

I got the same problem with node@v4.4.6

@cjihrig
Copy link
Contributor

cjihrig commented Jun 26, 2016

@lichenhao please try with the v6 release line. This isn't going to be fixed in v4.

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

No branches or pull requests

9 participants