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

Offline cache #2508

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions backend/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ route.GET('/assets/{subpath*}', {
// Files like `main.js` or `main.css` should be revalidated before use. Se we use the default headers.
// This should also be suitable for serving unversioned fonts and images.
return h.file(subpath)
.header('Cache-Control', 'public,max-age=604800,stale-while-revalidate=86400')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... why is this being added? I see that we have a special section for cached files above, so why should these be cached too when it seems like these are files that might change?

And if this is really necessary, could you add a comment here explaining why?

I want to avoid the app failing to update when files are updated...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because if we don't store these in the cache, the app can't possibly work offline. Adding stale-while-revalidate means that it's fine to serve stale files while they're being revalidated (normally, all cached requests need to be revalidated).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, the comment is outdated, so I agree that this should be updated.

})

route.GET(staticServeConfig.routePath, {}, {
Expand Down
4 changes: 3 additions & 1 deletion frontend/controller/app/identity.js
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,9 @@ export default (sbp('sbp/selectors/register', {
})
} else {
try {
await sbp('chelonia/contract/sync', identityContractID)
if (navigator.onLine !== false) {
await sbp('chelonia/contract/sync', identityContractID)
}
Comment on lines +391 to +393
Copy link
Member

@taoeffect taoeffect Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So... do we sync this once we do come back online? If so, where?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't explicitly. It'd be handled like any other contract by that point.

} catch (e) {
// Since we're throwing or returning, the `await` below will not
// be used. In either case, login won't complete after this point,
Expand Down
131 changes: 131 additions & 0 deletions frontend/controller/serviceworkers/cache.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import sbp from '@sbp/sbp'
import { NOTIFICATION_TYPE } from '~/shared/pubsub.js'

const CACHE_VERSION = '1.0.0'
const CURRENT_CACHES = {
assets: `assets-cache_v${CACHE_VERSION}`
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this version number be tied to some environment variable, e.g. GI_VERSION or something else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about that. It could, but I don't think it needs to, so long as it's updated if breaking changes to the cache are made.


if (
typeof Cache === 'function' &&
typeof CacheStorage === 'function' &&
typeof caches === 'object' &&
(caches instanceof CacheStorage)
) {
sbp('okTurtles.events/on', NOTIFICATION_TYPE.VERSION_INFO, (data) => {
if (data.GI_VERSION !== process.env.GI_VERSION) {
caches.delete(CURRENT_CACHES.assets).catch(e => {
console.error('Error trying to delete cache', CURRENT_CACHES.assets)
})
}
})

const locationUrl = new URL(self.location)
const routerBase = locationUrl.searchParams.get('routerBase') ?? '/app'

self.addEventListener('install', (event) => {
event.waitUntil(
caches
.open(CURRENT_CACHES.assets)
.then((cache) =>
cache.addAll([
`${routerBase}/`
]).catch(e => {
console.error('Error adding initial entries to cache')
})
)
)
})

// Taken from the MDN example:
// <https://developer.mozilla.org/en-US/docs/Web/API/Cache>
self.addEventListener('activate', (event) => {
const expectedCacheNamesSet = new Set(Object.values(CURRENT_CACHES))

event.waitUntil(
caches.keys().then((cacheNames) =>
Promise.allSettled(
cacheNames.map((cacheName) => {
if (!expectedCacheNamesSet.has(cacheName)) {
// If this cache name isn't present in the set of
// "expected" cache names, then delete it.
console.log('Deleting out of date cache:', cacheName)
return caches.delete(cacheName)
}

return undefined
})
)
)
)
})

self.addEventListener('fetch', function (event) {
console.debug(`[sw] fetch : ${event.request.method} - ${event.request.url}`)

if (!['GET', 'HEAD', 'OPTIONS'].includes(event.request.method)) {
return
}

let request = event.request

try {
const url = new URL(request.url)
if (url.pathname.startsWith('/file')) {
return
}

// If the route starts with `${routerBase}/`, use `${routerBase}/` as the
// URL, since the HTML content is presumed to be the same.
if (url.pathname.startsWith(`${routerBase}/`)) {
request = new Request(`${routerBase}/`, request)
}
} catch (e) {
return
}

event.respondWith(
caches.open(CURRENT_CACHES.assets).then((cache) => {
return cache
.match(request)
.then((cachedResponse) => {
if (cachedResponse) {
// If we're offline, return the cached response, if it exists
if (navigator.onLine === false) {
return cachedResponse
}
}

return fetch(request.clone()).then(async (response) => {
if (
// Save successful reponses
response.status >= 200 &&
response.status < 400 &&
response.status !== 206 && // Partial response
response.status !== 304 && // Not modified
// Which don't have a 'no-store' directive
!response.headers.get('cache-control')?.split(',').some(x => x.trim() === 'no-store')
) {
await cache.put(request, response.clone()).catch(e => {
console.error('Error adding request to cache')
})
} else {
await cache.delete(request)
}

return response
}).catch(e => {
if (cachedResponse) {
console.warn('Error while fetching', request.url, e)
// If there was a network error fetching, return the cached
// response, if it exists
return cachedResponse
}

throw e
})
})
})
)
})
}
5 changes: 1 addition & 4 deletions frontend/controller/serviceworkers/sw-primary.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
NEW_UNREAD_MESSAGES, NOTIFICATION_EMITTED, NOTIFICATION_REMOVED,
NOTIFICATION_STATUS_LOADED, OFFLINE, ONLINE, SERIOUS_ERROR, SWITCH_GROUP
} from '../../utils/events.js'
import './cache.js'
import './push.js'
import './sw-namespace.js'

Expand Down Expand Up @@ -256,10 +257,6 @@ self.addEventListener('activate', function (event) {
event.waitUntil(setupPromise.then(() => self.clients.claim()))
})

self.addEventListener('fetch', function (event) {
console.debug(`[sw] fetch : ${event.request.method} - ${event.request.url}`)
})

// TODO: this doesn't persist data across browser restarts, so try to use
// the cache instead, or just localstorage. Investigate whether the service worker
// has the ability to access and clear the localstorage periodically.
Expand Down