-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix stack trace #4428
Merged
GeoffreyBooth
merged 14 commits into
jashkenas:master
from
GeoffreyBooth:fix-stack-trace
Jan 22, 2017
Merged
Fix stack trace #4428
Changes from 13 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
0b0323b
Revert aee27fbff03870c5479c6c33e6b1f1a32219420c
GeoffreyBooth c70541b
Patch Jison’s output so that it requires `fs` only if we’re truly in …
GeoffreyBooth e2ac4b9
Temporary fix for exceptions getting thrown when trying to generate a…
GeoffreyBooth 96a8776
Save the test REPL history in the system temp folder, not in the Coff…
GeoffreyBooth 86e7d56
Rewrite `getSourceMap` to never read a file from disk, and therefore …
GeoffreyBooth c83410b
Add test to verify that stack traces reference the correct line numbe…
GeoffreyBooth 51097cd
Get the parser working in the browser compiler again; rather than det…
GeoffreyBooth 316a9f3
Follow Node’s standard of 4-space indentation of stack trace data
GeoffreyBooth e117647
Merge branch 'master' of github.com:jashkenas/coffeescript into fix-s…
GeoffreyBooth 14876e4
Better .gitignore
GeoffreyBooth 23e9fc7
Fix caching of compiled code and source maps; add more tests to verif…
GeoffreyBooth 6f03c8a
Better fallback value for the parser source
GeoffreyBooth 27c2833
Fix the stack traces and tests when running in a browser
GeoffreyBooth f6a2384
Update the browser compiler so that @murrayju doesn’t have any extra …
GeoffreyBooth File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
raw | ||
presentation | ||
test.coffee | ||
test*.coffee | ||
test.litcoffee | ||
parser.output | ||
test*.litcoffee | ||
test/*.js | ||
parser.output | ||
/node_modules | ||
npm-debug.log | ||
npm-debug.log* |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,22 +46,38 @@ withPrettyErrors = (fn) -> | |
throw err if typeof code isnt 'string' # Support `CoffeeScript.nodes(tokens)`. | ||
throw helpers.updateSyntaxError err, code, options.filename | ||
|
||
# For each compiled file, save its source in memory in case we need to | ||
# recompile it later. We might need to recompile if the first compilation | ||
# didn’t create a source map (faster) but something went wrong and we need | ||
# a stack trace. Assuming that most of the time, code isn’t throwing | ||
# exceptions, it’s probably more efficient to compile twice only when we | ||
# need a stack trace, rather than always generating a source map even when | ||
# it’s not likely to be used. Save in form of `filename`: `(source)` | ||
sources = {} | ||
# Also save source maps if generated, in form of `filename`: `(source map)`. | ||
sourceMaps = {} | ||
|
||
# Compile CoffeeScript code to JavaScript, using the Coffee/Jison compiler. | ||
# | ||
# If `options.sourceMap` is specified, then `options.filename` must also be specified. All | ||
# options that can be passed to `SourceMap#generate` may also be passed here. | ||
# If `options.sourceMap` is specified, then `options.filename` must also be | ||
# specified. All options that can be passed to `SourceMap#generate` may also | ||
# be passed here. | ||
# | ||
# This returns a javascript string, unless `options.sourceMap` is passed, | ||
# in which case this returns a `{js, v3SourceMap, sourceMap}` | ||
# object, where sourceMap is a sourcemap.coffee#SourceMap object, handy for doing programatic | ||
# lookups. | ||
# object, where sourceMap is a sourcemap.coffee#SourceMap object, handy for | ||
# doing programmatic lookups. | ||
exports.compile = compile = withPrettyErrors (code, options) -> | ||
{merge, extend} = helpers | ||
options = extend {}, options | ||
generateSourceMap = options.sourceMap or options.inlineMap | ||
# Always generate a source map if no filename is passed in, since without a | ||
# a filename we have no way to retrieve this source later in the event that | ||
# we need to recompile it to get a source map for `prepareStackTrace`. | ||
generateSourceMap = options.sourceMap or options.inlineMap or not options.filename? | ||
filename = options.filename or '<anonymous>' | ||
|
||
if generateSourceMap | ||
map = new SourceMap | ||
sources[filename] = code | ||
map = new SourceMap if generateSourceMap | ||
|
||
tokens = lexer.tokenize code, options | ||
|
||
|
@@ -71,7 +87,7 @@ exports.compile = compile = withPrettyErrors (code, options) -> | |
token[1] for token in tokens when token[0] is 'IDENTIFIER' | ||
) | ||
|
||
# Check for import or export; if found, force bare mode | ||
# Check for import or export; if found, force bare mode. | ||
unless options.bare? and options.bare is yes | ||
for token in tokens | ||
if token[0] in ['IMPORT', 'EXPORT'] | ||
|
@@ -86,7 +102,7 @@ exports.compile = compile = withPrettyErrors (code, options) -> | |
currentColumn = 0 | ||
js = "" | ||
for fragment in fragments | ||
# Update the sourcemap with data from each fragment | ||
# Update the sourcemap with data from each fragment. | ||
if generateSourceMap | ||
# Do not include empty, whitespace, or semicolon-only fragments. | ||
if fragment.locationData and not /^[;\s]*$/.test fragment.code | ||
|
@@ -110,6 +126,7 @@ exports.compile = compile = withPrettyErrors (code, options) -> | |
|
||
if generateSourceMap | ||
v3SourceMap = map.generate(options, code) | ||
sourceMaps[filename] = map | ||
|
||
if options.inlineMap | ||
encoded = base64encode JSON.stringify v3SourceMap | ||
|
@@ -146,13 +163,13 @@ exports.run = (code, options = {}) -> | |
|
||
# Set the filename. | ||
mainModule.filename = process.argv[1] = | ||
if options.filename then fs.realpathSync(options.filename) else '.' | ||
if options.filename then fs.realpathSync(options.filename) else '<anonymous>' | ||
|
||
# Clear the module cache. | ||
mainModule.moduleCache and= {} | ||
|
||
# Assign paths for node_modules loading | ||
dir = if options.filename | ||
dir = if options.filename? | ||
path.dirname fs.realpathSync options.filename | ||
else | ||
fs.realpathSync '.' | ||
|
@@ -218,6 +235,7 @@ if require.extensions | |
|
||
exports._compileFile = (filename, sourceMap = no, inlineMap = no) -> | ||
raw = fs.readFileSync filename, 'utf8' | ||
# Strip the Unicode byte order mark, if this file begins with one. | ||
stripped = if raw.charCodeAt(0) is 0xFEFF then raw.substring 1 else raw | ||
|
||
try | ||
|
@@ -283,3 +301,87 @@ parser.yy.parseError = (message, {token}) -> | |
# from the lexer. | ||
helpers.throwSyntaxError "unexpected #{errorText}", errorLoc | ||
|
||
# Based on http://v8.googlecode.com/svn/branches/bleeding_edge/src/messages.js | ||
# Modified to handle sourceMap | ||
formatSourcePosition = (frame, getSourceMapping) -> | ||
filename = undefined | ||
fileLocation = '' | ||
|
||
if frame.isNative() | ||
fileLocation = "native" | ||
else | ||
if frame.isEval() | ||
filename = frame.getScriptNameOrSourceURL() | ||
fileLocation = "#{frame.getEvalOrigin()}, " unless filename | ||
else | ||
filename = frame.getFileName() | ||
|
||
filename or= "<anonymous>" | ||
|
||
line = frame.getLineNumber() | ||
column = frame.getColumnNumber() | ||
|
||
# Check for a sourceMap position | ||
source = getSourceMapping filename, line, column | ||
fileLocation = | ||
if source | ||
"#{filename}:#{source[0]}:#{source[1]}" | ||
else | ||
"#{filename}:#{line}:#{column}" | ||
|
||
functionName = frame.getFunctionName() | ||
isConstructor = frame.isConstructor() | ||
isMethodCall = not (frame.isToplevel() or isConstructor) | ||
|
||
if isMethodCall | ||
methodName = frame.getMethodName() | ||
typeName = frame.getTypeName() | ||
|
||
if functionName | ||
tp = as = '' | ||
if typeName and functionName.indexOf typeName | ||
tp = "#{typeName}." | ||
if methodName and functionName.indexOf(".#{methodName}") isnt functionName.length - methodName.length - 1 | ||
as = " [as #{methodName}]" | ||
|
||
"#{tp}#{functionName}#{as} (#{fileLocation})" | ||
else | ||
"#{typeName}.#{methodName or '<anonymous>'} (#{fileLocation})" | ||
else if isConstructor | ||
"new #{functionName or '<anonymous>'} (#{fileLocation})" | ||
else if functionName | ||
"#{functionName} (#{fileLocation})" | ||
else | ||
fileLocation | ||
|
||
getSourceMap = (filename) -> | ||
if sourceMaps[filename]? | ||
sourceMaps[filename] | ||
# CoffeeScript compiled in a browser may get compiled with `options.filename` | ||
# of `<anonymous>`, but the browser may request the stack trace with the | ||
# filename of the script file. | ||
else if sourceMaps['<anonymous>']? | ||
sourceMaps['<anonymous>'] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See also. |
||
else if sources[filename]? | ||
answer = compile sources[filename], | ||
filename: filename | ||
sourceMap: yes | ||
answer.sourceMap | ||
else | ||
null | ||
|
||
# Based on [michaelficarra/CoffeeScriptRedux](http://goo.gl/ZTx1p) | ||
# NodeJS / V8 have no support for transforming positions in stack traces using | ||
# sourceMap, so we must monkey-patch Error to display CoffeeScript source | ||
# positions. | ||
Error.prepareStackTrace = (err, stack) -> | ||
getSourceMapping = (filename, line, column) -> | ||
sourceMap = getSourceMap filename | ||
answer = sourceMap.sourceLocation [line - 1, column - 1] if sourceMap? | ||
if answer? then [answer[0] + 1, answer[1] + 1] else null | ||
|
||
frames = for frame in stack | ||
break if frame.getFunction() is exports.run | ||
" at #{formatSourcePosition frame, getSourceMapping}" | ||
|
||
"#{err.toString()}\n#{frames.join '\n'}\n" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lydell I changed this so that we have consistent “filenames” when compiling from
run
oreval
or other non-file sources, which is important for looking up cached sources and source maps later. Do you see any issue with this? Was there anything special about a filename of.
before?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't see any obvious issue, but on the other hand I have no idea if there was anything special about the
.
before either.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. I just ran the browser tests and test.html and ran into (and fixed) some issues, which is what led me to change this. We never had a stack trace line number test that ran in the browser before, so I’m not sure our previous stack traces ever worked in the browser (I suspect they didn’t). I hope there aren’t any other edge cases I’m not thinking of.