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

pass the original error message to the catched error #450

Closed
timaschew opened this issue Sep 10, 2014 · 8 comments
Closed

pass the original error message to the catched error #450

timaschew opened this issue Sep 10, 2014 · 8 comments

Comments

@timaschew
Copy link

superagent is catching all errors: https://github.com/visionmedia/superagent/blob/f106c7226758a22f633d0cf1607b5bb6a3ce264e/lib/client.js#L458

so if you make a request and get a runtime error in the response callback it's totally annoying, because you only get a Parser is unable to parse the response and you need to debug it to get the origin error message, which has nothing to do with parsing the response.

@timaschew
Copy link
Author

@gjohnson
why the try block wraps the callback as well?
Isn't it enough to catch just the var res = new Response(self); ?

https://github.com/visionmedia/superagent/blob/f106c7226758a22f633d0cf1607b5bb6a3ce264e/lib/client.js#L454

@timaschew
Copy link
Author

maybe something like that:

  this.on('end', function(){
    var res;
    try {
     res = new Response(self);
    } catch(e) {
      var err = new Error('Parser is unable to parse the response');
      err.parse = true;
      err.original = e;
      self.callback(err);
    }
    if ('HEAD' == method) res.text = null;
    try {
      self.callback(null, res);
    } catch(e) {
      self.callback(e);
    }
  });
}

@joanniclaborde
Copy link

+1

@dcousens
Copy link

dcousens commented Jan 8, 2015

+1

1 similar comment
@constantx
Copy link

👍

@defunctzombie
Copy link
Contributor

This has been fixed in master. The callback is not in the try/catch anymore.

@dcousens
Copy link

Any word on a publish?
On 22 Feb 2015 6:01 pm, "Roman Shtylman" notifications@github.com wrote:

Closed #450 #450.


Reply to this email directly or view it on GitHub
#450 (comment).

@defunctzombie
Copy link
Contributor

Yea, I am trying to blow through the issue backlog today and tomorrow and want to try to get an RC out tomorrow but can't promise anything.

If you want to use master today you can certainly just reference git commits directly.

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

5 participants