jashkenas / coffeescript

Unfancy JavaScript
https://coffeescript.org/
MIT License
16.5k stars 1.98k forks source link

Bug: Stack traces from CoffeeScript.eval have JS line numbers #5216

Open edemaine opened 5 years ago

edemaine commented 5 years ago

In what I believe is a bug, line numbers in stack traces from errors thrown by CoffeeScript code executed via CoffeeScript.eval() (in NodeJS) give the JavaScript line numbers instead of the CoffeeScript line numbers.

Simple example

CoffeeScript = require 'coffeescript'
CoffeeScript.eval '''
  if undefined.foo
    weird = true # cause var hoist to shift lines
'''

The resulting stack trace and initial message uses a line number of 3 (current behavior) instead of 1 (expected behavior):

evalmachine.<anonymous>:3
if ((void 0).foo) {
             ^

TypeError: Cannot read property 'foo' of undefined
    at evalmachine.<anonymous>:3:14
    at Script.runInThisContext (vm.js:91:20)
    at Object.runInThisContext (vm.js:298:38)
    at Object.CoffeeScript.eval (...\node_modules\coffeescript\lib\coffeescript\index.js:127:17)
    at repl:2:28
    at repl:3:3
...

More real example with filename

In my application, the eval'd string actually comes from a .coffee file, and I can get the filename to appear correctly with enough options to CoffeeScript.eval (though admittedly I don't understand why so many arguments are needed for this to happen), yet the line numbers remain the same.

CoffeeScript.eval 'if undefined.foo then weird = true',
  filename: 'test.coffee'
  sourceFiles: ['test.coffee']
  inlineMap: true

results in:

test.coffee:3
if ((void 0).foo) {
             ^

TypeError: Cannot read property 'foo' of undefined
    at evalmachine.<anonymous>:3:14

Workaround

In my application, I constructed a workaround that corrects the line number in the initial message by using CoffeeScript.compile to get a source map, looking up/mapping the line number, and modifying the error's stack trace.

Proposed Solution

I think it would make sense for CoffeeScript.eval to do this kind of mangling of error stack traces. The REPL already does mangling of SyntaxErrors via helpers.updateSyntaxError, and some of this code can probably be shared. (It was the inspiration for my workaround.)

Related issues

Possibly related to #5129 and #4645. In particular, errors from the REPL seem to start every error with repl:2 whereas repl:1 would make more sense. I believe this is the same issue, so would also get fixed (though REPL seems less important to me).

Environment

thisredone commented 5 years ago

This problem, more importantly (IMO) also shows up with the CLI. I run CoffeeScript with the coffee CLI command for development all the time. This is the most frustrating part when it comes to otherwise blissful experience in developing CoffeeScript applications. For all non-trivial applications code run with coffee spits out javascript stack trace line numbers.

https://github.com/jashkenas/coffeescript/issues/5129 - this issue has been closed unfortunately

edemaine commented 5 years ago

Another approach would be to get Node to report proper line numbers in code with a sourcemap. I recently tried to get source-map-support to do this (optionally in conjunction with babel-plugin-stack-trace-sourcemap), but I couldn't get it to change line numbers at all (though it was clearly engaged, as it printed slightly different stack traces), even for Babel-supported language conversion (I tried JSX -> JS, as well as CS -> JS, with inline sourcemaps). If anyone has more luck with this, it'd be a viable alternative both in my application and in the coffee CLI.

GeoffreyBooth commented 5 years ago

Are you using the transpile option? Does that make a difference?

edemaine commented 5 years ago

@GeoffreyBooth I think Babel transpiling only makes things worse. I just tried transpile: presets: ['@babel/env'] on the simple example, and got evalmachine.<anonymous>:5 (was 3 without transpile, expected behavior is 1). Babel has a mode where it tries to preserve line numbers, which would probably bump it back to 3.

bcoe commented 5 years ago

👋 having worked on Istanbul for years, the lack of source-map support in transpiled code was really bugging me, guess what, we've built initial support into Node.js now for this:

https://nodejs.org/dist/latest-v12.x/docs/api/cli.html#cli_enable_source_maps

I've tested with compiled CoffeeScript code and it works like a charm.

There's still some work to be done, would love help and feedback:

https://github.com/nodejs/node/issues/29865


It's worth noting, I'm not 100% sure how we'd support eval behavior in Node.js or CoffeeScript, however, functionality seems to be most of the way there when using the CLI.

GeoffreyBooth commented 5 years ago

Thank you @bcoe! I saw that PR against Node and I was tentatively hopeful that it might somehow help CoffeeScript 😄 Do you have a demo repo where you can share how you’ve tested it?

Something that might help you is the monkey-patching of Error that coffeescript/register does:

https://github.com/jashkenas/coffeescript/blob/master/src/coffeescript.coffee#L341-L355

thisredone commented 4 years ago

One thing I've found - which is substantial but not a silver bullet unfortunately from what I can tell - is that any code in the dependency tree that does:

require 'coffeescript'

fucks up stack trace line numbers for all other code when the version of coffeescript they're using is old. A way of mitigating that (although a sub-optimal one) is via npm's shrinkwrap, yarn's "resolutions" or pnpm "hooks". This can lead to problems in the dependencies though.

Inve1951 commented 4 years ago

Found one scenario where you get invalid line numbers.

5333 when using --transpile.

The js file still gets written to disk and the line numbers don't even align with that. My guess is that it's the line numbers of the js file before it's been processed/adjusted by babel.