Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Fix some package detections #100

Merged
merged 8 commits into from
Nov 18, 2015
Merged

Fix some package detections #100

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9619aa6
:bug: Correctly determine the package for "Failed to load/activate" e…
50Wliu Nov 3, 2015
4054914
:bug: :checkered_flag: Fix linked packages detection on Windows
50Wliu Nov 3, 2015
effa7b2
nof
50Wliu Nov 3, 2015
e16f91c
load --> enable
50Wliu Nov 3, 2015
ed1f3d4
Also catch errors on grammar load
50Wliu Nov 3, 2015
7db06a6
Use package name metadata if available
50Wliu Nov 4, 2015
7e5cec3
:art move packageName to the top
50Wliu Nov 6, 2015
07454de
Use a real-path in the spec's fake stack trace
maxbrunsfeld Nov 10, 2015
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
6 changes: 4 additions & 2 deletions lib/notification-issue.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,11 @@ class NotificationIssue

getPackageName: ->
options = @notification.getOptions()
return unless options.stack? or options.detail?
return unless options.stack? or options.detail? or options.packageName?

packagePaths = @getPackagePathsByPackageName()
for packageName, packagePath of packagePaths
if packagePath.indexOf('.atom/dev/packages') > -1 or packagePath.indexOf('.atom/packages') > -1
if packagePath.indexOf(path.join('.atom', 'dev', 'packages')) > -1 or packagePath.indexOf(path.join('.atom', 'packages')) > -1
Copy link
Contributor

Choose a reason for hiding this comment

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

oh man. Cant believe this has been in there so long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this still might be incorrect...@kevinsawicki if someone sets their ATOM_HOME to something like /Users/user/ATOM, then will this check fail? If so, how would I fix that? By reading process.env.ATOM_HOME and then regexing it to capture the last folder path?

Copy link
Contributor

Choose a reason for hiding this comment

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

Gentle bump @kevinsawicki

Copy link
Contributor

Choose a reason for hiding this comment

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

The goal of this line is to check if the package is bundled or not right?

There is atom.packages.isBundledPackage and atom.packages.isBundledPackagePath that might be useful to use instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's actually to check if a package is symlinked or not, and then to get its true path since that's where the error will have been thrown from.

Eg if I have notifications linked to .atom\packages\notifications from C:\Users\user\Documents\Github\notifications and notifications throws an error, then this piece of code will correctly match the two.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh yeah, I'd say maybe just call fs.realpath in all cases then if we want to ensure the stack paths match the package paths.

packagePaths[packageName] = fs.realpathSync(packagePath)

getPackageName = (filePath) =>
Expand All @@ -190,6 +190,8 @@ class NotificationIssue
return packName unless /^\.\./.test(relativePath)
@getPackageNameFromFilePath(filePath)

return options.packageName if options.packageName?
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this could be returned at the top of this method? Like line 178?


if options.detail? and packageName = getPackageName(options.detail)
return packageName

Expand Down
159 changes: 159 additions & 0 deletions spec/notifications-spec.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,50 @@ describe "Notifications", ->
expect(issueBody).toContain '"notifications":'
expect(issueBody).not.toContain '"editor":'

describe "when an exception is thrown from a linked package", ->
beforeEach ->
spyOn(atom, 'inDevMode').andReturn false
generateFakeAjaxResponses()

packagesDir = path.join(temp.mkdirSync('atom-packages-'), '.atom', 'packages')
atom.packages.packageDirPaths.push(packagesDir)
packageDir = path.join(packagesDir, '..', '..', 'github', 'linked-package')
fs.makeTreeSync path.dirname(path.join(packagesDir, 'linked-package'))
fs.symlinkSync(packageDir, path.join(packagesDir, 'linked-package'), 'junction')
fs.writeFileSync path.join(packageDir, 'package.json'), """
{
"name": "linked-package",
"version": "1.0.0",
"repository": "https://github.com/atom/notifications"
}
"""
atom.packages.enablePackage('linked-package')

