error miss filename and line number in 4.x and 5.x when require file compile error

[dead-horse]

• Version: 4.4.7 and 5.12.0
• Platform: osx

when require a file within a syntax error, the error file name and line number won't set into err.message, but this is fixed in node 6. will we land this fix in node@4 ?

// t.js
try {
  require('./r.js')
} catch (err) {
  console.log(err.stack)
}

// r.js
function // syntax error

in 4 and 5, err.stack will be:

SyntaxError: Unexpected token }
    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> (/Users/deadhorse/git/basement/t.js:2:3)
    at Module._compile (module.js:413:34)
    at Object.Module._extensions..js (module.js:422:10)

in 0.12 and 6, err.stack will be:

/Users/deadhorse/git/basement/r.js:3
});
^
SyntaxError: Unexpected token }
    at Object.exports.runInThisContext (vm.js:53:16)
    at Module._compile (module.js:513:28)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/deadhorse/git/basement/t.js:2:3)
    at Module._compile (module.js:541:32)

[mscdex]

/cc @nodejs/lts Is this something that can be backported or are we pretty firm on not allowing any changes to error messages (especially in LTS)?

[rvagg]

Doubtful, is this coming directly from V8 so would mean patching V8? Can we narrow down the change that would need to be backported to assess the impact?

[rvagg]

Oh, no it's not from V8 is it, it's the bit with the filename isn't it? @mscdex do you know the commit here that changed this?

[mscdex]

Not offhand, no.

[evanlucas]

@rvagg 5700352?

[rvagg]

bingo, thanks @evanlucas, we may be able to treat this as a bugfix. @nodejs/lts see #4874 and weigh in

[MylesBorins]

errors are always a tough one, as it could be viewed as a contract. Modifying the stack message in particular could break things in unexpected ways. Personally I'll defer to @jasnell on this one as I know he has pretty strong opinions regarding error messages. It may also be interesting to hear how various APM providers feel about this

/cc @nodejs/diagnostics @watson @groundwater @Qard

[jasnell]

Yeah, while I recognize this is a bug fix, error messages are tricky things. I'm not that comfortable pulling this back.

[rvagg]

Fair enough, we have to be very conservative with v4. Thanks for the bug report @dead-horse but I don't think we can take this any further.

[MylesBorins]

@dead-horse I know that @jasnell was working on #6573 which has seemed to stall. This would potentially make it easier for us to make error messages extensible without requiring semver major bumps

[jasnell]

It's not so much stalled as it is pending. The ctc decision was to hold off
on landing the changes for a bit. I need to get it rebased and updated and
I'll be revisiting it in Aug/Sept.
On Jul 7, 2016 11:27 PM, "Myles Borins" notifications@github.com wrote:

@dead-horse https://github.com/dead-horse I know that @jasnell
https://github.com/jasnell was working on #6573
#6573 which has seemed to stall.
This would potentially make it easier for us to make error messages
extensible without requiring semver major bumps

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7578 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAa2eZpsqNj7d8B8Nnne_JdQGMH9xnaFks5qTe3agaJpZM4JGvs-
.

[bnoordhuis]

@manojkraftly No "fix my code for me" comment spam, please. Open an issue in nodejs/help, that's what it's for.

[manojkraftly]

@bnoordhuis I am not getting you, what you trying to explain me...

[bnoordhuis]

@manojkraftly You are necroing an old bug report that's only superficially similar to your issue. Don't do that, open an issue here.