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

undici@5.24.0

[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]

#1155 (comment)

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]

undici/lib/fetch/index.js

Lines 2171 to 2210 in 6a04edc

	this.body = new Readable({ read: resume })
	const decoders = []
	const willFollow = location && request.redirect === 'follow' &&
	redirectStatusSet.has(status)
	// https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding
	if (request.method !== 'HEAD' && request.method !== 'CONNECT' && !nullBodyStatus.includes(status) && !willFollow) {
	for (let i = 0; i < codings.length; ++i) {
	const coding = codings[i]
	// https://www.rfc-editor.org/rfc/rfc9112.html#section-7.2
	if (coding === 'x-gzip' || coding === 'gzip') {
	decoders.push(zlib.createGunzip({
	// Be less strict when decoding compressed responses, since sometimes
	// servers send slightly invalid responses that are still accepted
	// by common browsers.
	// Always using Z_SYNC_FLUSH is what cURL does.
	flush: zlib.constants.Z_SYNC_FLUSH,
	finishFlush: zlib.constants.Z_SYNC_FLUSH
	}))
	} else if (coding === 'deflate') {
	decoders.push(zlib.createInflate())
	} else if (coding === 'br') {
	decoders.push(zlib.createBrotliDecompress())
	} else {
	decoders.length = 0
	break
	}
	}
	}
	resolve({
	status,
	statusText,
	headersList,
	body: decoders.length
	? pipeline(this.body, ...decoders, () => { })
	: this.body.on('error', () => {})
	})

for the actual decompressing