stack = """
ReferenceError: path is not defined
at Object.module.exports.LinkedPackage.wow (#{path.join(packageDir, 'linked-package.coffee')}:29:15)
at atom-workspace.subscriptions.add.atom.commands.add.linked-package:wow (#{path.join(packageDir, 'linked-package.coffee')}:18:102)
at CommandRegistry.module.exports.CommandRegistry.handleCommandEvent (/Applications/Atom.app/Contents/Resources/app/src/command-registry.js:238:29)
at /Applications/Atom.app/Contents/Resources/app/src/command-registry.js:3:61
at CommandPaletteView.module.exports.CommandPaletteView.confirmed (/Applications/Atom.app/Contents/Resources/app/node_modules/command-palette/lib/command-palette-view.js:159:32)
"""
detail = "At #{path.join(packageDir, 'linked-package.coffee')}:41"
message = "Uncaught ReferenceError: path is not defined"
atom.notifications.addFatalError(message, {stack, detail, dismissable: true})
notificationContainer = workspaceElement.querySelector('atom-notifications')
fatalError = notificationContainer.querySelector('atom-notification.fatal')

it "displays a fatal error with the package name in the error", ->
waitsForPromise ->
fatalError.getRenderPromise()

runs ->
expect(notificationContainer.childNodes.length).toBe 1
expect(fatalError).toHaveClass 'has-close'
expect(fatalError.innerHTML).toContain "Uncaught ReferenceError: path is not defined"
expect(fatalError.innerHTML).toContain "<a href=\"https://github.com/atom/notifications\">linked-package package</a>"
expect(fatalError.issue.getPackageName()).toBe 'linked-package'

describe "when an exception is thrown from an unloaded package", ->
beforeEach ->
spyOn(atom, 'inDevMode').andReturn false
Expand Down Expand Up @@ -370,6 +414,121 @@ describe "Notifications", ->
expect(fatalError.innerHTML).toContain "<a href=\"https://github.com/atom/notifications\">unloaded package</a>"
expect(fatalError.issue.getPackageName()).toBe 'unloaded'

describe "when an exception is thrown from a package trying to load", ->
beforeEach ->
spyOn(atom, 'inDevMode').andReturn false
generateFakeAjaxResponses()

packagesDir = temp.mkdirSync('atom-packages-')
atom.packages.packageDirPaths.push(path.join(packagesDir, '.atom', 'packages'))
packageDir = path.join(packagesDir, '.atom', 'packages', 'broken-load')
fs.writeFileSync path.join(packageDir, 'package.json'), """
{
"name": "broken-load",
"version": "1.0.0",
"repository": "https://github.com/atom/notifications"
}
"""

stack = "TypeError: Cannot read property 'prototype' of undefined\n at __extends (<anonymous>:1:1)\n at Object.defineProperty.value [as .coffee] (/Applications/Atom.app/Contents/Resources/app.asar/src/compile-cache.js:169:21)"
detail = "TypeError: Cannot read property 'prototype' of undefined"
message = "Failed to load the broken-load package"
atom.notifications.addFatalError(message, {stack, detail, packageName: 'broken-load', dismissable: true})
notificationContainer = workspaceElement.querySelector('atom-notifications')
fatalError = notificationContainer.querySelector('atom-notification.fatal')

it "displays a fatal error with the package name in the error", ->
waitsForPromise ->
fatalError.getRenderPromise()

runs ->
expect(notificationContainer.childNodes.length).toBe 1
expect(fatalError).toHaveClass 'has-close'
expect(fatalError.innerHTML).toContain "TypeError: Cannot read property 'prototype' of undefined"
expect(fatalError.innerHTML).toContain "<a href=\"https://github.com/atom/notifications\">broken-load package</a>"
expect(fatalError.issue.getPackageName()).toBe 'broken-load'

