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

Error codes or types to make it easier to handle errors #189

Closed
bennycode opened this issue Mar 9, 2016 · 6 comments
Closed

Error codes or types to make it easier to handle errors #189

bennycode opened this issue Mar 9, 2016 · 6 comments

Comments

@bennycode
Copy link
Contributor

Hello, I am using Dexie in my web application and I like it really much.

But I saw today that errors in the code are thrown like this new Error("Cannot add version when database is open"). So if I want to use a general error handler which deals differently with some error types, then I have to compare the strings of the error.message like:

db.on('error', function (error) {
  switch (error.message) {
    case "Cannot add version when database is open":
        ...
        break;
    case "A mutation operation was attempted on a database that did not allow mutations.":
        ...
        break;
  }
});

But if error message strings change, then I have to change my logic again. So it would be nice if there where error codes so I could do things like this:

db.on('error', function (error) {
  switch (error.code) {
    case 4218:
        ...
        break;
    case 4219:
        ...
        break;
  }
});

Another approach would be to expose the error descriptions as prototype constants so that they can be checked like this:

db.on('error', function (error) {
  switch (error.message) {
    case Dexie.errors.DatabaseError.prototype.LOCK_ERROR:
        ...
        break;
    case Dexie.errors.DatabaseError.prototype.MUTATION_ERROR:
        ...
        break;
  }
});

Option number 3 would be to have error instances:

db.on('error', function (error) {
  if (error instanceof Dexie.errors.LockError) {
    ...
  } else if (error instanceof Dexie.errors.MutationError) {
    ...
  }
}

What do you think about it?

@dfahlander
Copy link
Collaborator

I get the point. There are various strategies to deal with; error codes, different error types (like java/c#/c++) or as DOMError solves it, e.name and e.code for a canonical identity of the error.

However, the caller need to be prepared for any type of error that callback code may throw, as well as DOMError from the indexedDB engine. The error you get might not have a "message" member - it can be a plain string for example - if another lib you are using from your Dexie callback throws a string during a db operation or transaction.

Just a note: It's better to subscribe to Dexie.Promise.on.error() instead of db.on.error() in order to catch all errors that may occur in your application while in a Promise callback. db.on.error will only get errors occurring within a db operation.

The most straight-forward way I could think of, would be to have specific error types for different errors. That way your if..else code could be managed to support DOMError as well. But with DOMError you would have to switch on error.code or error.name to get the real error.

So, the best way I would think of to solve this would be to introduce some error types. We already have Dexie.ModifyError. Could add a few others.

So in the end, you'd get a if..else sequence where you may put those errors you want to handle specifically. Or if catching the promise, you could do

db.operation().catch(Dexie.ModifyError, e => {
    // Handle Dexie.ModifyError
}).catch(DOMError, e => {
    // Handle DOMError
}).catch(Error, e => {
    // Handle all other error objects
}).catch(e => {
    // Handle custom errors and strings.
});

dfahlander added a commit that referenced this issue Mar 11, 2016
…f error to satisfy #189.

Should update wiki with what types of errors that may be catched.
@bennycode
Copy link
Contributor Author

Thanks for taking the time and providing a pull request. I also like the approach of having specific error types the most since you already started with Dexie.ModifyError. The Promise chain you provided looks also very clean to me.

By the way, we are using Dexie.js in a messaging application called Wire. You can see it in action here: Wire for Web. 😎

@dfahlander
Copy link
Collaborator

Everthing should be done now and typescript defintions are updated.

In principal, all error classes are accessable on the Dexie constructor, for example Dexie.AbortError, Dexie.ModifyError, etc.

Also, all DOMExceptions and DOMErrors that were previously catched and converted to plain Error instances due to easier debugging of the stack, are now converted to their corresponding DexieError class with the same name as the name property of the DOMException or DOMError from indexedDB.

So there are two alternate ways of knowing the type of error:

  1. Using promise.catch(ErrorConstructor, callback),
  2. Switching on error.name

See documentation of https://github.com/dfahlander/Dexie.js/wiki/DexieError where you can view samples on the sub classed errors. The documentation is still incomplete, but https://github.com/dfahlander/Dexie.js/wiki/Dexie.UpgradeError one with documented samples.

@bennycode
Copy link
Contributor Author

Follow-Up Question

I was catching errors on db.open like this:

1st

db.open()
.then(function() {
  console.log('Everything is cool.');
})
.catch(function (error) {
  console.warn('Danger: ' + error.message);
});

Now (with Dexie.js v1.3.4) I tried this:

2nd

db.on('error', function(error) {
  switch (error.name) {
    case Dexie.errnames.Upgrade:
      console.warn('Upgrade error');
    default:
      console.warn('Danger: ' + error.message);
  }
});

db.open()
.then(function() {
  console.log('Everything is cool.');
});

I was expecting that errors occuring on db.open will populte to db.on('error', ...) but it looks like they don't. I am hitting a "Dexie specification of currently installed DB version is missing" error which I can catch with the first code snippet but not with the second.

@dfahlander
Copy link
Collaborator

Must be an issue with db.on('error') that not all errors are shown there.

Should fix it. But as an advice to you, I think you should rely more on Dexie.Promise.on('error') because it catches all uncaught errors, even those that is not DB related. For example, if you are have an error that is not directly cancelling a transaction (because the error happens in the then-block of a committed transaction), and you've forgotten to catch that chain. Dexie.Promise.on('error') will get it, but not db.on('error') because its not directly related to a DB operation failing. The issue you found (UpgradeError) SHOULD have been signaled to db.on('error') though, so that's still a bug.

Both db.on('error') and Dexie.Promise.on('error') should be signaled only when a promise failed to be catched. But the latter is signaled also when the error wouldn't affect a database operation, so it's better to use that one.

Either I will deprecate db.on('error'), or take some action to find out for sure that it correctly signals all issues. But if deprecating it, I'll first need to check for sure that it really has no valid use case first (now that we have Dexie.Promise.on('error').

@bennycode
Copy link
Contributor Author

Dexie.Promise.on('error') catches it. Thanks! 😃

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

No branches or pull requests

2 participants