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

Fix stack trace #4428

Merged
merged 14 commits into from
Jan 22, 2017
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions Cakefile
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,15 @@ task 'build:full', 'rebuild the source twice, and run the tests', ->
task 'build:parser', 'rebuild the Jison parser (run build first)', ->
helpers.extend global, require 'util'
require 'jison'
parser = require('./lib/coffee-script/grammar').parser
fs.writeFileSync 'lib/coffee-script/parser.js', parser.generate()
parser = require('./lib/coffee-script/grammar').parser.generate()
# Patch Jison’s output, until https://github.com/zaach/jison/pull/339 is accepted,
# to ensure that require('fs') is only called where it exists.
parser = parser.replace "var source = require('fs')", """
var source = null;
var fs = require('fs');
if (typeof fs !== 'undefined' && fs !== null)
source = fs"""
fs.writeFileSync 'lib/coffee-script/parser.js', parser


task 'build:browser', 'rebuild the merged script for inclusion in the browser', ->
Expand Down
113 changes: 109 additions & 4 deletions lib/coffee-script/coffee-script.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion lib/coffee-script/parser.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

96 changes: 94 additions & 2 deletions src/coffee-script.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -216,12 +216,24 @@ if require.extensions
Use CoffeeScript.register() or require the coffee-script/register module to require #{ext} files.
"""

# 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.
compiledFiles = {}

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
compiledFiles[filename] = stripped
compileCode stripped, filename, sourceMap, inlineMap

compileCode = (code, filename, sourceMap = no, inlineMap = no) ->
try
answer = compile stripped, {
answer = compile code, {
filename, sourceMap, inlineMap
sourceFiles: [filename]
literate: helpers.isLiterate filename
Expand All @@ -230,7 +242,7 @@ exports._compileFile = (filename, sourceMap = no, inlineMap = no) ->
# As the filename and code of a dynamically loaded file will be different
# from the original file compiled with CoffeeScript.run, add that
# information to error so it can be pretty-printed later.
throw helpers.updateSyntaxError err, stripped, filename
throw helpers.updateSyntaxError err, code, filename

answer

Expand Down Expand Up @@ -283,3 +295,83 @@ 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

# Map of filenames: sourceMap objects.
sourceMaps = {}
# Generates the source map for a coffee file and stores it in the local cache variable.
getSourceMap = (filename) ->
if sourceMaps[filename]?
sourceMaps[filename]
else if compiledFiles[filename]?
answer = compileCode compiledFiles[filename], filename, yes, no
sourceMaps[filename] = 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}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While were improving all of this anyway, I think we should take the opportunity to indent the "at" lines just like Node.js does. That is, 4 spaces instead of 2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, really? I hate 4-space indentation . . .

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also prefer 2-space indentation in general, but I thought it might be a good idea to match the standard error formatting as closely as possible?


"#{err.toString()}\n#{frames.join '\n'}\n"
46 changes: 46 additions & 0 deletions test/error_messages.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,21 @@ test "compiler error formatting with mixed tab and space", ->


if require?
os = require 'os'
fs = require 'fs'
path = require 'path'

test "patchStackTrace line patching", ->
err = new Error 'error'
ok err.stack.match /test[\/\\]error_messages\.coffee:\d+:\d+\b/

test "patchStackTrace stack prelude consistent with V8", ->
err = new Error
ok err.stack.match /^Error\n/ # Notice no colon when no message.

err = new Error 'error'
ok err.stack.match /^Error: error\n/

test "#2849: compilation error in a require()d file", ->
# Create a temporary file to require().
ok not fs.existsSync 'test/syntax-error.coffee'
Expand All @@ -74,6 +86,40 @@ if require?
finally
fs.unlinkSync 'test/syntax-error.coffee'

test "#3890 Error.prepareStackTrace doesn't throw an error if a compiled file is deleted", ->
# Adapted from https://github.com/atom/coffee-cash/blob/master/spec/coffee-cash-spec.coffee
filePath = path.join os.tmpdir(), 'PrepareStackTraceTestFile.coffee'
fs.writeFileSync filePath, "module.exports = -> throw new Error('hello world')"
throwsAnError = require(filePath)
fs.unlinkSync filePath

caughtError = null
try
throwsAnError()
catch error
caughtError = error

eq caughtError.message, 'hello world'
doesNotThrow(-> caughtError.stack)
ok caughtError.stack.toString().indexOf(filePath) isnt -1

test "#4418 stack traces reference the correct line number", ->
filePath = path.join os.tmpdir(), 'StackTraceLineNumberTestFile.coffee'
fileContents = "module.exports = ->\n"
fileContents += " line = #{i}\n" for i in [2..1337]
fileContents += " throw new Error('hello world')"
fs.writeFileSync filePath, fileContents
throwsAnError = require(filePath)
fs.unlinkSync filePath

caughtError = null
try
throwsAnError()
catch error
caughtError = error

ok "#{caughtError.stack.toString().indexOf(filePath)}:1339" isnt -1


test "#1096: unexpected generated tokens", ->
# Implicit ends
Expand Down
4 changes: 3 additions & 1 deletion test/repl.coffee
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
return if global.testingBrowser

os = require 'os'
fs = require 'fs'
path = require 'path'

# REPL
# ----
Expand Down Expand Up @@ -28,7 +30,7 @@ class MockOutputStream extends Stream
@written[@written.length - 1 + fromEnd].replace /\r?\n$/, ''

# Create a dummy history file
historyFile = '.coffee_history_test'
historyFile = path.join os.tmpdir(), '.coffee_history_test'
fs.writeFileSync historyFile, '1 + 2\n'

testRepl = (desc, fn) ->
Expand Down