Better errors #4906
Better errors #4906
Conversation
|
Love this 👍 |
|
There are so many places we throw sketch app-killing errors. |
| @@ -282,7 +287,9 @@ class Project extends Model | |||
| # Returns a promise that resolves to the {TextBuffer}. | |||
| buildBuffer: (absoluteFilePath) -> | |||
| if fs.getSizeSync(absoluteFilePath) >= 2 * 1048576 # 2MB | |||
| throw new Error("Atom can only handle files < 2MB for now.") | |||
| error = new Error("Atom can only handle files < 2MB for now.") | |||
| error.name = 'OversizeFileError' | |||
There was a problem hiding this comment.
What do you think about just using a custom code instead? To be consistent with other IO errors from node.
This could be be like error.code = 'ETOOLARGETOOPEN' or something.
benogle
force-pushed
the
bo-better-errors
branch
from
eed8ab3
to
7972032
Compare
| item ?= atom.project.open(uri, options) | ||
| catch error | ||
| if error.code is 'EFILETOOLARGE' | ||
| atom.notifications.addWarning(error.message + " Large file support is being tracked at [atom/atom#307](https://github.com/atom/atom/issues/307).") |
There was a problem hiding this comment.
💄 "#{error.message} Large file support..."
benogle
force-pushed
the
bo-better-errors
branch
2 times, most recently
from
265cb7c
to
4b64db8
Compare
benogle
force-pushed
the
bo-better-errors
branch
2 times, most recently
from
c1e7ca8
to
43b06f7
Compare
| @@ -832,25 +832,29 @@ class Config | |||
| @configFileHasErrors = false | |||
| catch error | |||
| @configFileHasErrors = true | |||
| @notifyFailure('Failed to load config.cson', error) | |||
| if error.location? | |||
| @notifyFailure('Failed to load config.cson', error.stack) | |||
There was a problem hiding this comment.
This might be config.json so you may want to use path.basename(@getUserConfigPath())
benogle
force-pushed
the
bo-better-errors
branch
from
7aff289
to
effb672
Compare
|
Ok, I think this is ready to go. @kevinsawicki another quick pass would be rad |
|
Just a heads up, you might want to rebase, doesn't look like this will merge cleanly currently. |
|
|
||
| KeymapManager::subscribeToFileReadFailure = -> | ||
| this.onDidFailToReadFile (error) -> | ||
| atom.notifications.addError('Failed to load keymap.cson', {detail: error.stack, dismissable: true}) | ||
| this.onDidFailToReadFile (error) => |
There was a problem hiding this comment.
@ instead of this? I see it was using this before, but might be nice to change it now since it is being edited.
benogle
force-pushed
the
bo-better-errors
branch
from
c455f3e
to
d67216b
Compare
They work for me!
Use the computed path as well
benogle
force-pushed
the
bo-better-errors
branch
from
d67216b
to
5cb18fc
Compare
TODO
unable to watch patherrors for config, keymap, user scripts on startup