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

Figure out when to sync contracts #2653

Open
taoeffect opened this issue Feb 19, 2025 · 0 comments
Open

Figure out when to sync contracts #2653

taoeffect opened this issue Feb 19, 2025 · 0 comments
Assignees
Labels
App:Frontend Kind:Bug Kind:Core Anything that changes or affects the fundamental core data structures & design of the application.

Comments

@taoeffect
Copy link
Member

taoeffect commented Feb 19, 2025

Problem

In setupChelonia.js this code was commented out:

  /*
  await sbp('gi.db/settings/load', SETTING_CURRENT_USER).then(async (identityContractID) => {
    // This loads CHELONIA_STATE when _not_ running as a service worker
    const cheloniaState = await sbp('chelonia/rootState')
    if (!cheloniaState || !identityContractID) return
    if (cheloniaState.loggedIn?.identityContractID !== identityContractID) return
    // it is important we first login before syncing any contracts here since that will load the
    // state and the contract sideEffects will sometimes need that state, e.g. loggedIn.identityContractID
    await sbp('chelonia/contract/sync', identityContractID).then(async () => {
      const contractIDs = groupContractsByType(cheloniaState.contracts)
      await syncContractsInOrder(contractIDs)
    }).catch(e => {
      console.error('[setupChelonia] Error syncing identity contract and groups', e)
    })
  })
  */

It's since been uncommented and @corrideat says he doesn't like it there because:

The problem is that:
(1) as it stands, anytime the SW is awoken it'll re-sync all contracts. This is bad because it results in unnecessary network activity (imagine you get a notification and you sync entire chatrooms). It's also bad for snapshots and for building an offline PWA.
(2) however, if we remove it, notifications break (as they are now), because not subscribing to a contract (over WS) means that we'll stop getting push events for them later
(3) fixing (2) is complicated by the fact that if we don't sync a contract, getting notifications or other events for them isn't very useful, as a sync is needed.
I'm leaning towards a 'lazy sync' approach, using different strategies based on the situation:
For regular SW wake up calls, the contract will merely be marked as a 'contract of interest', but we won't get any events for it, unless we manually sync it.
For app loads (using the browser), a full sync is done, much like now, except that in a way that's compatible both with local snapshots and the offline PWA
More specific details are still a WIP

I asked in a followup why in (1) @corrideat said "and for building an offline PWA." His reply:

Because the SW will be awoken but it's offline, so a sync isn't possible?

So we should look into detecting whether or not we're online before doing that sync, and then if we aren't, doing it once we come back online.

Solution

It needs to be uncommented but also rewritten to take into account the changes coming in with user state snapshots and the offline cache stuff.

@taoeffect taoeffect added App:Frontend Kind:Bug Kind:Core Anything that changes or affects the fundamental core data structures & design of the application. Priority:High labels Feb 19, 2025
taoeffect added a commit that referenced this issue Feb 20, 2025
taoeffect added a commit that referenced this issue Feb 22, 2025
* misc fixes and improvements

* many fixes and improvements to all the notification things, mostly frontend

* improve logging

* log the subscription ID on both the frontend and backend

* improved logging

* some bugfixes and improvements to backend/push.js

* feedback

* remove extraneous call to setupChelonia()

* minor improvements

* address all remaining issue. tested in Safari + Brave + FF

* prevent log spam

* oops

* update strings

* possibly address #2653

* ensure local Cypress tests don't get tripped by by push issues

* react to notification permission changes in Safari

* improvement to syncContractsInOrder to prevent syncing removed contracts

* feedback

* feedback: syncContractsInOrder should attempt to sync all contracts

* improve error message

* improve logging in syncContractsInOrder

* webkit bugfix + throttle handler for Chrome

* move back the call to syncContractsInOrder where it was

* handle 'WebSocket connection is not open' on 'push/reportExistingSubscription'

* better comments + logging

* improve gi.ui prompt strings

* remove redundant alert() + improve error logging

* Remove Notification object on Cypress

* feedback

* created ''sw/deviceSettings/*' and used it for handling DISABLE_NOTIFICATIONS

* final feedback

---------

Co-authored-by: Ricardo Iván Vieitez Parra <[email protected]>
@taoeffect taoeffect changed the title Figure out when to sync contracts to fix chatrooms not being synced and missed notifications Figure out when to sync contracts Feb 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
App:Frontend Kind:Bug Kind:Core Anything that changes or affects the fundamental core data structures & design of the application.
Projects
None yet
Development

No branches or pull requests

2 participants