request dont decompress response body

[PandaWorker]

Bug Description

Why doesn't request support response body decompression?

import { request } from "undici";

const resp = await request('https://jsonplaceholder.typicode.com/todos/1', {
    headers: {
        'accept-encoding': 'gzip'
    }
});

const content = await resp.body.text();
console.log(`status:`, resp.statusCode);
console.log(`encoding:`, resp.headers['content-encoding']);
console.log(`body:`, content); // compressed

Logs & Screenshots

using fetch

using request

Environment

[email protected]

[metcoder95]

That’s because there’s no hard need for request to decompress the body. By spec fetch requires to do so.

This is not a bug in Undici but rather an expected outcome. Decompress is up to the responde handler and not to the client itself per se

[PandaWorker]

Maybe you need to add some parameter to unpack body ?

[metcoder95]

I'm not 100% convinced but not against it. I have the feeling that it can cause several regressions, as well as hide possible optimizations that can be made on the implementer's side. Like custom backpressure and whatnot.

@ronag @mcollina @KhafraDev any thoughts? I think Khafra is already +1 😄 ?

[mcollina]

I'm -1 in adding it to request, it would slow us down even if it's not enabled.

I'm ok to add it under an option, if you really think it's necessary.

[PandaWorker]

I think we need to add the decompress: boolean parameter in requestOptions

[Ealenn]

Hello ! Maybe something automatic, like if the response contains gzip encoding header we should decompress it ?

[metcoder95]

I'm more into the possibility of creating a DecompressHandler that infers the content-encoding header and applies decompression, similar to what we are doing for retries on #2264, but -1 on adding it as part of the request options or within core at all

[KhafraDev]

https://github.com/nodejs/undici/discussions/1155#discussioncomment-4644321

I think that if we are going to recommend request over fetch, it should be just as easy to use. I'm fine with it not being enabled by default, but it wouldn't be that hard to reuse the logic from fetch.

[ronag]

I'm fine with an option. An alternative is to add it through a handler, similar to what https://github.com/nxtedition/nxt-undici does.

Not sure if I like our current "interceptor" pattern but it could go through something like that.

[mcollina]

I'm +1 to an option or a documentated interceptor.

[metcoder95]

Marking as good-first-issue

[Lewiscowles1986]

I Don't know how "good first issue" this is, but here are the files redirect interceptor follows...

https://github.com/nodejs/undici/blob/6a04edcd5867c4de6bd53d10f6b44eaecf02755e/lib/interceptor/redirectInterceptor.js https://github.com/nodejs/undici/blob/6a04edcd5867c4de6bd53d10f6b44eaecf02755e/lib/handler/RedirectHandler.js https://github.com/nodejs/undici/blob/main/test/types/interceptor.test-d.ts#L5 https://github.com/nodejs/undici/blob/main/lib/client.js#L267-269 https://github.com/nodejs/undici/blob/main/types/index.d.ts#L41 https://github.com/nodejs/undici/blob/main/lib/agent.js#L47 https://github.com/nodejs/undici/blob/main/index.js#L44

It might also help (although the redirect interceptor doesn't seem to have); to provide unit tests for the various parts prior to connecting through the whole of undici.

[Lewiscowles1986]

See #951 as well RE: last comment.

[metcoder95]

See RetryHandler to get a sense of how an interceptor looks like.

[KhafraDev]

https://github.com/nodejs/undici/blob/6a04edcd5867c4de6bd53d10f6b44eaecf02755e/lib/fetch/index.js#L2171-L2210 for the actual decompressing