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

Wrong line numbers in stack trace. #4418

Closed
rayzorben opened this issue Jan 5, 2017 · 3 comments · Fixed by #4428
Closed

Wrong line numbers in stack trace. #4418

rayzorben opened this issue Jan 5, 2017 · 3 comments · Fixed by #4428
Labels

Comments

@rayzorben
Copy link

rayzorben commented Jan 5, 2017

Read on a few websites that just running 'coffee app.coffee' should generate source maps and print out correct debugging information.

Taken from another website, the simple example:

  1 test = ->
  2     people =
  3         john:
  4             first_name: 'john'
  5             last_name: 'doe'
  6         mary:
  7             first_name: 'mary'
  8             last_name: 'jane'
  9
 10     console.log("Welcome", people[p].first_name, people[p].last_name, "!!!")     for p in ['john', 'mary', 'josh']
 11
 12 process.nextTick test

When run with coffee test.coffee I get the following error

Welcome john doe !!!
Welcome mary jane !!!
/home/pi/onkyo/nodejs-onkyoserial/test.coffee:20
      results.push(console.log("Welcome", people[p].first_name, people[p].last_name, "!!!"));
                                                   ^

TypeError: Cannot read property 'first_name' of undefined
    at test (/home/pi/onkyo/nodejs-onkyoserial/test.coffee:20:52)
    at _combinedTickCallback (internal/process/next_tick.js:67:7)
    at process._tickCallback (internal/process/next_tick.js:98:9)
    at Module.runMain (module.js:607:11)
    at run (bootstrap_node.js:420:7)
    at startup (bootstrap_node.js:139:9)
    at bootstrap_node.js:535:3

Shows the error on test.coffee line 20, but there are only 12 lines in the file. I tried compiling it with a source map (-m) as well as with an inline map (-M) and running the resulting .js file through node, and still get errors on or around line 20.

Running coffee-script 1.12.2

@lydell
Copy link
Collaborator

lydell commented Jan 6, 2017

Thanks for the issue, @rayzorben!

My guess is that the following bullet point from the 1.12.2 changelog is the cause (see #4399 for more information):

The error-prone patched version of Error.prepareStackTrace has been removed.

Here is the output using CoffeeScript 1.12.1:

$ coffee test.coffee 
Welcome john doe !!!
Welcome mary jane !!!
/home/lydell/test.coffee:20
      results.push(console.log("Welcome", people[p].first_name, people[p].last_name, "!!!"));
                                                   ^

TypeError: Cannot read property 'first_name' of undefined
  at test (/home/lydell/test.coffee:10:37)
  at _combinedTickCallback (internal/process/next_tick.js:67:7)
  at process._tickCallback (internal/process/next_tick.js:98:9)
  at Module.runMain (module.js:607:11)
  at run (bootstrap_node.js:420:7)
  at startup (bootstrap_node.js:139:9)
  at bootstrap_node.js:535:3

It still says line 20 near the top, but in the stack trace it says line 10.

@davibe
Copy link

davibe commented Jan 18, 2017

i can confirm the same problem. It works as expected in 1.12.1 but it does not using 1.12.2.

Looks like a pretty major bug to me I don't see how this is getting so little attention.

w00t 2858543

@connec
Copy link
Collaborator

connec commented Jan 18, 2017

See #4399 for discussion on this.

GeoffreyBooth added a commit to GeoffreyBooth/coffeescript that referenced this issue Jan 21, 2017
GeoffreyBooth added a commit that referenced this issue Jan 22, 2017
* Revert aee27fb

* Patch Jison’s output so that it requires `fs` only if we’re truly in a CommonJS/Node environment, not a browser environment that may happen to have globals named `require` and `exports` (as would be the case if require.js is being used). Fixes #4391.

* Temporary fix for exceptions getting thrown when trying to generate a stack trace for a file that has been deleted since compilation; fixes #3890, but not well. A better solution would not try to recompile the file when trying to retrieve its stack trace.

* Save the test REPL history in the system temp folder, not in the CoffeeScript project folder

* Rewrite `getSourceMap` to never read a file from disk, and therefore not throw IO-related exceptions; source maps are either retrieved from memory, or the related source code is retrieved from memory to generate a new source map. Fixes #3890 the proper way.

* Add test to verify that stack traces reference the correct line number. Closes #4418.

* Get the parser working in the browser compiler again; rather than detecting a CommonJS environment generally, just check for `fs` before trying to use it

* Follow Node’s standard of 4-space indentation of stack trace data

* Better .gitignore

* Fix caching of compiled code and source maps; add more tests to verify correct line numbers in stack traces

* Better fallback value for the parser source

* Fix the stack traces and tests when running in a browser

* Update the browser compiler so that @murrayju doesn’t have any extra work to do to test this branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants