Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

timeout does not work for slow responses #60

Closed
tripodsan opened this issue Jun 25, 2020 · 1 comment
Closed

timeout does not work for slow responses #60

tripodsan opened this issue Jun 25, 2020 · 1 comment
Assignees

Comments

@tripodsan
Copy link
Contributor

tripodsan commented Jun 25, 2020

Description
the timeout that can be specified in the options only applies to the connection/first-byte response time, but not to the overall response time. if the caller wants to limit the time fetch spends downloading a resource, this is not possible.

To Reproduce

it('handles dripped response', async() => {
  const context = fetchAPI.context({
    // httpProtocol: 'http1',
    // httpsProtocols: ['http1'],
  });
  try {
    const resp = await context.fetch('https://httpbin.org/drip?duration=10&numbytes=10&code=200&delay=1', {
      cache: 'no-store',
      redirect: 'follow',
      timeout: 3000,
    });
    const text = await resp.text();
    console.log(resp.ok, resp.status, resp.headers, text);
    assert.fail('should timeout');
  } catch (e) {
    console.error(e);
    // ok!
  } finally {
    await context.disconnectAll();
  }
}).timeout(5000);

Expected behavior
the request should timeout and the test should pass.

@tripodsan tripodsan added the bug Something isn't working label Jun 25, 2020
@stefan-guggisberg stefan-guggisberg self-assigned this Jun 25, 2020
@stefan-guggisberg stefan-guggisberg removed the bug Something isn't working label Jun 30, 2020
@stefan-guggisberg
Copy link
Contributor

The current behaviour is consistent with request, phin and other HTTP client modules, i.e. the timeout only applies up to the point where the response headers are received. I therefore don't consider it a bug.

Due to these shortcomings I have deprecated the non-standard timeout option and provided the standard API AbortController as a standard-compliant alternative, see #64.

See https://github.com/adobe/helix-fetch/blob/master/test/index.test.js#L440-L457 for an equivalent test case using abort signal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants