sindresorhus / got

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

Timeout does not release control within acceptable amount of time #1705

Closed MatthewLymer closed 3 years ago

MatthewLymer commented 3 years ago

Describe the bug

The timeout configuration does not release control back to the caller within a reasonable timeframe.

Actual behavior

The error should fire within a 250 ms of the configured timeout

Expected behavior

The error fires 5 to 20 seconds after configured timeout.

  1) got timeout
       should timeout if request takes longer than 1 seconds:

      AssertionError [ERR_ASSERTION]: Timeout elapsed of '6202ms' greater than threshold of '1250ms'.

  2) got timeout
       should timeout if request takes longer than 2 seconds:

      AssertionError [ERR_ASSERTION]: Timeout elapsed of '9134ms' greater than threshold of '2250ms'.

  3) got timeout
       should timeout if request takes longer than 4 seconds:

      AssertionError [ERR_ASSERTION]: Timeout elapsed of '15105ms' greater than threshold of '4250ms'.

  4) got timeout
       should timeout if request takes longer than 8 seconds:

      AssertionError [ERR_ASSERTION]: Timeout elapsed of '27110ms' greater than threshold of '8250ms'.

Code to reproduce

const got = require("got");
const assert = require('assert');

describe('got HttpClient', () => {

  // NOTE: httpbin.org supports a maximum of 10s delay
  [1, 2, 4, 8].forEach(timeoutSeconds => {
    it(`should timeout if request takes longer than ${timeoutSeconds} seconds`, async function () {

      // set mocha timeout with a 30 second buffer
      this.timeout((timeoutSeconds + 30) * 1000);

      const timeoutMilliseconds = timeoutSeconds * 1000;
      const maximumElapsedMilliseconds = timeoutMilliseconds + 250;
      const minimumElapsedMilliseconds = timeoutMilliseconds - 250;

      const start = Date.now();

      try {
        await got.get(`https://httpbin.org/delay/${timeoutSeconds+1}`, {timeout: timeoutMilliseconds});
        assert.fail("Timeout was expected but never happened.");
      }
      catch (err) {
        const elapsed = Date.now() - start;

        assert(elapsed <= maximumElapsedMilliseconds, `Timeout elapsed of '${elapsed}ms' greater than threshold of '${maximumElapsedMilliseconds}ms'.`);
        assert(elapsed >= minimumElapsedMilliseconds, `Timeout elapsed of '${elapsed}ms' less than threshold of '${minimumElapsedMilliseconds}ms'.`);
      }
    });
  });
});

Checklist

szmarczak commented 3 years ago

By default, Got will retry on failure. To disable this option, set options.retry to 0.

szmarczak commented 3 years ago

https://github.com/sindresorhus/got#api