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

Make sure that Metabase JS/CSS isn't cached by browser #2210

Closed
agilliland opened this issue Mar 22, 2016 · 14 comments
Closed

Make sure that Metabase JS/CSS isn't cached by browser #2210

agilliland opened this issue Mar 22, 2016 · 14 comments
Assignees
Labels
Type:Bug Product defects
Milestone

Comments

@agilliland
Copy link
Contributor

Seen a couple reports of cases where the app was updated and the index.html page served by the metabase frontend had changed, but the user's browser was continuing to use an older version of the page until a hard page refresh was done.

@camsaul
Copy link
Member

camsaul commented Mar 22, 2016

A normal refresh didn't do it?

@agilliland
Copy link
Contributor Author

nope. had to be a hard refresh.

@camsaul
Copy link
Member

camsaul commented Mar 22, 2016

Here's the headers that come back with index.html. Something must be off here

screen shot 2016-03-22 at 3 47 04 pm

Also, what browser were you using?

@tlrobinson
Copy link
Contributor

FWIW we don't actually know if it was the index.html or the js/css that was being cached, do we? In theory we have a cache busting set up for anything loaded by webpack, but it's possible it's not working.

@camsaul camsaul added the New label Mar 22, 2016
@agilliland
Copy link
Contributor Author

Correct. I filed this because we saw a user run into this problem and a refresh was the solution, but we don't know the exact cause.

@agilliland
Copy link
Contributor Author

Just a quick update that we've had several other people report running into this issue on the latest 0.16.0 release candidate, so it's definitely happening. Here's a screenshot of the problem in action on the QB ...

screen shot 2016-03-23 at 2 20 31 pm

@camsaul
Copy link
Member

camsaul commented Mar 23, 2016

I think @tlrobinson is right about the JS / CSS being the issue here. The cache-busting headers are only added to index.html and API calls.

app.bundle.js

screen shot 2016-03-23 at 2 36 31 pm

relevant code:
(defn add-security-headers
  "Add HTTP headers to tell browsers not to cache API responses."
  [handler]
  (fn [request]
    (let [response (handler request)]
      (update response :headers merge (cond
                                        (api-call? request) (api-security-headers)
                                        (index? request)    (index-page-security-headers))))))

@camsaul camsaul changed the title Make sure that Metabase html page isn't cached by browser Make sure that Metabase JS/CSS page isn't cached by browser Mar 23, 2016
@camsaul camsaul changed the title Make sure that Metabase JS/CSS page isn't cached by browser Make sure that Metabase JS/CSS isn't cached by browser Mar 23, 2016
@agilliland
Copy link
Contributor Author

The cache busting for the JS/CSS should be happening due to the fact that we append a randomized hash on those urls right?

In any case, given that we've seen several folks run into this since deploying 0.16.0 can we address this somehow before the release goes out?

@tlrobinson
Copy link
Contributor

@agilliland correct, JS and CSS files should be aggressively cached, then "cache busted" by changing the hash in the query string when re-built. AFAIK Webpack is doing this correctly, so I'm pretty stumped why this is happening. I'll keep poking at it.

@camsaul
Copy link
Member

camsaul commented Mar 24, 2016

I don't think the randomized hash is working correctly. I'm looking at the network inspector and when I soft reload a page it fetches vendor.bundle.js with the exact same hash as the last time.

@camsaul
Copy link
Member

camsaul commented Mar 24, 2016

Why can't we just add cache-prevention headers to the JS and CSS files and skip this random hash stuff?

@tlrobinson
Copy link
Contributor

Because we have like 2MB of JS/CSS that we don't want to reload every time unless it actually changes. This is a pretty common practice.

@tlrobinson
Copy link
Contributor

Ok so I'm pretty sure I figured it out... it wasn't index.html, css, or js being cached, it was...

screen shot 2016-03-25 at 10 48 50 am

One of the last few Angular HTML partials, hooray!

Changing the headers for partials won't fix this for people who already have the partial cached. A couple options:

  1. Easier: manually add a cache-busting query string here: https://github.com/metabase/metabase/blob/master/frontend/src/card/card.module.js#L13
  2. Better: set up partials to be loaded through Webpack's file-loader (doesn't bundle them, but can do cache busting I think)

@agilliland
Copy link
Contributor Author

I'm fine with taking the easy approach here and calling it done. The real fix can come when we move this stuff over to redux and don't have any other external templates to load.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type:Bug Product defects
Projects
None yet
Development

No branches or pull requests

3 participants