statusCode for responses labeled undefined when promise has a statusCode of 200

dominusmars commented 3 years ago

Describe the bug

Node.js version: v12.19.0.
OS & version: windows 10 20H2

Actual behavior

... terminates node.js process due to unhandled error. Stated that statusCode is undefined.

(node:28884) UnhandledPromiseRejectionWarning: TypeError: Cannot destructure property 'statusCode' of 'response' as it is undefined. at Object.exports.isResponseOk (C:\Users\OneandOnly\Desktop\Sensor\node_modules\got\dist\source\core\utils\is-response-ok.js:5:13) at Request.<anonymous> (C:\Users\OneandOnly\Desktop\Sensor\node_modules\got\dist\source\as-promise\index.js:116:39) at processTicksAndRejections (internal/process/task_queues.js:97:5)

Expected behavior

... returns promise statusCode as 200 if the site is live.

Code to reproduce

const site= got.extend({
        method: "GET",
        followRedirect: false,
        headers: {
            "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:82.0) Gecko/20100101 Firefox/82.0",
            accept: "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8",
            "accept-language": "en-US,en;q=0.9,en",
            "accept-encoding": "gzip, deflate, br",
            Connection: "keep-alive",
            "Upgrade-Insecure-Request": "1",
        },
        hooks: {
            beforeResponse: [(options) => {}],
            afterResponse: [
                (response) => {
                    setTimeout(function () {
                        fs.writeFileSync("response.txt", response.body);
                    }, 1000);
                },
            ],
        },
    });
    try {
        var { response } = site.get("https://www.google.com");
        return response;
    } catch (error) {
        console.log(error);
    }

Checklist

[x] I have read the documentation.
[x] I have tried my code with the latest version of Node.js and Got.

sindresorhus commented 3 years ago

TypeError: Cannot destructure property 'statusCode' of 'response' as it is undefined.

This indeed a Got bug that crashes here, but your code is also invalid and even if Got didn't crash here, yours would.

afterResponse: [
                (response) => {
                    setTimeout(function () {
                        fs.writeFileSync("response.txt", response.body);
                    }, 1000);
                },
            ],

You should not use setTimeout. This will cause an unhandled rejection when the write call fails. Use delay or something similar, make afterResponse an async function, and promisify the fs call.

sindresorhus commented 3 years ago

@szmarczak We should add validation that ensures the user returns a response from afterResponse and throw a human-friendly error message if not.

Each function should return the response. - https://github.com/sindresorhus/got#hooksafterresponse

dominusmars commented 3 years ago

TypeError: Cannot destructure property 'statusCode' of 'response' as it is undefined.

This indeed a Got bug that crashes here, but your code is also invalid and even if Got didn't crash here, yours would.
afterResponse: [
              (response) => {
                  setTimeout(function () {
                      fs.writeFileSync("response.txt", response.body);
                  }, 1000);
              },
          ],
You should not use setTimeout. This will cause an unhandled rejection when the write call fails. Use delay or something similar, make afterResponse an async function, and promisify the fs call.

thanks for the advice. was trying stuff out to see what would happen.

sindresorhus / got