cacheable: Got Overwriting cache with 304 response

with got 8.2.0 got.stream(imgUrl, { cache: keyv }).pipe(res); records in mongodb: { "_id": ObjectID("5ab084465a793ecc436a7849"), "key": "cacheable-request:GET:https://farm2.staticflickr.com/1521/26180297656_675e75a2ce_h.jpg", "value": "{\"value\":{\"cachePolicy\":{\"v\":1,\"t\":1521635364661,\"sh\":true,\"ch\":0.1,\"imm\":86400000,\"st\":200,\"resh\":{\"date\":\"Wed, 21 Mar 2018 12:29:24 GMT\",\"connection\":\"close\",\"expires\":\"Wed, 28 Mar 2018 12:29:24 UTC\",\"cache-control\":\"max-age=604800,public\",\"via\":\"http/1.1 pc-pool116.flickr.gq1.yahoo.com (ApacheTrafficServer [cMsSfW])\",\"server\":\"ATS\",\"x-photo-farm\":\"2\",\"x-photo-farm-guess\":\"2\",\"access-control-allow-origin\":\"*\",\"access-control-allow-methods\":\"POST, GET, OPTIONS\"},\"rescc\":{\"max-age\":\"604800\",\"public\":true},\"m\":\"GET\",\"a\":true,\"reqh\":null,\"reqcc\":{}},\"url\":\"\",\"statusCode\":304,\"body\":\":base64:\"},\"expires\":null}", "expiresAt": null }

About this issue

Original URL
State: closed
Created 6 years ago
Comments: 55 (27 by maintainers)

Commits related to this issue

Make clients gracefully handle mismatched 304 responses https://github.com/lukechilds/cacheable-request/issues/28 — committed to kornelski/http-cache-semantics by kornelski 6 years ago
Preserve original status code when returning cached responses (#67) Potential solution to https://github.com/lukechilds/cacheable-request/issues/28 — committed to jaredwray/cacheable by lukechilds 5 years ago

Most upvoted comments

Thanks for looking into this @UltCombo, really appreciate that.

I’m not sure if we should try and patch things up by setting what we think the server means to respond with.

If the server isn’t following the spec then it might be getting other things wrong and it’s possible that it shouldn’t even be returning a 304.

If this issue causes the cache to be updated with an empty response then it must be this check returning false: https://github.com/lukechilds/cacheable-request/blob/master/src/index.js#L82-L88 which happens inside the http-cache-semantics lib. If we decide to work around this, it should be handled there. (pinging @kornelski just so you’re aware)

See if @kornelski is interested in handling this scenario. If not, thanks for sharing your solution, it’ll be helpful for anyone else who comes accross this.

lukechilds on Dec 24, 2018

cacheable-request@6.0.0 published which I think should resolve this. I’d be grateful if anyone who was encoutnering the issue could test this.

lukechilds on Jan 4, 2019

@CyberShadow - so sorry about that and will include you on the release notes.

jaredwray on Jul 5, 2022

Thank you. In the future, if you use my work, I would appreciate it if you would add due credit in the commit message. Thanks!

CyberShadow on Jul 5, 2022

We have stopped using this long ago so we are not able to verify this.

adityapatadia on May 19, 2022

I am going to put logs in our production system to see if it is reproduced. I will post here in a few days.

adityapatadia on Jan 31, 2019

I have deployed on production. Let’s see. I will close this after confirming the issue is resolved.

adityapatadia on Jan 13, 2019

It’s fixed in http-cache-semantics@v4.0.2 and we require ^4.0.0, so fresh installs should be good.

lukechilds on Jan 11, 2019

I’ve removed my workaround, installed cacheable-request@6.0.0 in got@9.5.0 and the issue still persists: getting empty response body.

I just double-checked and my dependency tree is up-to-date:

└─┬ got@9.5.0
  └─┬ cacheable-request@6.0.0
    └── http-cache-semantics@4.0.1

UltCombo on Jan 8, 2019