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

No way to set timeout for .text() / .json()? #647

Closed
Mercencium opened this issue Oct 30, 2024 · 3 comments
Closed

No way to set timeout for .text() / .json()? #647

Mercencium opened this issue Oct 30, 2024 · 3 comments

Comments

@Mercencium
Copy link

I've sent three requests via await ky.get(url, {timeout: 1_500}).json()

image

The first two requests are cancelled on timeout as expected.

But the third request has connected within 1.5s and started downloading the response body, exceeding the configured timeout:

image

Is there a way to set timeout for reading the body?

Without it, & .text() & .json() promises have the potential to hang indefinitely - that's exactly what happened to me today with vanilla fetch API.

@sholladay
Copy link
Collaborator

I'm pretty sure the AbortController is supposed to cancel the download if one is in progress. This would be good to add to our test suite. Do you have some reproducible code?

@Mercencium
Copy link
Author

Here's the code:

import ky from "ky";

const URL = "https://jsonplaceholder.typicode.com/comments";
const TIMEOUT = 800;

async function test() {
  function elapsed() {
    return Math.round(new Date() - started_at);
  }
  const started_at = new Date();

  let resp;
  try {
    resp = await ky.get(URL, { timeout: TIMEOUT });
  } catch (e) {
    console.error(`Fetch error in ${elapsed()}ms`);
    return;
  }
  console.info(`Connection started in ${elapsed()}ms`);
  let data;
  try {
    data = await resp.json();
  } catch (e) {
    console.error(`JSON error in ${elapsed()}ms`);
    return;
  }

  const total = elapsed();
  const warning = total > TIMEOUT ? "⚠️" : "";
  console.info(`Done in ${total}ms ${warning}`);

  return data;
}

await test();
console.log();
await test();

I ran it with node.js and the output was:

Fetch error in 812ms

Connection started in 618ms
Done in 2720ms ⚠️ 

The first request got timed out at 800ms as expected,
but the second one connected before timeout (in 618ms), and then read the json body in about 2 seconds (2720 - 618).

Note that you need to throttle your connection somehow (Or lower the timeout significantly)
For me It works regardless because my internet sucks 😅

@Mercencium
Copy link
Author

Just discovered that replacing timeout: TIMEOUT option with signal: AbortSignal.timeout(TIMEOUT) fixes this.

Now it can throw an error on .json() call when it timeouts:

Fetch error in 809ms

Connection started in 511ms
JSON error in 803ms

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