Got: Unhandled promise rejection with "socket hang up" & timeout

[darksabrefr]

On socket hang up errors, timeouts are not cleared.
When the socket hang up occurs, it is immediately thrown so the promise returned by got immediately rejects. But as timeouts are not cleared, you will get an unhandled promise rejection after the time set in your timeout.

• Node.js version: 13.2.0
• OS & version: Ubuntu 19.10

Actual behavior

UnhandledPromiseRejectionWarning: GotError: Timeout awaiting 'request' for 1000ms
    at onError (/home/clement/dev/tests/node_modules/got/dist/source/request-as-event-emitter.js:121:29)
    at ClientRequest.<anonymous> (/home/clement/dev/tests/node_modules/got/dist/source/request-as-event-emitter.js:136:21)
    at Object.onceWrapper (events.js:300:26)
    at ClientRequest.emit (events.js:215:7)
    at Immediate.timeoutHandler [as _onImmediate] (/home/clement/dev/tests/node_modules/got/dist/source/utils/timed-out.js:58:17)
    at Immediate.timeoutHandler [as _onImmediate] (/home/clement/dev/tests/node_modules/got/dist/source/utils/timed-out.js:58:31)
    at processImmediate (internal/timers.js:441:21)
    at emitUnhandledRejectionWarning (internal/process/promises.js:141:15)
    at processPromiseRejections (internal/process/promises.js:203:11)
    at processTicksAndRejections (internal/process/task_queues.js:94:32)

Code to reproduce

You need a socket hang up to see the bug. A good manner to have one if to put socket.destroy(); here. It's a simulation, but I can generate the same stack trace as my real life application where "true" socket hang ups sometimes occur.

'use strict';

const got = require('got');

const gotOptions = {
    timeout: 1000,
    retry: 0
};
got('https://google.com', gotOptions)
    .catch((e) => {
        console.log(e.message)
    });

setTimeout(() => {}, 5000);

Checklist

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

[szmarczak]

The check was introduced here and it was proper while using the same check in request-as-event-emitter.ts, but it has been changed in #908

[szmarczak]

I think it's safe to remove it now.

[darksabrefr]

Thank you again @szmarczak for your fast fix !

[xPaw]

I think it this still happens if you cancel the promise on 10.0.2.

1. Call promise.cancel() (for example in downloadProgress)
2. catch CancelError: Promise was canceled
3. then resolved with first param being undefined

And later I'm getting

(node:14920) UnhandledPromiseRejectionWarning: GotError: Timeout awaiting 'request' for 1000ms                                                        
at onError (D:\GitHub\thelounge\node_modules\got\dist\source\request-as-event-emitter.js:121:29)                                                  
at ClientRequest.<anonymous> (D:\GitHub\thelounge\node_modules\got\dist\source\request-as-event-emitter.js:136:21)  

[xPaw]

Furthermore, if a request times out after receiving headers (response event), it throws GotError timeout, and then I get a unhandled rejection with premature close (I am not canceling the request anywhere myself.

catch GotError: Timeout awaiting 'request' for 1000ms <- fine

And then,

(node:15204) UnhandledPromiseRejectionWarning: Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at IncomingMessage.onclose (internal/streams/end-of-stream.js:61:15)
    at IncomingMessage.emit (events.js:219:5)
    at IncomingMessage.origin.emit (D:\GitHub\thelounge\node_modules\@szmarczak\http-timer\dist\index.js:40:20)
    at TLSSocket.socketCloseListener (_http_client.js:377:11)
    at TLSSocket.emit (events.js:224:7)
    at net.js:666:12
    at TCP.done (_tls_wrap.js:488:7)

[szmarczak]

@xPaw A RunKit example(s) would be best.

[szmarczak]

I cannot reproduce: https://runkit.com/szmarczak/5dec2a32477b9a001a784841

[xPaw]

This code reproduces:

const got = require('got');

process.on('unhandledRejection', console.log);

(async () => {
    const promise = got('https://example.com', {
        timeout: 1000,
        retry: 0,
    }).on('downloadProgress', () => {
        promise.cancel();
    });

    try {
        await promise;
    } catch (error) {
        console.log('Caught:', error);
    }

    await new Promise(resolve => setTimeout(resolve, 2000));
})();

I think retry: 0 is important for it to happen.

The other issue I mentioned I tested against a php script that had a sleep(5); in it after flushing down some data to the socket.

[szmarczak]

@xPaw Indeed. Could you open a new issue about this?

[calvinl]

Furthermore, if a request times out after receiving headers (response event), it throws GotError timeout, and then I get a unhandled rejection with premature close (I am not canceling the request anywhere myself.

catch GotError: Timeout awaiting 'request' for 1000ms <- fine

And then,

(node:15204) UnhandledPromiseRejectionWarning: Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at IncomingMessage.onclose (internal/streams/end-of-stream.js:61:15)
    at IncomingMessage.emit (events.js:219:5)
    at IncomingMessage.origin.emit (D:\GitHub\thelounge\node_modules\@szmarczak\http-timer\dist\index.js:40:20)
    at TLSSocket.socketCloseListener (_http_client.js:377:11)
    at TLSSocket.emit (events.js:224:7)
    at net.js:666:12
    at TCP.done (_tls_wrap.js:488:7)

I'm not 100% certain yet, but I think I may be seeing this issue (hopefully it's not something in my code). I have a script that makes 100 requests to various URLs and even though I've wrapped the requests in a try/catch block there are some that slip through with a UnhandledPromiseRejectionWarning error after the timeout. Not sure how to reproduce though, but here's some pseudocode:

const urls = [...];
const processUrl = async url => {
  try {
    const res = await got(url, { ...gotOptions });

    // do something
    return res.body;
  } catch (e) {
    // handle error
  }
}
const results = await Promise.all(urls.map(processUrl));

I also tried with 10.0.3 and it still seems to be happening. Maybe I'm doing something wrong here?

[szmarczak]

Please include some reproducible code. That's not enough info to solve your issue :/

[szmarczak]

What are your options?

[calvinl]

@szmarczak I will - just wanted to raise it first since @xPaw brought it up but their reproducible bit had cancellations in there. I suspect it may be a race condition, but will keep you posted. Thank you!