live-server icon indicating copy to clipboard operation
live-server copied to clipboard

On 404, send a meta refresh header

Open mgsloan opened this issue 6 years ago • 6 comments

Workaround for 404s causing stuckness

With the static site generator I'm using, other paths get changed before the html of the page I'm viewing. This would cause the page to reload, but before the html file has been written. The html file is deleted at the beginning of site regeneration, so this would cause a 404 error. Since the 404 does not have the change watching script, the browser would get stuck and no longer show a live preview.

I experimented with sending the 404 with the change watching script. However, this often does not work properly. What would happen is that the file would get created with size 0, and would be read before it is populated. This 0 size file would then get served, and this would be another stuckness state.

This is a hacky solution, and I am not very familiar with node. It works well enough for my usecase, though, so I figured I would open a PR.

Race condition..

Unfortunately, serving the change watching script when the file is 0 size is not sufficient. This is because there is a race condition where file changes are ignored during reload.

It looks to me like the implementation of live-server does not have good guarantees of promptly showing the most recent version. In other words, it is straightforward to trigger a scenario where the browser is viewing an old version of the code. Here's what happens:

  1. A change occurs, notifying the client to reload
  2. The client reloads
  3. The user makes a change
  4. The reload script begins watching for changes again

Between (1) and (4), there is a gap where the client is not made aware of changes, and so the user's change in (3) will be ignored. My suggestion would be to either add incrementing counters or timestamps to the protocol. On load of the page, the client should immediately reload if it is already out of date.

mgsloan avatar Jun 10 '18 19:06 mgsloan

Also, it looks to me like the content-length value computed in inject is wrong, because it does not take into account the length of the tag that is being substituted.

I can open issues for these things, if you would prefer that to feedback inline in PR

mgsloan avatar Jun 10 '18 19:06 mgsloan

Another potential solution might be to avoid injection entirely, and serve some HTML that has a fullscreen iframe which will load the content. Then the outer html can have a persistent websocket that does not disconnect, and can cause the iframe to refresh.

mgsloan avatar Jun 10 '18 19:06 mgsloan

FWIW I have decided to switch to https://www.npmjs.com/package/livereload - using a chrome extension is a more reliable way of subscribing to changes - in lieu of a more complicated protocol. Not trying to be snarky! live-server seems pretty decent.

mgsloan avatar Jun 10 '18 19:06 mgsloan

@mgsloan I tried to reproduce the race unsuccessfully. I set up a small project:

https://github.com/sanji-programmer/live-server-potential-race

I used a tool I've worked with to test different schedules of the event loop, but the race you described does not show up. Could you take a look if the test case in the project is what you meant.

sanji-programmer avatar Jul 12 '19 08:07 sanji-programmer

@sanji-programmer Hey, I really appreciate that you are digging into this. The context of my issue was https://github.com/mgsloan/mgsloan-site , where the static site generator output directory was being deleted before being recreated. The directory was being served by python3 -m http.server, and livereload was also running.

The deletion of the directory would trigger live reload, which caused a browser reload before the directory got repopulated. This would result in a 404. The race is that livereload does not cause a reload once the directory gets repopulated. I haven't dug deeply into it, but I'm thinking my proposed PR here may not be the proper solution, instead just a patch for this case.

Thanks! -Michael

mgsloan avatar Jul 12 '19 21:07 mgsloan

@mgsloan I just added two tests:

  • one that removes and recreates a file (https://github.com/sanji-programmer/live-server-potential-race/blob/master/race-file-rem-test.js)
  • one that removes and recreates the root dir (https://github.com/sanji-programmer/live-server-potential-race/blob/master/race-root-dir-test.js)

These tests can reproduce the issue with the 404 error. For trivial runs of these tests, your fix works.

Yet, it seems that some race remains. Only changing the file (https://github.com/sanji-programmer/live-server-potential-race/blob/master/race-test.js) and using the tool I've worked was not enough to reveal the race. Nevertheless, for the tests that remove/recreate file/directory, there are some callback interleaving in which the last updated is not loaded (even with your fix). I could go as far as to notice that it is some callback related to Chokidar. If some maintainer is interested, we can go in detail with our tool's outputs.

sanji-programmer avatar Sep 17 '19 09:09 sanji-programmer