describe "when an exception is thrown from a package trying to load a grammar", ->
beforeEach ->
spyOn(atom, 'inDevMode').andReturn false
generateFakeAjaxResponses()

packagesDir = temp.mkdirSync('atom-packages-')
atom.packages.packageDirPaths.push(path.join(packagesDir, '.atom', 'packages'))
packageDir = path.join(packagesDir, '.atom', 'packages', 'language-broken-grammar')
fs.writeFileSync path.join(packageDir, 'package.json'), """
{
"name": "language-broken-grammar",
"version": "1.0.0",
"repository": "https://github.com/atom/notifications"
}
"""

stack = """
Unexpected string
at nodeTransforms.Literal (/usr/share/atom/resources/app/node_modules/season/node_modules/cson-parser/lib/parse.js:100:15)
at #{path.join('packageDir', 'grammars', 'broken-grammar.cson')}:1:1
"""
detail = """
At Syntax error on line 241, column 18: evalmachine.<anonymous>:1
"#\\{" "end": "\\}"
^^^^^
Unexpected string in #{path.join('packageDir', 'grammars', 'broken-grammar.cson')}

SyntaxError: Syntax error on line 241, column 18: evalmachine.<anonymous>:1
"#\\{" "end": "\\}"
^^^^^
"""
message = "Failed to load a language-broken-grammar package grammar"
atom.notifications.addFatalError(message, {stack, detail, packageName: 'language-broken-grammar', dismissable: true})
notificationContainer = workspaceElement.querySelector('atom-notifications')
fatalError = notificationContainer.querySelector('atom-notification.fatal')

it "displays a fatal error with the package name in the error", ->
waitsForPromise ->
fatalError.getRenderPromise()

runs ->
expect(notificationContainer.childNodes.length).toBe 1
expect(fatalError).toHaveClass 'has-close'
expect(fatalError.innerHTML).toContain "Failed to load a language-broken-grammar package grammar"
expect(fatalError.innerHTML).toContain "<a href=\"https://github.com/atom/notifications\">language-broken-grammar package</a>"
expect(fatalError.issue.getPackageName()).toBe 'language-broken-grammar'

describe "when an exception is thrown from a package trying to activate", ->
beforeEach ->
spyOn(atom, 'inDevMode').andReturn false
generateFakeAjaxResponses()

packagesDir = temp.mkdirSync('atom-packages-')
atom.packages.packageDirPaths.push(path.join(packagesDir, '.atom', 'packages'))
packageDir = path.join(packagesDir, '.atom', 'packages', 'broken-activation')
fs.writeFileSync path.join(packageDir, 'package.json'), """
{
"name": "broken-activation",
"version": "1.0.0",
"repository": "https://github.com/atom/notifications"
}
"""

stack = "TypeError: Cannot read property 'command' of undefined\n at Object.module.exports.activate (<anonymous>:7:23)\n at Package.module.exports.Package.activateNow (/Applications/Atom.app/Contents/Resources/app.asar/src/package.js:232:19)"
detail = "TypeError: Cannot read property 'command' of undefined"
message = "Failed to activate the broken-activation package"
atom.notifications.addFatalError(message, {stack, detail, packageName: 'broken-activation', dismissable: true})
notificationContainer = workspaceElement.querySelector('atom-notifications')
fatalError = notificationContainer.querySelector('atom-notification.fatal')

it "displays a fatal error with the package name in the error", ->
waitsForPromise ->
fatalError.getRenderPromise()

runs ->
expect(notificationContainer.childNodes.length).toBe 1
expect(fatalError).toHaveClass 'has-close'
expect(fatalError.innerHTML).toContain "TypeError: Cannot read property 'command' of undefined"
expect(fatalError.innerHTML).toContain "<a href=\"https://github.com/atom/notifications\">broken-activation package</a>"
expect(fatalError.issue.getPackageName()).toBe 'broken-activation'

describe "when an exception is thrown from a package without a trace, but with a URL", ->
beforeEach ->
issueBody = null
Expand Down