Giter Site home page Giter Site logo

unhandled 'error' event in node about axios HOT 16 CLOSED

axios avatar axios commented on March 28, 2024
unhandled 'error' event in node

from axios.

Comments (16)

mzabriskie avatar mzabriskie commented on March 28, 2024

@jergason good catch. I can add the error handler. Not sure how the error fits in the reject either.

A couple options:

  1. Add an error property to the response object
  2. Just provide the error object directly to reject

In either case you're going to have to check if an error exists:

// option 1
.catch(function (res) {
  if (!res.data && res.error) {
    console.log('error', res.error);
  } else {
    console.log('it worked!');
  }
});

// option 2
.catch(function (res) {
  if (res instanceof Error) {
    console.log('error', res);
  } else {
    console.log('it worked!');
  }
});

from axios.

mzabriskie avatar mzabriskie commented on March 28, 2024

I keep stewing on this, but still don't love either option, and haven't come up with anything else better. Stay tuned...

from axios.

kentcdodds avatar kentcdodds commented on March 28, 2024

Maybe I'm misunderstanding things, but if they're using .catch then the request didn't "work" and there shouldn't be data correct? I mean the response can come back with the reason for the error, but that could be considered the error right? So res will always be an error, right?

from axios.

ryanflorence avatar ryanflorence commented on March 28, 2024

I would expect catch to reject with an error object.

from axios.

jmdobry avatar jmdobry commented on March 28, 2024

The issue is combing thrown errors and an http response into a single argument.

request.js example:

var request = require('request');
request('http://www.google.com', function (error, response, body) {
  if (!error && response.statusCode == 200) {
    console.log(body) // Print the google web page.
  }
})

In this example it's obvious which argument would be an Error object, caused by some exception in the code execution.

The response argument can be inspected separately, as it's status code might represent an error from a different domain, unrelated to our code's execution.

With axios:

.catch(function (res) {
  // what is res?
  // was there an exception? 
  // can I print a stack trace?
  // or do I just need to inspect the status code for 500, etc.?
});

The question is whether res will be the first or second (or both) argument from the first example. An Error object makes sense if an exception occurred, with a stack trace captured. But like @jergason said, it doesn't fit with {data, status, headers, config} which corresponds to the response argument in the first example.

A few options:

  • Always pass an Error object and requiring the developer to know how to inspect it for exception/stacktrace vs response meta data
  • Pass either an Error object or the response meta data, allowing the dev just check for which one
  • (An extension of #1) Create custom error class(es) (inheriting from Error) and expose them on axios' API. Example below:
.catch(function (err) {
  if (err instanceof axios.HttpError) {
    // determined by status code, no stack trace
    // handle non 2xx/3xx response
  } else if (err instanceof Error) {
    // caused by exception in code
    // handle error from code
  } else {
    // shouldn't get here?
  }
});

Another solution would be to follow how request.js does it (not how $http does it), as in don't decide for the developer that a particular http response should cause the promise to be rejected. In request.js, only exceptions in the code populate that error argument.

</spiel>

from axios.

kentcdodds avatar kentcdodds commented on March 28, 2024

Well explained @jmdobry... That last comment was interesting, but I think I'd recommend against going that direction for two reasons 1) you'd be forced to check the status code in your successful requests which would be annoying (especially in cases where you don't care if it was unsuccessful) 2) a lot of people use and love $http, so they'll expect axios to work similarly.

That second argument is a bit weak I think, but I thought I'd mention it anyway.

I think something like this would be reasonable:

.catch(function (result) {
  if (result.error) {
    // caused by exception in code
    // handle error from code
  } else {
    // determined by status code, no stack trace
    // handle non 2xx/3xx response
    console.log(result.data); // what came back from the server.
  }
});

from axios.

mzabriskie avatar mzabriskie commented on March 28, 2024

@jmdobry request's API doesn't apply to axios, since request is using a single callback to handle success/error. Versus using then/catch a la Promises. Also Promises limit then/catch to a single argument, so whether I like request's API or not, it can't be done the same way with axios.

I think I like just rejecting with the Error:

.catch(function (res) {
  if (res instanceof Error) {
    console.log(res.message);
  } else {
    console.log(res.status, res.data);
  }
});

from axios.

mzabriskie avatar mzabriskie commented on March 28, 2024

Actually it looks like a PR introduced this pattern already:

https://github.com/mzabriskie/axios/blob/master/lib/adapters/http.js#L35

Anyone have any heartburn over leaving it this way? I'd prefer not to change an API that people may already depend on in order to resolve this issue.

from axios.

kentcdodds avatar kentcdodds commented on March 28, 2024

I think that it's a totally reasonable/sensible solution 👍

from axios.

jmdobry avatar jmdobry commented on March 28, 2024

@mzabriskie Yes of course. I was just using request's api as a contrasting example to clearly define this issue by showing what axios can't do. I should have been clearer in my comment.

from axios.

ryanflorence avatar ryanflorence commented on March 28, 2024

I'm not doing as much catching up as I should, but the value sent to reject should contain more than just an error (I wasn't very clear before).

Very often you'll send a non 2xx status code but still send data. You also may need the headers, etc.

I don't think you can just throw an error into the arg and call it good, there's a lot of information about the response you may need in the handler.

from axios.

kentcdodds avatar kentcdodds commented on March 28, 2024

I think that's what @mzabriskie is suggesting with:

.catch(function (res) {
  if (res instanceof Error) {
    console.log(res.message); // this was a js error
  } else {
    console.log(res.status, res.data); // this was a non 2xx or 3xx response status
  }
});

from axios.

ryanflorence avatar ryanflorence commented on March 28, 2024

seems fine to me then

from axios.

mzabriskie avatar mzabriskie commented on March 28, 2024

Yes @kentcdodds sums up the approach correctly.

.catch(function (res) {
  if (res instanceof Error) {
    // In this case a request was never sent to the server
    // Something happened in setting up the request that triggered an Error
  } else {
    // Here the request was made, but the server responded with a status code
    // that falls out of the range of 2xx
    // You will have the full response details available
    console.log(res.data); // The data that the server responded with
    console.log(res.headers); // The response headers from the server
    console.log(res.status); // The response status code
    console.log(res.config); // The config that was used to make the request
  }
});

from axios.

kentcdodds avatar kentcdodds commented on March 28, 2024

👏 totally reasonable api. Thanks again for making this thing dude.

from axios.

mzabriskie avatar mzabriskie commented on March 28, 2024

Thanks everyone for the dialog. I made the change as discussed and it is now on npm.

from axios.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.