Paths

Table of Contentst

[web] make sure all tabs and worker are operating on the same web version
ClosedPublic
Actions

Authored by kamil on Jan 10 2024, 5:42 AM.

Details

Reviewers

tomek
michal

Commits

rCOMM7a071de9cb5b: [web] make sure all tabs and worker are operating on the same web version

Summary

ENG-6326

The initial solution I was thinking of is:

Store content hash (of .wasm file) in worker memory.
Each time a new tab is opened compare content hashes.
On difference kill the worker and initialize again to fetch a new bundle.
Reload old tabs.

A couple of issues with this:

There is no easy way to terminate a worker. Terminate does not work for Shared Workers, close only clear task queue, and does not cause re-fetching worker script.
We can close all tabs, leave only the newest one, and refresh - this doesn't make any sense.
We can refresh all tabs at once - this is very hard to implement because it must be done simultaneously. There is a risk of race condition, when with multiple tabs first already finished refreshing when the latest has not started yet.

That being said, I am proposing a simpler and safer solution:

We create a worker, which name is associated with the web version - when we open a new tab with the new version we will create new Shared worker with the newest bundle (which is in sync with main thread and .wasm file).
We broadcast (supported by all browsers) current version to other tabs.
When we receive the version we compare it, if it's different this tab is old, we refresh it - download the newest web app version, and connect to the new worker.
After all tabs are reloaded, the old worker dies (there is no connection to it).
When we open a new tab with the new version, old tabs are in the background so it should be safe because the worker should not be performing anything on DB.

Test Plan

Run prod web app (hot reloading messes up this test)
Open a couple of tabs
Bump web codeVersion
Run the prod web app again
Open a new tab, which should cause reloading old tabs, bumping spawning new worker with a name including new version, killing an old worker.
Inspect the network tab to make sure that to create a worker with a new name script is fetched.
Tested on Chrome, Firefox, and Safari (on Safari there is no way to inspect workers, so I based this test only on information on whether old tabs were reloaded).

Diff Detail

Repository

rCOMM Comm

Branch

fix-worker

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

kamil created this revision.Jan 10 2024, 5:42 AM

kamil held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptJan 10 2024, 5:42 AM

kamil published this revision for review.Jan 10 2024, 5:56 AM

kamil added inline comments.

web/database/database-module-provider.js
156–157	I had to move it, while this started to work config was not yet set

Is there a risk of us making the user lose some work (or lose the navigation state) by refreshing? Wondering if you've considered potential solutions that don't require a refresh. I'm guessing that would not work well because you'd either have two workers modifying the same state, or you'd have a single worker trying to interact with two different version tabs (which is what got us into this mess in the first place)

Harbormaster completed remote builds in B25658: Diff 35475.Jan 10 2024, 6:06 AM

In D10592#306475, @ashoat wrote:

Is there a risk of us making the user lose some work (or lose the navigation state) by refreshing?

The entire state (drafts, etc.) is persisted, when the old tab gets refreshed the state shouldn't disappear (I tested this).

Wondering if you've considered potential solutions that don't require a refresh. I'm guessing that would not work well because you'd either have two workers modifying the same state, or you'd have a single worker trying to interact with two different version tabs (which is what got us into this mess in the first place)

Exactly, those are reasons why we need to refresh to match old tabs to the newest as there is one database for all.

The entire state (drafts, etc.) is persisted, when the old tab gets refreshed the state shouldn't disappear (I tested this).

This doesn't include everything... for instance, imagine the user had a modal open, and they were working on editing a thread's description. The edited description would be lost in that scenario, right?

Wondering if you've considered potential solutions that don't require a refresh. I'm guessing that would not work well because you'd either have two workers modifying the same state, or you'd have a single worker trying to interact with two different version tabs (which is what got us into this mess in the first place)

Exactly, those are reasons why we need to refresh to match old tabs to the newest as there is one database for all.

I'm wondering if there is a better "best practice" for this situation. I can't imagine that we're the first to deal with this sort of scenario. What do other web apps do?

Ultimately I don't want to block this diff, but I would like to understand a bit more about the problem space, so we can confirm that the tradeoff is worth taking.

When we receive the version we compare it, if it's different this tab is old, we refresh it - download the newest web app version, and connect to the new worker.

Are there any risks here? Can the refresh fail, or be stopped by a user?

In D10592#306527, @ashoat wrote:

The entire state (drafts, etc.) is persisted, when the old tab gets refreshed the state shouldn't disappear (I tested this).

This doesn't include everything... for instance, imagine the user had a modal open, and they were working on editing a thread's description. The edited description would be lost in that scenario, right?

The probability of this scenario is low because we're not updating the app frequently, but it's still possible. Some ideas we can consider:

Persist all the state, including pending edit state.
Instead of refreshing, show a modal in a tab with the new version, describing the situation and allowing the user to 1. continue the work on the older tabs 2. updating the older tabs which could make some work lost. This modal would block any interaction with the new tab.

For me, 1 sounds like an overkill, and would require more decisions about e.g. backup. 2 Migh be challenging from the technical point of view, and could cause some confusion to a user.

Overall, this diff is a big improvement compared to the current state, where the conflict is causing a crash, which definitely makes the user's work lost.

When we open a new tab with the new version, old tabs are in the background so it should be safe because the worker should not be performing anything on DB.

Websocket still does work for some time after a tab is backgrounded right? If user open the new tab fast enough could there be an update for the old tab that triggers DB operation? (that's probably very unlikely so I still think that would be fine)

An alternative solution to the (2) point that @tomek proposed would be using the ServiceWorker Cache and Fetch APIs to cache the old web app (main js bundle, worker bundles and .wasm files). Then if a user opens a new tab while there is another tab open with an older version, instead of showing a modal that blocks all interaction, we would return the cached older version. We would then display some kind of popup which would have a button that would allow the user to safely reload all tabs to the new version.

This would have the benefit of making the new tab still usable. Also if we added an option to always display the refresh popup even if this is the only tab and code signed the the web app it would allow us to potentially solve ENG-2988 : Mechanism for verifying the integrity of a web app?

This revision is now accepted and ready to land.Jan 11 2024, 3:45 AM

In D10592#306527, @ashoat wrote:

The entire state (drafts, etc.) is persisted, when the old tab gets refreshed the state shouldn't disappear (I tested this).

This doesn't include everything... for instance, imagine the user had a modal open, and they were working on editing a thread's description. The edited description would be lost in that scenario, right?

Unfortunately yes

I'm wondering if there is a better "best practice" for this situation. I can't imagine that we're the first to deal with this sort of scenario. What do other web apps do?

I can imagine a couple of possibilities:

Solution we have but more gentle, without reloading tabs but presenting message/alert/modal to the user and letting them decide what to do (described by @tomek).
Using Service Worker to serve the cached app version until the user wants to upgrade to a newer version (described by @michal).
Allow using only one tab at a time, I've seen some apps showing the message "you have the app opened in another tab", this could simplify a lot of things and for me from my personal user's perspective is fine but I guess some people consider this a huge UX regression.

In D10592#306715, @tomek wrote:

When we receive the version we compare it, if it's different this tab is old, we refresh it - download the newest web app version, and connect to the new worker.

Are there any risks here? Can the refresh fail, or be stopped by a user?

I think when the reload is started and will fail app is not usable anyway so the user will have to open the tab and manually refresh it so this shouldn't be a problem.

Created a task to discuss some potential follow-ups in the future: ENG-6460.