sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.27k stars 935 forks source link

Errors have undefined body when using streams #1138

Closed adityapatadia closed 4 years ago

adityapatadia commented 4 years ago

Describe the bug

Actual behavior

When isStream: true is set and request results in HTTPError, the stream emits error event. The emit handler is only called with error argument and body and response arguments are undefined.

...

Expected behavior

As per the docs, the body and response arguments should be set with error event. ...

Code to reproduce

const got = require('got');

let stream = got("https://www.google.com/non-existant-path", {
  isStream: true,
  method: "GET",
  headers: {}
});

stream.on("response", (response) => {
  console.log(response.headers);
  console.log(response.statusCode)
});

stream.on("error", (err, body, response) => {
  // err is set
  console.log(err);

  // body should have been set but it's undefined
  console.log(body);

  // response should have been set but it's undefined.
  console.log(response);
});

Checklist

szmarczak commented 4 years ago

This has been also fixed in #1051

szmarczak commented 4 years ago

You need to access the response via error.response, the docs are outdated

adityapatadia commented 4 years ago

Is this going to be consistent in future versions? Or is it going to change in #1051 ?

szmarczak commented 4 years ago

I don't know exactly what do you mean, but this issue is fixed in #1051.

adityapatadia commented 4 years ago

I meant currently err.response works. My question is will the function be .on("error", (err, body, response)= {}) or it will be .on("error", (err)= {}) ? Are you going to update the docs with the existing functionality or you are going to give functionality as per current docs?

I am asking because if it's going to change, we will also need to change code.

szmarczak commented 4 years ago

.on('error', error => { ... })

adityapatadia commented 4 years ago

Cool. I will disregard the docs then. I am doing coding as per err.response now.

adityapatadia commented 4 years ago

Btw, I found another bug. Even if decompress:true is passed, the error.response.body is not decompressed if it's a gzip from the server. Should I report that one?

szmarczak commented 4 years ago

yes, it'd be best to include some reproducible code (but I think I can sketch something on my own so that's not necessary, but it will be welcome)

adityapatadia commented 4 years ago

Opened #1142