openfisca / legislation-explorer

Explore legislation formulas and parameters.
https://legislation.demo.openfisca.org
GNU Affero General Public License v3.0
26 stars 12 forks source link

Links on search results have different paths on server and client side rendering #202

Closed Morendil closed 5 years ago

Morendil commented 6 years ago

Initial title: "Intermittent 404 on following links to variables pages"

(As observed & reproduced on 2 different machines during Openfisca demo 2018-10-03)

sandcha commented 6 years ago

This is probably linked to #200 (also intermittent) bug fix. 😞

Morendil commented 6 years ago

As a rule of thumb, I keep "it's no longer happening" as a last resort for closing a bug. It tends to be a self-defeating prophecy. Better in most cases to understand precisely what was causing a bug in the first place (being able to repro reliably is a good indicator) and fixing with certainty.

I'm getting the opposite behaviour on my home computer (Mac mini, OSX 10.13.6, Chrome 68), I'm trying hard to get the correct links but getting only links that are missing the base path /legislation/ - all variable links are 404s. (Edit: nope, what's actually happening is that I get 404s systematically when I use Alt-Click to open links in a new tab, but I land on the correct page when I just click.)

The variability based on which computer I use suggests a race condition, with a likely culprit having to do with the History/Router linkup.

Morendil commented 6 years ago

This seems to have something to do with server-side rendering (which I wasn't aware we were using).

The provision for a "base path" (in the case of our production, /legislation) seems to be interpreted differently on server-side and client-side rendering. This (not a race condition) would explain why the links have the correct paths when the search results list is re-rendered after pressing the Back button from visiting a variable, entity or parameter page.

The server-side rendering can be corrected by altering handleRender in render.jsx as follows:

export default function handleRender(state) {
  const pathname = process.env.PATHNAME || ''
  return function (req, res) {
    state.locale = getLocale(req.headers['accept-language'], state.messages)

    match({routes, location: req.url, basename: pathname}, (error, redirectLocation, renderProps) => {

However that still does not explain the variability in behaviour - sometimes clicking on a variable/entity/parameter link will land on a 404, sometimes it will render the expected component chain (Home/CountryModelItem).

I don't (yet) understand how we serve 404s. The 404 page we've been seeing does not seem to be implemented in React inside the legislation-explorer repo, I'm guessing it's part of our production setup.

Morendil commented 6 years ago

However that still does not explain the variability in behaviour

I just got an Aha! on this just after writing it. I'll systematically get sent to a 404 if I click on a link while the page is still loading (in Chrome, while the spinner is still spinning inside the tab title). It's not intermittent but quite deterministic. :)

I'm not 100% sure this is correct but my guess is that at the end of the page load the server-side rendering gets replaced with its "hydrated" version (where the properties come from the client-side generated components which have the correct basename). Until this happens, a click on a link just goes to the href of the link, which is incorrect for the reasons above.

I could still be wrong but I'm now reasonably confident that fixing the server-side broken basename will fix the bug.