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

Cache not decompressed before attempting to JSON.parse #1158

Closed
2 tasks done
simontabor opened this issue Apr 14, 2020 · 6 comments
Closed
2 tasks done

Cache not decompressed before attempting to JSON.parse #1158

simontabor opened this issue Apr 14, 2020 · 6 comments
Labels
bug Something does not work as it should

Comments

@simontabor
Copy link

Describe the bug

  • Node.js version: v10.15.3
  • OS & version: Amazon Linux + MacOS 10.14.6

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

  • I have read the documentation.
  • I have tried my code with the latest version of Node.js and Got.
@simontabor
Copy link
Author

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

@szmarczak
Copy link
Collaborator

Can you set up a RunKit example please?

@szmarczak
Copy link
Collaborator

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 szmarczak added the bug Something does not work as it should label Apr 14, 2020
@szmarczak
Copy link
Collaborator

@szmarczak
Copy link
Collaborator

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

@szmarczak
Copy link
Collaborator

Fixed in 6ca17e2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something does not work as it should
Projects
None yet
Development

No branches or pull requests

2 participants