diff --git a/lib/reporter.js b/lib/reporter.js index da0dedb..42886c6 100644 --- a/lib/reporter.js +++ b/lib/reporter.js @@ -15,6 +15,7 @@ export default class Reporter { this.request = params.request || window.fetch this.alwaysReport = params.hasOwnProperty('alwaysReport') ? params.alwaysReport : false this.reportPreviousErrors = params.hasOwnProperty('reportPreviousErrors') ? params.reportPreviousErrors : true + this.resourcePath = this.normalizePath(params.resourcePath || process.resourcesPath) this.reportedErrors = [] this.reportedAssertionFailures = [] } @@ -57,7 +58,7 @@ export default class Reporter { buildStackTraceJSON (error, projectRoot) { return this.parseStackTrace(error).map(callSite => { return { - file: this.normalizePath(callSite.getFileName()), + file: this.scrubPath(callSite.getFileName()), method: callSite.getMethodName() || callSite.getFunctionName() || "none", lineNumber: callSite.getLineNumber(), columnNumber: callSite.getColumnNumber(), @@ -66,12 +67,22 @@ export default class Reporter { }) } - normalizePath (path) { - return path.replace('file:///', '') // Randomly inserted file url protocols - .replace(/[/]/g, '\\') // Temp switch for Windows home matching - .replace(fs.getHomeDirectory(), '~') // Remove users home dir for apm-dev'ed packages - .replace(/\\/g, '/') // Switch \ back to / for everyone - .replace(/.*(\/(app\.asar|packages\/).*)/, '$1') // Remove everything before app.asar or pacakges + normalizePath (pathToNormalize) { + return pathToNormalize + .replace('file:///', '') // Sometimes it's a uri + .replace(/\\/g, '/') // Unify path separators across Win/macOS/Linux + } + + scrubPath (pathToScrub) { + const absolutePath = this.normalizePath(pathToScrub) + + if (this.isBundledFile(absolutePath)) { + return this.normalizePath(path.relative(this.resourcePath, absolutePath)) + } else { + return absolutePath + .replace(this.normalizePath(fs.getHomeDirectory()), '~') // Remove users home dir + .replace(/.*(\/packages\/.*)/, '$1') // Remove everything before app.asar or packages + } } getDefaultNotificationParams () { @@ -107,7 +118,7 @@ export default class Reporter { const topFrame = this.parseStackTrace(error)[0] const fileName = topFrame ? topFrame.getFileName() : null - return fileName && (isBundledFile(fileName) || isTeletypeFile(fileName)) + return fileName && (this.isBundledFile(fileName) || this.isTeletypeFile(fileName)) } parseStackTrace (error) { @@ -240,16 +251,17 @@ export default class Reporter { setRequestFunction (requestFunction) { this.request = requestFunction } -} -function isBundledFile (fileName) { - return fileName.indexOf(atom.getLoadSettings().resourcePath) === 0 -} + isBundledFile (fileName) { + return this.normalizePath(fileName).indexOf(this.resourcePath) === 0 + } -function isTeletypeFile (fileName) { - const teletypePath = atom.packages.resolvePackagePath('teletype') - return teletypePath && fileName.indexOf(teletypePath) === 0 + isTeletypeFile (fileName) { + const teletypePath = atom.packages.resolvePackagePath('teletype') + return teletypePath && this.normalizePath(fileName).indexOf(teletypePath) === 0 + } } + Reporter.API_KEY = API_KEY Reporter.LIB_VERSION = LIB_VERSION diff --git a/spec/reporter-spec.js b/spec/reporter-spec.js index 53823b1..2be509b 100644 --- a/spec/reporter-spec.js +++ b/spec/reporter-spec.js @@ -1,6 +1,8 @@ const Reporter = require('../lib/reporter') const semver = require('semver') const os = require('os') +const path = require('path') +const fs = require('fs-plus') let osVersion = `${os.platform()}-${os.arch()}-${os.release()}` let getReleaseChannel = version => { @@ -12,7 +14,7 @@ let getReleaseChannel = version => { } describe("Reporter", () => { - let reporter, requests, initialStackTraceLimit, mockActivePackages + let reporter, requests, initialStackTraceLimit, initialFsGetHomeDirectory, mockActivePackages beforeEach(() => { reporter = new Reporter({ @@ -27,12 +29,24 @@ describe("Reporter", () => { initialStackTraceLimit = Error.stackTraceLimit Error.stackTraceLimit = 1 + initialFsGetHomeDirectory = fs.getHomeDirectory }) - afterEach(() => Error.stackTraceLimit = initialStackTraceLimit) + afterEach(() => { + fs.getHomeDirectory = initialFsGetHomeDirectory + Error.stackTraceLimit = initialStackTraceLimit + }) describe(".reportUncaughtException(error)", () => { - it("posts errors to bugsnag", () => { + it("posts errors originated inside Atom Core to BugSnag", () => { + const repositoryRootPath = path.join(__dirname, '..') + reporter = new Reporter({ + request: (url, options) => requests.push(Object.assign({url}, options)), + alwaysReport: true, + reportPreviousErrors: false, + resourcePath: repositoryRootPath + }) + let error = new Error() Error.captureStackTrace(error) reporter.reportUncaughtException(error) @@ -45,11 +59,58 @@ describe("Reporter", () => { expect(request.headers.get("Content-Type")).toBe("application/json") let body = JSON.parse(request.body) - // asserting the correct path is difficult on CI. let's do 'close enough'. - expect(body.events[0].exceptions[0].stacktrace[0].file).toMatch(/reporter-spec/) - expect(body.events[0].exceptions[0].stacktrace[0].file).not.toMatch(/\\/) - delete body.events[0].exceptions[0].stacktrace[0].file - delete body.events[0].exceptions[0].stacktrace[0].inProject + expect(body).toEqual({ + "apiKey": Reporter.API_KEY, + "notifier": { + "name": "Atom", + "version": Reporter.LIB_VERSION, + "url": "https://www.atom.io" + }, + "events": [ + { + "payloadVersion": "2", + "exceptions": [ + { + "errorClass": "Error", + "message": "", + "stacktrace": [ + { + "method": semver.gt(process.versions.electron, '1.6.0') ? 'Spec.it' : 'it', + "file": "spec/reporter-spec.js", + "lineNumber": lineNumber, + "columnNumber": columnNumber, + "inProject": true + } + ] + } + ], + "severity": "error", + "user": {}, + "app": { + "version": atom.getVersion(), + "releaseStage": getReleaseChannel(atom.getVersion()) + }, + "device": { + "osVersion": osVersion + } + } + ] + });}) + + it("posts errors originated outside Atom Core to BugSnag", () => { + fs.getHomeDirectory = () => path.join(__dirname, '..', '..') + + let error = new Error() + Error.captureStackTrace(error) + reporter.reportUncaughtException(error) + let [lineNumber, columnNumber] = error.stack.match(/.js:(\d+):(\d+)/).slice(1).map(s => parseInt(s)) + + expect(requests.length).toBe(1) + let [request] = requests + expect(request.method).toBe("POST") + expect(request.url).toBe("https://notify.bugsnag.com") + expect(request.headers.get("Content-Type")).toBe("application/json") + let body = JSON.parse(request.body) expect(body).toEqual({ "apiKey": Reporter.API_KEY, @@ -68,8 +129,10 @@ describe("Reporter", () => { "stacktrace": [ { "method": semver.gt(process.versions.electron, '1.6.0') ? 'Spec.it' : 'it', + "file": '~/exception-reporting/spec/reporter-spec.js', "lineNumber": lineNumber, - "columnNumber": columnNumber + "columnNumber": columnNumber, + "inProject": true } ] } @@ -210,6 +273,8 @@ describe("Reporter", () => { describe(".reportFailedAssertion(error)", () => { it("posts warnings to bugsnag", () => { + fs.getHomeDirectory = () => path.join(__dirname, '..', '..') + let error = new Error() Error.captureStackTrace(error) reporter.reportFailedAssertion(error) @@ -222,12 +287,6 @@ describe("Reporter", () => { expect(request.headers.get("Content-Type")).toBe("application/json") let body = JSON.parse(request.body) - // asserting the correct path is difficult on CI. let's do 'close enough'. - expect(body.events[0].exceptions[0].stacktrace[0].file).toMatch(/reporter-spec/) - expect(body.events[0].exceptions[0].stacktrace[0].file).not.toMatch(/\\/) - delete body.events[0].exceptions[0].stacktrace[0].file - delete body.events[0].exceptions[0].stacktrace[0].inProject - expect(body).toEqual({ "apiKey": Reporter.API_KEY, "notifier": { @@ -245,8 +304,10 @@ describe("Reporter", () => { "stacktrace": [ { "method": semver.gt(process.versions.electron, '1.6.0') ? 'Spec.it' : 'it', + "file": '~/exception-reporting/spec/reporter-spec.js', "lineNumber": lineNumber, - "columnNumber": columnNumber + "columnNumber": columnNumber, + "inProject": true } ] } @@ -405,7 +466,6 @@ describe("Reporter", () => { const lastRequest = requests[requests.length - 1] const body = JSON.parse(lastRequest.body) - console.log(body); expect(body.events[0].metaData.previousErrors).toEqual(['A', 'B']) expect(body.events[0].metaData.previousAssertionFailures).toEqual(['X', 'Y']) })