regression: ky aborting in 1.8.0

[curtdept]

When using options ky seems to abort, provide no return and wont process any further requests.

example:

import HttpRequestMock from "http-request-mock";
import ky from "ky";

const mockHttp = HttpRequestMock.setupForFetch();

mockHttp.post("https://api.site.test/call", {
  data: { payload: "testing" },
});

async function test() {
  const test1 = await ky.post(`https://api.site.test/call`, {
    json: { testBody: "test" },
  });
  console.log("test1", test1);

  //   const test2 = await ky.post(`https://api.site.test/call`);
  //   console.log("test2:", test2);
}

test()
  .then(() => {
    process.exit(0);
  })
  .catch((err) => {
    console.error(err);
    process.exit(1);
  });

Scenarios:

• when running test1 alone, no return, seem to just skip
• when running test1 and test2, no return, both skip
• when running test2 alone, returns as expected

if I comment this out, all scenarios work
https://github.com/sindresorhus/ky/blob/main/source/core/Ky.ts#L70-L72

[sholladay]

All tests for the release passed. This is probably some kind of incompatibility between Ky and the http-request-mock library you are using.

if I comment this out, all scenarios work

That code was added in PR #651 to fix #650. If that is truly the cause, then the fix will have to avoid re-introducing the memory leak. The only way I can think of to do that would be to re-construct the request from scratch on every retry, instead of storing a clone ahead of time. Currently, only function_ is retried. We would have to add at least something like this line, perhaps even the entire constructor, to the retry loop. I'm okay with that, it's arguably the correct thing to do, but beware that it may be tricky. Feel free to send a PR if you want to give that a shot.

[curtdept]

@sholladay ya can't tell you source. Just know 1.7.5 works and commenting works, but no idea why at this point.

[natanhermes]

@curtdept @sholladay I'm a beginner with ky and I'm having a similar problem.

const api = ky.create({
  prefixUrl: 'http://localhost:3333',
})

export async function signInWithEmailAndPassword(data: FormData) {
  console.log(Object.fromEntries(data))
  const { email, password } = Object.fromEntries(data)

  const result = await api
    .post('sessions/password', {
      json: {
        email: String(email),
        password: String(password),
      },
    })
    .json()

  console.log(result)
}

This function is executed in the form action. This request remains pending and nothing happens.

I downgraded to version 1.7.5 and it worked correctly. But, I'm curious to know the cause of this problem.

[sholladay]

What environment? A runnable example on something like CodeSandbox would be useful.

[natanhermes]

What environment? A runnable example on something like CodeSandbox would be useful.

https://codesandbox.io/p/devbox/v2qhp8

I noticed that if I execute the request without await, the code works, when adding it, it remains pending.

not work

export async function signInWithEmailAndPassword(data: FormData) {
  const { email, password } = Object.fromEntries(data);

  const result = await api.post("ky", {
    json: {
      email,
      password,
    },
  });

  console.log(result);
}

works

export async function signInWithEmailAndPassword(data: FormData) {
  const { email, password } = Object.fromEntries(data);

  const result = api.post("ky", {
    json: {
      email,
      password,
    },
  });

  console.log(result);
}

[sukidhar]

experiencing same thing. fetch works, postman works.. but ky does not for some reason. the request happens, but the response is blocked

[naveen-github]

I'm experiencing the same issue when using the POST method.

Here's a minimal reproducible example:

async function makeRequest() {
  try {
    const method: "POST" | "GET" = "POST";
    const body = { title: "foo", body: "bar", userId: 1 }; // Example POST body

    let result;

    if (method === "POST") {
      result = await ky
        .post("https://jsonplaceholder.typicode.com/posts", { json: body })
        .json();
      console.log("POST Response:", result);
    } else {
      result = await ky
        .get("https://jsonplaceholder.typicode.com/posts")
        .json();
      console.log("GET Response:", result[0]);
    }

    return result;
  } catch (err: any) {
    console.error("Request error:", err.message);
  }
}

When I switch the method to GET, everything works as expected:
const method: "POST" | "GET" = "GET";
But with POST, the request doesn't return a response, and the function hangs without throwing an error.

Note: I'm using ky version 1.8.1 if i downgrade the ky version to 1.7.5 then request with POST method also working fine

[segevfiner]

The bug seems to be in this code: v1.7.5...v1.8.1#diff-1d28e040cea335f5a22aa9c3f678f5d00b353e3ce966dc0978a91fe56f240b04R89-R91, it seems to cancel the request body stream before it is even sent...

Tested using the code supplied by @naveen-github above. Commenting out these lines of code fixes the issue, but obviously will fail to cleanup the stream in the cases where it is needed to cancel it.

@sindresorhus @sholladay

[sholladay]

Any fixes are welcome. As I said earlier, we may need to reconstruct the request from scratch on every retry, instead of using a clone. But that's a drastic change that would come with its own set of problems, such as what to do for requests that have a stream body.

We're definitely going to need help on this from anyone who has time to dig into it thoroughly.

[segevfiner]

This currently means 1.8.0 is broken IMHO... Might want to consider a revert and bring the relevant code back into a PR so new users won't hit this issue until someone be it you or someone else has the time to look into it?

[sholladay]

I'm not opposed to a revert. However, the counterpoint is that it's not broken for any scenario in our test suite, which is quite thorough and realistic.

If the test suite were small, or we discovered that the tests passed when they shouldn't have, or somehow all of the tests were missing something fundamental, then that would be a different story. From what I can see, though, that's not the case. So I'm inclined to pursue a fix over a revert that will re-introduce a more widespread memory leak (which we now have a test for).

[segevfiner]

I wonder if a test for this really trivial case is missing or something in the tests does not replicate the behavior of the real streams/fetch well enough.

The question is whether it's accidently closing the wrong stream due to a mistake in the code, the clone isn't cloning the stream, and it needs to be cloned differently, (I think there is some tee functions foe web streams), or something else. If I got the issue correctly.