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

Implement error object #5

Open
ssured opened this issue Aug 29, 2014 · 7 comments
Open

Implement error object #5

ssured opened this issue Aug 29, 2014 · 7 comments

Comments

@ssured
Copy link
Contributor

ssured commented Aug 29, 2014

The current solution when not finding a record relies on parsing the Error.message to find out which type+id failed to load. Parsing Errors is not a good practise.
My solution would be to introduce an EmberPouchError object, which has a documented api. Ember Data has something similar implemented for validation, see https://github.com/emberjs/data/blob/689b2fa05a1137303d7742faa0d74da2eb9abba5/packages/ember-data/lib/system/adapter.js#L7-L66.

This code would then be possible:

find(...).catch(function(err){
  if (err instanceof EmberPouchError) {
    switch (err.message) {
      case 'Not found':
        console.log('obj not found, type=', err.type, ' id=', err.id);
        return something sane here;
    }
  }
  throw err
})
@nolanlawson
Copy link
Member

I think it's time to check with the ED folks to see what the best approach is here. I agree that returning a helpful error object would be best, but I want to make sure I choose a pattern that they're also choosing.

@nolanlawson
Copy link
Member

In the #emberjs channel I was informed that there's no real official way to do this, so yeah, we can do this however we like.

However, I think it would be dangerous to do something like what you describe, because then you'd write your code to some very specific ember-pouch implementation details, which may be different from what you'd get with the RESTAdapter. If there's no official way I'm supposed to be returning 404 errors, then I suspect the Ember devs do not want you to try parsing errors like this.

Is there any reason you really need to parse the error? I mean:

store.find('foo', 'bar').catch(function (err){
  // this is probably a 404. and we know foo and bar
});

@ssured
Copy link
Contributor Author

ssured commented Aug 29, 2014

I think knowing more of the error is needed in the end. One scenario would be where someone uses Pouch to talk to a Couch, and the internet connection goes down. That's really something different than a 404 error.

I think handling these errors is up to the developer, the libraries can only communicate them. If so, it does not matter where the error originates. Ember Data is flawed in error handling, so till that is fixed I think it is fine to have a custom error handler. Ember Pouch 2.0 will hopefully rely on (an extended) ED error handling system.

The implementation details of EmberPouchErrors should be public API for this lib, then we are fine. Saying the find promise can be rejected with a finite set of pouchdb related errors seems ok. Some of these errors will contain more specific information on the error.

The design of the Error system in PouchDB is nice too, where all default errors are subclasses of the general EmberPouchError object is a nicer implementation strategy. Then we would throw a EmberPouchErrorNotFound.

My JS foo is not good enough to make a final call on this topic. I am interested in helping this lib some further though.

@nolanlawson
Copy link
Member

Yeah, I would really rather kick the can on this until ED gives us clear marching orders. Otherwise you will write your code to whatever custom EmberPouchError we write, and then ED will change it later.

If even the RESTAdapter does not have a solution to the problem of "I want to distinguish between 404s and random network errors", then I really have no idea how we should be doing it here.

Leaving this open, though, since it's an important discussion and I want to fix it eventually.

@tchak
Copy link

tchak commented Dec 7, 2014

I am working right now to fix ember-data errors handling story. I hope to bring some good news soon.

@nolanlawson
Copy link
Member

Cool, thanks for the update! :) Excited to see ember-data grow with the ecosystem.

@ear
Copy link

ear commented Mar 20, 2016

Hi! What is the status on this?

The story seems straightened up beautifully in Ember-data. E.g. saves that result in error are meant to be 422 responses with a particular shape, etc.

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

4 participants