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

Better error handling #21

Closed
mimol91 opened this issue Dec 2, 2016 · 12 comments
Closed

Better error handling #21

mimol91 opened this issue Dec 2, 2016 · 12 comments

Comments

@mimol91
Copy link
Contributor

mimol91 commented Dec 2, 2016

I am using traverson for a while.
Recently I've noticed that there is no way to detect if error during request is from traverson or traverson-hal. Moreover it returns only plain message, without any error type.
Like in https://github.com/basti1302/traverson-hal/blob/master/index.js#L64
I need to know if its a linkError or embeddedError (For example link to resource is not in server response)
Currently I can only parse strings, but it would be much more convenient to get error type, either by
create own Error types,
pass to Error object instead string
or just add ctx as 2nd argument of Error

@basti1302
Copy link
Member

basti1302 commented Dec 5, 2016

This sounds like a good idea. However, I'd like to really understand your use case and the requirements here. Could you paste a code snippet of the part where you are using traverson/traverson-hal. You probably have some kind of if statement that does different things depending on the kind of error, I would like to see that, if possible.

@mimol91
Copy link
Contributor Author

mimol91 commented Dec 5, 2016

Hey.
Thanks for response.

It is taken form Traverson Readme

  traverson
    .from('http://api.io')
    .jsonHal()
    .follow('ht:me', 'ht:posts')
    .getResource(function(error, document) {
      if (error) {
        console.error(error.name)
      }
    });

I can get error type - which is JSONError
https://github.com/basti1302/traverson/blob/0fd4f349e2bf007ea8440284e3e1e252f08e7ae1/lib/transforms/parse.js - so its really great.

I've also tried

  traverson
    .from('http://haltalk.herokuapp.com/')
    .jsonHal()
    .follow('ht:me')
    .getResource(function(error, document) {
      if (error) {
        console.log(error.name);
      }
    });

error.name - HTTPError, becauses erver status code = 500

The problem is with

  traverson
    .from('http://api.opensupporter.org/api/v1/')
    .jsonHal()
    .follow('self', 'invalid_link')
    .getResource(function(error, document) {
      if (error) {
        console.log(error.name);
      }
    });

error.name is just 'error' https://github.com/basti1302/traverson-hal/blob/master/index.js#L64

I would like to know what type of error it is. (like linkMissing), and some other errors for ctx.linkError or ctx.embeddedError.
I need to be able to detect if there is no link which I want to traverse (invalid_link) in server response

@basti1302
Copy link
Member

Thanks for the code snippets. That's not exactly what I meant, though. What I am interested in is what you are going to do, depending on various error types. How does you code handle these different errors? You surely do not want to differentiate between them just to call console.log right?

@mimol91
Copy link
Contributor Author

mimol91 commented Dec 6, 2016

Sure.
Basically I want to distinguish at least to types:

  • When server status code >= 300 I want to display modal which says that something goes wrong and user need to refresh page
  • If link is missing, I need to catch it and set result as empty array (I m using redux, and API do not return link if collection is empty)

@mimol91
Copy link
Contributor Author

mimol91 commented Dec 13, 2016

@basti1302 Should I prepare PR for that?

@basti1302
Copy link
Member

Yes, that would be welcome!

@mimol91
Copy link
Contributor Author

mimol91 commented Dec 15, 2016

@basti1302
Could you do CodeReview for #22 ?

@basti1302
Copy link
Member

Sure, I'll probably can get it done tomorrow. A little more patience :)

@mimol91
Copy link
Contributor Author

mimol91 commented Dec 19, 2016

Thanks for merging.
I will close this issue,

@mimol91 mimol91 closed this as completed Dec 19, 2016
@basti1302 basti1302 mentioned this issue Dec 20, 2016
basti1302 added a commit that referenced this issue Dec 20, 2016
@basti1302
Copy link
Member

This has been released with version 5.0.0.

@basti1302
Copy link
Member

basti1302 commented Dec 20, 2016

From the release notes, for reference: All Error objects created by Traverson and traverson-hal now have the name property set, see Traverson API docs on error names and traverson-hal docs on error names. (#21 and #22, thanks to @mimol91)

@basti1302
Copy link
Member

Btw, thanks for the PR over in #22 and bringing the issue up in general. After merging your changes I extended them to the whole codebase. You might want to check out this commit in traverson-hal and this commit in traverson to see if it still fits your requirements (it should, I think).

Also, this comment might be relevant for your use case (depends on your hal documents).

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