diff --git a/lib/reporter.js b/lib/reporter.js index 9ea87a1c6..f7dad1039 100644 --- a/lib/reporter.js +++ b/lib/reporter.js @@ -1,4 +1,5 @@ var util = require('util') +var resolve = require('url').resolve var log = require('./logger').create('reporter') var MultiReporter = require('./reporters/multi') var baseReporterDecoratorFactory = require('./reporters/base').decoratorFactory @@ -23,7 +24,7 @@ var createErrorFormatter = function (basePath, emitter, SourceMapConsumer) { } var URL_REGEXP = new RegExp('(?:https?:\\/\\/[^\\/]*)?\\/?' + - '(base|absolute)' + // prefix + '(base/|absolute)' + // prefix, including slash for base/ to create relative paths. '((?:[A-z]\\:)?[^\\?\\s\\:]*)' + // path '(\\?\\w*)?' + // sha '(\\:(\\d+))?' + // line @@ -53,11 +54,8 @@ var createErrorFormatter = function (basePath, emitter, SourceMapConsumer) { // remove domain and timestamp from source files // and resolve base path / absolute path urls into absolute path var msg = input.replace(URL_REGEXP, function (_, prefix, path, __, ___, line, ____, column) { - if (prefix === 'base') { - path = basePath + path - } - - var file = findFile(path) + // Find the file using basePath + path, but use the more readable path down below. + var file = findFile(prefix === 'base/' ? basePath + '/' + path : path) if (file && file.sourceMap && line) { line = parseInt(line || '0', 10) @@ -72,9 +70,12 @@ var createErrorFormatter = function (basePath, emitter, SourceMapConsumer) { var original = getSourceMapConsumer(file.sourceMap) .originalPositionFor({line: line, column: (column || 0), bias: bias}) + // Source maps often only have a local file name, resolve to turn into a full path if + // the path is not absolute yet. + var sourcePath = resolve(path, original.source) var formattedColumn = column ? util.format(':%s', column) : '' - return util.format('%s:%d%s <- %s:%d:%d', path, line, formattedColumn, original.source, - original.line, original.column) + return util.format('%s:%d:%d <- %s:%d%s', sourcePath, original.line, original.column, + path, line, formattedColumn) } catch (e) { log.warn('SourceMap position not found for trace: %s', msg) // Fall back to non-source-mapped formatting. diff --git a/test/unit/reporter.spec.js b/test/unit/reporter.spec.js index a60692d6c..5b5b88507 100644 --- a/test/unit/reporter.spec.js +++ b/test/unit/reporter.spec.js @@ -45,18 +45,18 @@ describe('reporter', () => { }) it('should remove domain from files', () => { - expect(formatError('file http://localhost:8080/base/usr/a.js and http://127.0.0.1:8080/base/home/b.js')).to.be.equal('file /usr/a.js and /home/b.js\n') + expect(formatError('file http://localhost:8080/base/usr/a.js and http://127.0.0.1:8080/absolute/home/b.js')).to.be.equal('file usr/a.js and /home/b.js\n') }) // TODO(vojta): enable once we serve source under urlRoot it.skip('should handle non default karma service folders', () => { formatError = m.createErrorFormatter('', '/_karma_/') - expect(formatError('file http://localhost:8080/_karma_/base/usr/a.js and http://127.0.0.1:8080/_karma_/base/home/b.js')).to.be.equal('file /usr/a.js and /home/b.js\n') + expect(formatError('file http://localhost:8080/_karma_/base/usr/a.js and http://127.0.0.1:8080/_karma_/base/home/b.js')).to.be.equal('file usr/a.js and home/b.js\n') }) it('should remove shas', () => { var ERROR = 'file http://localhost:8080/base/usr/file.js?6e31cb249ee5b32d91f37ea516ca0f84bddc5aa9 and http://127.0.0.1:8080/absolute/home/file.js?6e31cb249ee5b32d91f37ea516ca0f84bddc5aa9' - expect(formatError(ERROR)).to.be.equal('file /usr/file.js and /home/file.js\n') + expect(formatError(ERROR)).to.be.equal('file usr/file.js and /home/file.js\n') }) it('should indent all lines', () => { @@ -65,7 +65,7 @@ describe('reporter', () => { it('should restore base paths', () => { formatError = m.createErrorFormatter('/some/base', emitter) - expect(formatError('at http://localhost:123/base/a.js?123')).to.equal('at /some/base/a.js\n') + expect(formatError('at http://localhost:123/base/a.js?123')).to.equal('at a.js\n') }) it('should restore absolute paths', () => { @@ -90,10 +90,11 @@ describe('reporter', () => { describe('source maps', () => { var originalPositionForCallCount = 0 + var sourceMappingPath = null class MockSourceMapConsumer { constructor (sourceMap) { - this.source = sourceMap.content.replace('SOURCE MAP ', '/original/') + this.source = sourceMap.content.replace('SOURCE MAP ', sourceMappingPath) } originalPositionFor (position) { @@ -112,6 +113,7 @@ describe('reporter', () => { beforeEach(() => { originalPositionForCallCount = 0 + sourceMappingPath = '/original/' }) MockSourceMapConsumer.GREATEST_LOWER_BOUND = 1 @@ -127,7 +129,7 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at http://localhost:123/base/b.js:2:6' - expect(formatError(ERROR)).to.equal('at /some/base/b.js:2:6 <- /original/b.js:4:8\n') + expect(formatError(ERROR)).to.equal('at /original/b.js:4:8 <- b.js:2:6\n') done() }) }) @@ -142,7 +144,7 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at http://localhost:123/base/b.js:2' - expect(formatError(ERROR)).to.equal('at /some/base/b.js:2 <- /original/b.js:4:2\n') + expect(formatError(ERROR)).to.equal('at /original/b.js:4:2 <- b.js:2\n') done() }) }) @@ -157,7 +159,22 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at /base/b.js:2:6' - expect(formatError(ERROR)).to.equal('at /some/base/b.js:2:6 <- /original/b.js:4:8\n') + expect(formatError(ERROR)).to.equal('at /original/b.js:4:8 <- b.js:2:6\n') + done() + }) + }) + + it('should resolve relative urls from source maps', (done) => { + sourceMappingPath = 'original/' // Note: relative path. + formatError = m.createErrorFormatter('/some/base', emitter, MockSourceMapConsumer) + var servedFiles = [new File('/some/base/path/a.js')] + servedFiles[0].sourceMap = {content: 'SOURCE MAP a.fancyjs'} + + emitter.emit('file_list_modified', {served: servedFiles}) + + _.defer(() => { + var ERROR = 'at /base/path/a.js:2:6' + expect(formatError(ERROR)).to.equal('at path/original/a.fancyjs:4:8 <- path/a.js:2:6\n') done() }) }) @@ -172,7 +189,7 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at http://localhost:123/base/b.js:0:0' - expect(formatError(ERROR)).to.equal('at /some/base/b.js\n') + expect(formatError(ERROR)).to.equal('at b.js\n') done() }) }) @@ -187,7 +204,7 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at http://localhost:123/base/b.js' - expect(formatError(ERROR)).to.equal('at /some/base/b.js\n') + expect(formatError(ERROR)).to.equal('at b.js\n') expect(originalPositionForCallCount).to.equal(0) done() }) @@ -208,7 +225,7 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at http://localhost:123/absoluteC:/a/b/c.js:2:6' - expect(formatError(ERROR)).to.equal('at C:/a/b/c.js:2:6 <- /original/b.js:4:8\n') + expect(formatError(ERROR)).to.equal('at c:/original/b.js:4:8 <- C:/a/b/c.js:2:6\n') done() }) }) @@ -218,7 +235,7 @@ describe('reporter', () => { _.defer(() => { var ERROR = 'at http://localhost:123/absoluteC:/a/b/c.js?da39a3ee5e6:2:6' - expect(formatError(ERROR)).to.equal('at C:/a/b/c.js:2:6 <- /original/b.js:4:8\n') + expect(formatError(ERROR)).to.equal('at c:/original/b.js:4:8 <- C:/a/b/c.js:2:6\n') done() }) })