Skip to content
This repository has been archived by the owner on Mar 3, 2023. It is now read-only.

Recreate #15175 #22541

Merged
merged 5 commits into from Jun 7, 2021
Merged

Recreate #15175 #22541

merged 5 commits into from Jun 7, 2021

Conversation

icecream17
Copy link
Contributor

@icecream17 icecream17 commented Jun 4, 2021

See #15175
Resolves #15172 (although it's already closed anyways)

Copy link
Contributor

@sadick254 sadick254 left a comment

Choose a reason for hiding this comment

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

The changes look good. Do we know what events that lead to the error?

@icecream17
Copy link
Contributor Author

icecream17 commented Jun 4, 2021

From what I gather about #15172 we don't know why the error happened. This just adds a console.error handler

@sadick254
Copy link
Contributor

I think a good approach would be to display the error as a notification. This way we can have visibility when the error occurs. A user will be able to create an issue from the notification modal. See https://flight-manual.atom.io/api/v1.57.0/NotificationManager/

I'm assuming that `addError` displays an error.

I'm also logging the error on the console - for theoretical debugging in case the notification is dismissed - but maybe it's unnecessary.
I renamed `event` to `error` in the `onupgradeneeded` handler
@icecream17
Copy link
Contributor Author

Is the console.error unnecessary now that there's a notification?

@sadick254
Copy link
Contributor

The console.error is fine.

@UziTech
Copy link
Contributor

UziTech commented Jun 4, 2021

I think addFatalError might be better so the button to easily create an issue appears.

Copy link
Contributor

@sadick254 sadick254 left a comment

Choose a reason for hiding this comment

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

I agree with @UziTech

src/state-store.js Outdated Show resolved Hide resolved
src/state-store.js Outdated Show resolved Hide resolved
icecream17 and others added 2 commits June 4, 2021 13:15
Co-authored-by: Sadick <sadickjunior@gmail.com>
Co-authored-by: Sadick <sadickjunior@gmail.com>
@sadick254 sadick254 merged commit 83ffb7a into atom:master Jun 7, 2021
@icecream17 icecream17 deleted the patch-1 branch June 7, 2021 12:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uncaught TypeError: Cannot read property 'createObjectStore' of undefined
3 participants