sindresorhus / got

🌐 Human-friendly and powerful HTTP request library for Node.js
MIT License
14.27k stars 935 forks source link

Cache not decompressed before attempting to JSON.parse #1158

Closed simontabor closed 4 years ago

simontabor commented 4 years ago

Describe the bug

When using the request cache with decompress: true, responseType: 'json', the first request succeeds. For a response with a 60s ttl, the next 60s of requests succeed too.

Then, when it comes to revalidating the cache, a 304 response is returned, and an error is thrown.

so

Actual behavior

The cached response is gzipped, which is then not decompressed again (it seems), which means JSON.parse throws an error as it's trying to parse bytes...

GotError: Unexpected token \u001f in JSON at position 0 in \"url-here\"\n    at EventEmitter.emitter.on (/Users/simon.tabor/Development/fe-router/node_modules/got/dist/source/as-promise.js:78:40)\n    at JSON.parse (<anonymous>)\n    at parseBody (/Users/simon.tabor/Development/fe-router/node_modules/got/dist/source/as-promise.js:12:46)\n    at EventEmitter.emitter.on (/Users/simon.tabor/Development/fe-router/node_modules/got/dist/source/as-promise.js:72:33)\n    at process._tickCallback (internal/process/next_tick.js:68:7)

Expected behavior

We expect the cached body to be used. I'd also prefer the cache to be decompressed, so each call to the cache uses less CPU.

Code to reproduce

const cache = new Map();
const response = await got(url, {
    cache,
    responseType: 'json',
    decompress: true,
    timeout: 5000,
    retry: 2,
});

The URL in question responds with cache-control: public,max-age:60 and etag headers.

Setting decompress: false fixes the issue

Checklist

simontabor commented 4 years ago

This appears to be fixed when using ^11.0.0-beta.1

szmarczak commented 4 years ago

Can you set up a RunKit example please?

szmarczak commented 4 years ago

I'd also prefer the cache to be decompressed, so each call to the cache uses less CPU.

Please make an issue in the cacheable-request repository.

szmarczak commented 4 years ago

https://runkit.com/szmarczak/5e95e5db36834d0013b5633a

szmarczak commented 4 years ago

Reproduced in the example above, will now make a test.

szmarczak commented 4 years ago

Fixed in 6ca17e20460685016035fba607fa394efb3e89bf