undici icon indicating copy to clipboard operation
undici copied to clipboard

native `fetch` api does not respect `keepalive` parameter in `RequestInit`

Open TheMythologist opened this issue 1 year ago • 8 comments

Version

18.16.0

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

No response

What steps will reproduce the bug?

fetch("http://localhost", { keepalive: false })

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

Do not send the HTTP connection: keep-alive header. Instead, send a connection: close header.

What do you see instead?

The connection: keep-alive header will still be sent in the HTTP request. image

Additional information

No response

TheMythologist avatar Jun 23 '23 03:06 TheMythologist

use axios instead or downgrade

Gloriouspete avatar Jun 23 '23 08:06 Gloriouspete

@ronag undici sets connection: close by default, but we should also support keepalive imo? Or document it.

KhafraDev avatar Jun 23 '23 17:06 KhafraDev

keepalive is false by default, and it has nothing to do with the connection: keep-alive header.

See https://fetch.spec.whatwg.org/#request-keepalive-flag

request . keepalive Returns a boolean indicating whether or not request can outlive the global in which it was created.

I think this option doesn't make much sense in Node.js (if the global environment is shutdown, there is nothing left, unlike a browser that may still keep a request alive after closing the tab that initiated it).

targos avatar Jun 23 '23 17:06 targos

global in this case could mean a worker, which would be applicable to node (although there are other issues with fetch in worker threads). You're right about it not involving the connection header, I didn't look into it before commenting.

I guess the best example would be:

import { Worker } from 'node:worker_threads'

new Worker(`fetch('https://some.large/file/that/takes/time/to/download', { keepalive: true })`)

KhafraDev avatar Jun 23 '23 18:06 KhafraDev

but in that case it's not something we can fix, so this is just a documentation issue

KhafraDev avatar Jun 23 '23 18:06 KhafraDev

Apologies, it seems I'm confused. I ran into this error when I was attempting to migrate away from node-fetch and use the native node's fetch API.

For context, I am mocking several HTTP API routes using koa, and using them in my tests via jest. However, after the migration, all behaviour was normal and working as expected, except the tests would fail because the jest tests take longer than 5s (the default) to complete. This was caused by the header connection: keep-alive being sent, causing the koa server to refuse to close. (As a positive control, I added a middleware that forcefully set the connection: close header and the tests passed once again.)

Is disabling of this connection: keep-alive HTTP header something that can be set in RequestInit? From what I understood from your comments it seem the keepalive option in RequestInit is not what I'm looking for.

Thanks for your help!

TheMythologist avatar Jun 24 '23 01:06 TheMythologist

Is disabling of this connection: keep-alive HTTP header something that can be set in RequestInit

No, neither undici or the fetch spec allow modifying the connection header. If you search undici's issues you should be able to find previous discussions about it.

KhafraDev avatar Jun 24 '23 01:06 KhafraDev

It's actually possible to disable it by tweaking the Agent.

mcollina avatar Jun 24 '23 08:06 mcollina

i found it from https://github.com/nodejs/undici/pull/1829#pullrequestreview-1233773275

add conection: "close" to header

phapntm avatar Feb 24 '24 05:02 phapntm