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

Non-GET http2 requests without a body hang indefninitely #2589

Closed
ktaekwon000 opened this issue Jan 4, 2024 · 8 comments
Closed

Non-GET http2 requests without a body hang indefninitely #2589

ktaekwon000 opened this issue Jan 4, 2024 · 8 comments
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@ktaekwon000
Copy link

Bug Description

When using fetch to make non-GET/HEAD http2 requests, the function call hangs indefinitely. Passing in an empty string in place of the body fixes this issue.

Reproducible By

The following js script triggers the issue on undici v6.2.1

'use strict'

const { fetch, Client } = require('undici')

async function main () {
    const client = new Client("https://google.com", { allowH2: true });
    const res = await fetch("https://google.com", {
        dispatcher: client,
        baseURL: 'https://google.com',
        url: 'https://google.com/',
        method: "POST",
    })

    const data = await res.text()
    console.log('response received', res.status)
    console.log('headers', res.headers)
    console.log('data', data)
}
main()

Expected Behavior

The function call should send the request with an empty body.

Environment

macOS 14.0, node v20.8.1, undici v6.2.1

Additional context

I believe the issue is closely related to #2258 , but while this PR had only fixed GET/HEAD requests, this actually affects all kinds of requests.

@ktaekwon000 ktaekwon000 added the bug Something isn't working label Jan 4, 2024
@metcoder95
Copy link
Member

Hey! Thanks for the report

I believe this should be scope to validate wether or not there's a body at all being sent.

Would you like to submit a PR?

@metcoder95 metcoder95 added the good first issue Good for newcomers label Jan 4, 2024
@ronag
Copy link
Member

ronag commented Jan 4, 2024

I believe this should be scope to validate wether or not there's a body at all being sent.

I'm confused. It should be possible to send a fetch without body? Also this sounds like we are missing something in the spec language? @KhafraDev

@KhafraDev
Copy link
Member

this isn't an issue with fetch afaict

@timursevimli
Copy link
Contributor

While I was trying to investigate this problem, I discovered a completely different problem.

When we pass body the following object appears:

{
   finalBody: {
     stream: ReadableStream { locked: false, state: 'readable', supportsBYOB: true },
     source: 'helloWorld',
     length: 10
   }
}

(For this output I used output a variable called finalBody in undici/lib/fetch/request.js:505)

Well, if we pass an object to body, then we get the following output:

{
   finalBody: {
     stream: ReadableStream { locked: false, state: 'readable', supportsBYOB: true },
     source: '[object Object]',
     length: 15
   }
}

In this case, regardless of the size of our object, length will always be 15 and source will be incorrect.

@ktaekwon000
Copy link
Author

@timursevimli I believe the problem you're facing is not a bug, and not related to the problem I've posted in the OP. Try passing in your object as data not body, or stringifiying the JSON yourself and setting the respective content-type if using body.

@Piyush-Kumar-Ghosh
Copy link

Proposed Solutions:

  1. Enhancing undici Library:

    • File to Modify: lib/fetch.js
    • Approach: I suggest a tweak in the undici library where we can specifically check for an empty body. This adjustment would involve handling the content-length header appropriately.
  2. Adjusting Fetch Options:

    • Resolution Approach: To address the issue, consider adjusting the fetch options. Explicitly include an empty body in the request.
    // Example Code Snippet
    const res = await fetch("https://google.com", {
        dispatcher: client,
        baseURL: 'https://google.com',
        url: 'https://google.com/',
        method: "POST",
        // Include an empty body
        body: ''
    });

    This modification ensures that the function call sends the request with an empty body, effectively resolving the hanging behavior.

@mcollina
Copy link
Member

mcollina commented Jan 8, 2024

Given this works with HTTP/1.1, it's not a bug in fetch() but in our HTTP/2 implementation.

@ktaekwon000 ktaekwon000 changed the title fetch hangs on non-GET http2 requests without a body Non-GET http2 requests without a body hang indefninitely Jan 22, 2024
@ktaekwon000
Copy link
Author

I can verify that the script in the OP runs without issues on v6.4.0 of undici. Thanks everyone for the comments and @timursevimli for the PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

7 participants