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

fetch based library (ky), randomly returns empty string instead of actual body (json) #6969

Closed
dimonnwc3 opened this issue Nov 7, 2023 · 10 comments
Labels
bug Something isn't working needs investigate Needs to be investigated to find the root cause

Comments

@dimonnwc3
Copy link

What version of Bun is running?

1.0.10+d85bd49d8

What platform is your computer?

Darwin 23.1.0 arm64 arm

What steps can reproduce the bug?

run this code with bun run

import ky from "ky"

setInterval(async () => {
  const data = await ky(`https://dummyjson.com/products`).json()

  if (!data) {
    console.log("no data")
    console.log(data)
  }
}, 500)

wait for 1 minute, it will print randomly "no data" into the console, meaning that some response bodies are empty string

What is the expected behavior?

I expect to get correct response every time I do request

What do you see instead?

I see empty string in response body

Additional information

same code is running in node v18.15.0 returning consistent results

@dimonnwc3 dimonnwc3 added the bug Something isn't working label Nov 7, 2023
@Electroid Electroid added the needs investigate Needs to be investigated to find the root cause label Nov 7, 2023
@Electroid
Copy link
Contributor

Did you notice this after upgrading Bun recently? Or did you just run into this

@dimonnwc3
Copy link
Author

just run into this randomly

@liz3
Copy link
Contributor

liz3 commented Nov 10, 2023

@dimonnwc3 @Electroid #6468 fixes this whenver it gets merged, just confirmed its because ky uses response.clone() internally and that is currently not handled correctly

@liz3
Copy link
Contributor

liz3 commented Nov 10, 2023

The issue is a race condition whether the internal network process received the entire body payload before resolving the initial fetch promise, if yes the body is correctly copied and all is good, if data is still coming in the internal body state is locked leading to the clone just having a empty body.

@BrennanColberg
Copy link

Experiencing this Response.clone() bug as well with https://www.npmjs.com/package/openapi-fetch.

@liz3
Copy link
Contributor

liz3 commented Dec 8, 2023

The pr would be ready for final review and merge(if it passes review) but the maintainers don't prioritize such bug fixes(api compliance) sadly over adding new features because its a "bigger change", so i cant give you a estimate.

@BrennanColberg
Copy link

BrennanColberg commented Dec 8, 2023

No problem! I've worked around it for now by just replacing response.clone() with response in that library via a well-documented postinstall script, which is unblocking in my use case (do not try this at home)
Screenshot 2023-12-08 at 11 48 13 AM

@dangeredwolf
Copy link

dangeredwolf commented Jan 22, 2024

I am experiencing this bug because I was using clone() in my own code to prevent "Error: Body already used" when initially calling .text() (to perform cryptographic verification) and later in the code calling .json().

Simplest way to trigger the bug as of Bun v1.0.25 (latest as of time of writing)

const server = Bun.serve({
  fetch: async (req) => {
    console.log(await (await req.clone()).text()) // This is empty instead of containing the body text
    console.log(await req.text()) // Contains the actual body, but now you can't call .text(), .json(), etc. again
  },
});

This looks awfully similar to #913 which was apparently fixed, but does not seem to be. Is this actually a regression of that?

Edit: Turns out this is the same underlying issue as #6348 actually

@Electroid
Copy link
Contributor

Duplicate of #6348

@Electroid Electroid marked this as a duplicate of #6348 Jan 22, 2024
@Electroid Electroid closed this as not planned Won't fix, can't repro, duplicate, stale Jan 22, 2024
@Electroid
Copy link
Contributor

(We fill fix this)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs investigate Needs to be investigated to find the root cause
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants