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

Change http response headers (cache-control changed, expires added) #61

Merged
merged 2 commits into from Mar 19, 2018

Conversation

Projects
None yet
2 participants
@chhh
Contributor

chhh commented Mar 18, 2018

I tried using ga-beacon on github today for the first time (2018-03-17). It worked once, while I was editing the README.md, but then updates to analytics stopped. I looked into the source of the landing page of my repo returned by github (the rendering of README.md) and the beacon image was cached. Googling the problem led to rather old tickets in github's own issue tracker. The suggestion by them was to set cache-control and expires. This looks work for me, I've deployed the patched version to https://ga-beacon-nocache.appspot.com if you want to try it out.

I'm setting the expired http header to now, but i'm not exactly sure if that's ok. Maybe it will make more sense to set it to like a few minutes in the future.

@igrigorik

Minor knit (see comment), but otherwise lgtm!

@@ -144,7 +145,9 @@ func handler(w http.ResponseWriter, r *http.Request) {
}
if len(cid) != 0 {
w.Header().Set("Cache-Control", "private, no-store")
var cacheUntil = time.Now().Format(http.TimeFormat)
w.Header().Set("Cache-Control", "no-cache, no-store, must-revalidate")

This comment has been minimized.

@igrigorik

igrigorik Mar 19, 2018

Owner

See #54, should probably keep "private" as well?

@chhh

This comment has been minimized.

Contributor

chhh commented Mar 19, 2018

Added "private" back. But:
I don't think private has any effect when it's no-cache, no-store and expiration is set to be immediate. I used the same parameters that https://shields.io are using for their badges. I don't know about fastly and I haven't tried it in this configuration myself.

@igrigorik

This comment has been minimized.

Owner

igrigorik commented Mar 19, 2018

Looks good, thanks for the great work Dmitry!

@igrigorik igrigorik merged commit 2235bbf into igrigorik:master Mar 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment