Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

By default, log to console.error if any Promise was uncaught. #206

Merged
merged 3 commits into from
Mar 23, 2016

Conversation

dfahlander
Copy link
Collaborator

Before, it was nescessary to always explicitely add a line like this somewhere in your bootstrapping code, or you could have issues debugging your code:

Dexie.Promise.on('error', e => console.error(e))

But with this change, you don't need to do that anymore unless you'd like to show those errors somewhere else than in the console. Dexie.Promise will by default log any uncaught promise rejection. An exception from this rule is when you do stuff in a transaction scope. You never have to catch those operations but can instead catch the Promise that results from the transaction.

When writing this change, I noticed that Promise.on('error') was actually logging too much. Even though I was catching errors, the event subscriber would sometimes be triggered anyway. So this issue had to be dealt with, resulting in some changes in the Promise class and also in Dexie's transaction method.

Another change from before is that there can only be one subscriber to Promise.on('error'). So if you subscribe to it, the default subscriber wont log anymore, but if you unsubscribe, the default handler will start logging again. If another subscriber was present, also that subscriber will not be triggered anymore until you unsubscribe. This is reasonable because if you subscribe to that error event, the error should not be considered "uncaught" anymore since you now have a chance to handle it.

…ery uncaught Promise. Will hopefully help out debugging and not nescessary to use Promise.on('error').

Bugfix: Promise.on('error') gave false alarm (errors that were catched were also signaled to that event).
@dfahlander dfahlander merged commit c7e72e4 into master Mar 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant