Page MenuHomePhabricator

[Timebox 4h] - Add proper cache related headers in nginx config for UI
Closed, ResolvedPublic

Description

Client-side caching for the nginx file server that is serving the assets for ui could be improved further:

Make sure index.html is never being served from cache without validating it is up to date

When we change the Vue application, a new bundles for JS and CSS are created. Once shipped, browsers should immediately pick up a new index.html document, referencing the new bundles.

Currently, this does not seem to work reliably. We do send ETag and Last-Modified headers with the root document, but there is no Cache-Control header, leaving it up to browsers to decide whether they are allowed to reuse a previously cached response. To trigger the behavior we want, Cache-Control: no-cache needs to be added to such responses. This means, the browser will only ever use a previously cached asset if the server returned a 304 Not Modified response.

Another option considering the small size of the index.html document would be using Cache-Control: no-store, forcing the browser to fetch the document on every reload.

Add harder caching for revisioned assets

JS and CSS bundles are revisioned by the build system. However, we do not send a Cache-Control header here either. As these files will never change after being created (as opposed to index.html), we can set "hard" cache control values to make sure users will never have to redownload the same bundle twice. Something like Cache-Control: max-age: 31536000, immutable should work well.


Bonus question

Serving a single page app like this is common practice. We could also try to find out if there is a "best-practices" nginx config or even Docker image that can do all of the above for us, and we can think about more important things.

Event Timeline

Just leaving a pointer here that for Wikidata production we solved essentially the same issue in T301461. (TL;DR: we also went for Cache-Control: no-cache. Keep in mind that applies not only to index.html but also to embed.html!)

When @conny-kawohl_WMDE and @Charlie_WMDE looked at this we decided that this ought to be timeboxed; we've moved it back to let engineers decide on a sensible timebox for this work in a refinement session.

Deniz_WMDE renamed this task from Add proper cache related headers in nginx config for UI to [Timebox 4h] - Add proper cache related headers in nginx config for UI.Feb 8 2024, 2:42 PM

In todays tech refinement session Tom and I agreed that 4 hours may be a sensible timebox for this, since it seems we're quite close to solving this but caching can cause issues in complex ways.

timebox left: 3h

Findings so far:

  • 1. this doesn't seem to be reproducible in our local dev env, probably due to the minikube tunnel we are using (just a guess)
  • 2. adding add_header Cache-Control "no-cache"; to the platform-ui's nginx conf adds the header locally, but on staging it isn't there (not entirely sure why, but I guess related to 1.)

I assume we have to look at the ingress nginx conf instead, but I want to discuss this with other engineers further first.

The magic keyword always seems to do the trick (thanks Frederik!): https://github.com/wbstack/ui/pull/791

Frederik noticed that /config.js does not get bundled and does not get distributed with a cache-busting filename. I added it to the nginx conf so it gets Cache-Control: no-cache

Tested this locally and approved the PR.