search

denoland / deploy_feedback

For reporting issues with Deno Deploy
https://deno.com/deploy
74 stars 5 forks source link

[Bug]: gzip encoding is not consistent #643

Open sylc opened 6 months ago

sylc commented 6 months ago

Problem description

gzip compression seems random. Some files are sometimes served un-gzipped when they should, based on https://docs.deno.com/deploy/api/compression

Steps to reproduce

It is not always the same set of files being served gzipped.

Expected behavior

I expect the behavior to be consistent with the documentation. i.e gzip is applied consistently.

Environment

It seems to only affect deno deploy. I cannot reproduce on deno v1.42.0.

Possible solution

No response

Additional context

No response

losfair commented 6 months ago

Thanks for finding this!

We are recently rolling out edge cache for static assets. There is a bug that causes the accept-encoding header to be dropped from backend requests. We are working on a fix.

Currently 20% of static asset requests are processed by the cache, so there's the randomness you are seeing :)

sylc commented 6 months ago

Thanks for confirming. I came across this because i have some code that set a cookie if a cookie is not already present. however sometimes the cookie was getting reset even though it was provided in the request. It seems to correspond with when the response was not gzipped when it should have been. Would it be possible that not only the accept-encoding was dropped, but also the cookie header ?

losfair commented 6 months ago

Are you observing this behavior only on requests to paths starting with /_frsh/? Currently caching is only enabled for request and response pairs with a /_frsh/* URL and a proper cache-control header.

We do drop Cookie and Authorization headers from the request for those that hit the cache.

sylc commented 6 months ago

Are you observing this behavior only on requests to paths starting with /_frsh/?

yes. Since i didn't really need this cookie for those routes, i implemented a workaround like below

if (ctx.destination == "static" || ctx.destination == "internal") { 
   // don't try to check cookie, just return
    return await ctx.next();
  }

We do drop Cookie and Authorization headers from the request for those that hit the cache.

Since i implemented my change, this should not impact me anymore, but just to be clear for the future, when you will implement the fix for the accept-encoding header, all other headers, including the Cookie and Authorization headers, will also be fixed, i.e preserved again, correct?