feat: set fetch timeout to 300 seconds

[kyrylkov]

No description provided.

[ronag]

LGTM. Would be nice with a test.

[kyrylkov]

@ronag Like no timeout for a request taking 60 seconds for example?

[ronag]

@ronag Like no timeout for a request taking 60 seconds for example?

Or a timeout for a request taking more than 300s. Look at out other test and how we handle timeout testing.

[kyrylkov]

@ronag added tests

[codecov-commenter]

Codecov Report

Merging #1386 (6390e7d) into main (8a8a20f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1386   +/-   ##
=======================================
  Coverage   94.25%   94.25%           
=======================================
  Files          45       45           
  Lines        4211     4211           
=======================================
  Hits         3969     3969           
  Misses        242      242           

Impacted Files	Coverage Δ
lib/fetch/index.js	80.89% <ø> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0608be8...6390e7d. Read the comment docs.

[ronag]

Can you implement the tests using FakeTimers? See our other timeout tests.

[kyrylkov]

@ronag I don't really know how to make it work. This naive doesn't work:

    if (p === '/timeout300s') {
      const clock = FakeTimers.install()
      clock.setTimeout(() => {
        res.statusCode = 200
        res.setHeader('Content-Type', 'text/plain')
        res.end('text')
      }, 300_000)
      clock.tick(300_000)
      clock.uninstall()
    }

[ronag]

@ronag I don't really know how to make it work. This naive doesn't work:

    if (p === '/timeout300s') {
      const clock = FakeTimers.install()
      clock.setTimeout(() => {
        res.statusCode = 200
        res.setHeader('Content-Type', 'text/plain')
        res.end('text')
      }, 300_000)
      clock.tick(300_000)
      clock.uninstall()
    }

No worries. Let's skip the test then and land this. Can you remove it?

[kyrylkov]

I didn't commit FakeTimers. PR has tests with regular timers.

[ronag]

I didn't commit FakeTimers. PR has tests with regular timers.

Can you add { skip: true } to them then. Otherwise, it will take very long to complete the tests? That's why we want to use fake timers.

[kyrylkov]

Done

[kyrylkov]

@ronag It looks like your change is wrong. Fetch uses mocha which requires this.skip(), not an {skip: true} as argument

[kyrylkov]

And that's the reason I cannot make FakeTimers work, cause fetch has different testing infrastructure

[kyrylkov]

Here is the error I'm getting with 300 second timeout when using FakeTimers with setTimeout