Skip to content

Commit 6a532c6

Browse files
authored
fix: refresh and ttl edgecases (#841)
* fix: hard refresh uses service-worker * test: remove unused FrameLocator types * fix: registrationTTL unregisters sw after returning response * fix: ttl expiry race condition, subdomain+path hard refresh * fix: config page request recognition & e2e tests * fix: remove unnecessary renderUI call
1 parent a6a99d6 commit 6a532c6

File tree

9 files changed

+142
-25
lines changed

9 files changed

+142
-25
lines changed

src/index.tsx

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,24 +23,49 @@ async function renderUi (): Promise<void> {
2323

2424
async function main (): Promise<void> {
2525
if (!('serviceWorker' in navigator)) {
26-
// no service worker, render the UI
26+
// no service worker support, render the UI
2727
await renderUi()
2828
return
2929
}
3030

3131
const url = new URL(window.location.href)
3232
const state = await getStateFromUrl(url)
3333

34-
if (!state.requestForContentAddressedData) {
34+
if (!state.requestForContentAddressedData || state.isConfigRequest) {
3535
// not a request for content addressed data, render the UI
3636
await renderUi()
3737
return
3838
} else if (state.hasConfig) {
39-
// user is requesting content addressed data and has the config already, render the UI
40-
await renderUi()
39+
// user is requesting content addressed data and has the config already
40+
// we need to ensure a SW is registered and let it handle the request
41+
const translatedUrl = translateIpfsRedirectUrl(url)
42+
if (state.isHardRefresh) {
43+
// this is a hard refresh, we need to reload to ensure the service worker captures the request.
44+
window.location.replace(translatedUrl.href) // replace with translated URL to remove helia-sw param
45+
return
46+
}
47+
// else, there is some other reason why sw didn't capture the request, ensure sw is registered and reload
48+
try {
49+
await ensureSwScope()
50+
await registerServiceWorker()
51+
} catch (err) {
52+
// eslint-disable-next-line no-console
53+
console.error('helia:sw-gateway:index: error ensuring sw scope and registration', err)
54+
}
55+
window.location.replace(translatedUrl.href) // replace with translated URL to remove helia-sw param
4156
return
4257
}
43-
// the user is requesting content addressed data and does not have the config, continue with the config loading flow
58+
59+
/**
60+
* **********************************************
61+
* From here on, we are handling the case where:
62+
* - the user is requesting content addressed data
63+
* - the user does not have the config
64+
*
65+
* We need to load the config, which may involve a redirect to the root domain
66+
* before the service worker can be registered
67+
* ***********************************************
68+
*/
4469
loadingIndicatorElement?.classList.remove('hidden')
4570

4671
if (state.hasConfig && state.isIsolatedOrigin) {

src/lib/constants.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,11 @@ export const QUERY_PARAMS = {
1212
}
1313

1414
export const HASH_FRAGMENTS = {
15+
/**
16+
* The hash fragment that is used to request the UI config page.
17+
*/
18+
IPFS_SW_CONFIG_UI: 'ipfs-sw-config',
19+
1520
/**
1621
* The hash fragment that is used to send the config to the subdomain service worker.
1722
*/

src/lib/first-hit-helpers.ts

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,24 @@ interface NavigationState {
1919
/**
2020
* If the user is requesting content addressed data (instead of service worker gateway UI), this will be true.
2121
*
22-
* e.g. sw-gateway landing page, ipfs-sw-config view, ipfs-sw-origin-isolation-warning view, etc.
22+
* e.g. NOT the sw-gateway landing page, ipfs-sw-config view, ipfs-sw-origin-isolation-warning view, etc.
2323
*/
2424
requestForContentAddressedData: boolean
25+
26+
/**
27+
* If the user is requesting content addressed data, the current URL is an isolated subdomain, and we already have the config,
28+
* this will be true iff navigator.serviceWorker.controller is null.
29+
*
30+
* In this case, we need to reload the page to ensure the service worker captures the request.
31+
*
32+
* @see https://www.w3.org/TR/service-workers/#dom-serviceworkercontainer-controller
33+
*/
34+
isHardRefresh: boolean
35+
36+
/**
37+
* If the user is requesting the config, this will be true.
38+
*/
39+
isConfigRequest: boolean
2540
}
2641

2742
const log = uiLogger.forComponent('first-hit-helpers')
@@ -135,11 +150,6 @@ function isRequestForContentAddressedData (url: URL): boolean {
135150
// query param request
136151
return true
137152
}
138-
if (url.hash.includes('/ipfs-sw-config')) {
139-
// hash request for UI page, with no path/subdomain/query indicating request for content addressed data
140-
// we need to do this after the path/subdomain/query check because we need to ensure the config is properly loaded from the root domain
141-
return false
142-
}
143153
return false
144154
}
145155

@@ -154,10 +164,28 @@ export async function getStateFromUrl (url: URL): Promise<NavigationState> {
154164
const urlHasSubdomainConfigRequest = hasHashFragment(url, HASH_FRAGMENTS.IPFS_SW_SUBDOMAIN_REQUEST) && url.searchParams.get(QUERY_PARAMS.HELIA_SW) != null
155165
let hasConfig = false
156166
const supportsSubdomains = await checkSubdomainSupport(url)
167+
let isHardRefresh = false
168+
let isConfigRequest = false
157169

158170
if (isIsolatedOrigin) {
159171
// check if indexedDb has config
160172
hasConfig = await isConfigSet(uiLogger)
173+
if (hasConfig) {
174+
// Check service worker state
175+
const registration = await navigator.serviceWorker.getRegistration()
176+
const hasActiveWorker = registration?.active != null
177+
const hasControllingWorker = navigator.serviceWorker.controller != null
178+
179+
if (hasActiveWorker && !hasControllingWorker) {
180+
// this is a hard refresh
181+
isHardRefresh = true
182+
}
183+
}
184+
}
185+
186+
// check if url.hash matches exactly
187+
if (url.hash === `#/${HASH_FRAGMENTS.IPFS_SW_CONFIG_UI}`) {
188+
isConfigRequest = true
161189
}
162190

163191
return {
@@ -168,7 +196,9 @@ export async function getStateFromUrl (url: URL): Promise<NavigationState> {
168196
subdomainParts: { parentDomain, id, protocol },
169197
compressedConfig: getHashFragment(url, HASH_FRAGMENTS.IPFS_SW_CFG),
170198
supportsSubdomains,
171-
requestForContentAddressedData: isRequestForContentAddressedData(url)
199+
requestForContentAddressedData: isRequestForContentAddressedData(url),
200+
isHardRefresh,
201+
isConfigRequest
172202
} satisfies NavigationState
173203
}
174204

src/lib/get-subdomain-parts.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,23 @@
11
import { dnsLinkLabelDecoder, isInlinedDnsLink } from './dns-link-labels.js'
22

33
export interface UrlParts {
4+
/**
5+
* The CID or DNSLink name or peer ID.
6+
*
7+
* e.g. `bafyfoo` for `bafyfoo.ipfs.example.com`
8+
*/
49
id: string | null
10+
/**
11+
* The protocol of the subdomain.
12+
*
13+
* e.g. `ipfs` for `bafyfoo.ipfs.example.com` or `ipns` for `bafyfoo.ipns.example.com`
14+
*/
515
protocol: string | null
16+
/**
17+
* The parent domain of the subdomain.
18+
*
19+
* e.g. `example.com` for `bafyfoo.ipfs.example.com`
20+
*/
621
parentDomain: string
722
}
823

src/sw.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,19 @@ self.addEventListener('fetch', (event) => {
188188
event.waitUntil(requestRouting(event, url).then(async (shouldHandle) => {
189189
if (shouldHandle) {
190190
log.trace('handling request for %s', url)
191-
event.respondWith(getResponseFromCacheOrFetch(event))
191+
event.respondWith(getResponseFromCacheOrFetch(event).then(async (response) => {
192+
if (!isServiceWorkerRegistrationTTLValid()) {
193+
log('Service worker registration TTL expired, unregistering service worker')
194+
const clonedResponse = response.clone()
195+
event.waitUntil(
196+
clonedResponse.blob().then(() => {
197+
log('Service worker registration TTL expired, unregistering after response consumed')
198+
}).finally(() => self.registration.unregister())
199+
)
200+
return response
201+
}
202+
return response
203+
}))
192204
} else {
193205
log.trace('not handling request for %s', url)
194206
}
@@ -201,11 +213,7 @@ self.addEventListener('fetch', (event) => {
201213
******************************************************
202214
*/
203215
async function requestRouting (event: FetchEvent, url: URL): Promise<boolean> {
204-
if (!isServiceWorkerRegistrationTTLValid()) {
205-
log.trace('Service worker registration TTL expired, unregistering service worker')
206-
event.waitUntil(self.registration.unregister())
207-
return false
208-
} else if (isUnregisterRequest(event.request.url)) {
216+
if (isUnregisterRequest(event.request.url)) {
209217
event.waitUntil(self.registration.unregister())
210218
event.respondWith(new Response('Service worker unregistered', {
211219
status: 200
@@ -759,6 +767,10 @@ function isServiceWorkerRegistrationTTLValid (): boolean {
759767
/**
760768
* When we unregister the service worker, the a new one will be installed on the next page load.
761769
*
770+
* Note: returning true here means if the user is not online, we will not unregister the service worker.
771+
* However, browsers will have `navigator.onLine === true` if connected to a LAN that is not internet-connected,
772+
* so we may want to be smarter about this in the future.
773+
*
762774
* @see https://github.com/ipfs/service-worker-gateway/issues/724
763775
*/
764776
return true

test-e2e/fixtures/capture-all-sw-responses.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ export async function * captureAllSwResponses (page: Page, signal: AbortSignal):
3232
responseQueue.push(response)
3333
}
3434
}
35-
3635
page.on('response', onResponse)
3736

3837
try {

test-e2e/fixtures/locators.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
1-
import type { FrameLocator, Locator, Page } from '@playwright/test'
1+
import type { Locator, Page } from '@playwright/test'
22

33
export interface GetLocator {
4-
(page: Page | FrameLocator): Locator
5-
}
6-
export interface GetFrameLocator {
7-
(page: Page | FrameLocator): FrameLocator
4+
(page: Page): Locator
85
}
96

107
/**

test-e2e/layout.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ test.describe('smoketests', () => {
3838
testSubdomainRouting.describe('smoketests', () => {
3939
testSubdomainRouting.describe('config section on subdomains', () => {
4040
// TODO: remove this test because we don't want to support config page on subdomains. See
41-
testSubdomainRouting('only config and header are visible on /#/ipfs-sw-config requests', async ({ page, baseURL, rootDomain, protocol }) => {
41+
testSubdomainRouting('only config and header are visible on /#/ipfs-sw-config requests', async ({ page, baseURL, rootDomain, protocol, browserName }) => {
42+
testSubdomainRouting.skip(browserName === 'firefox', 'Playwright doesn\'t treat hard refreshes correctly in Firefox.')
4243
await page.goto(baseURL, { waitUntil: 'networkidle' })
4344
await waitForServiceWorker(page, baseURL)
4445
await page.goto(`${protocol}//bafkqablimvwgy3y.ipfs.${rootDomain}/#/ipfs-sw-config`, { waitUntil: 'networkidle' })

test-e2e/smoke-test.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,4 +101,37 @@ test.describe('smoke test', () => {
101101

102102
expect(noRegistration).toBe(true)
103103
})
104+
105+
test('service worker unregisters automatically when ttl expires', async ({ page, baseURL, protocol, rootDomain, swResponses }) => {
106+
await page.goto(baseURL, { waitUntil: 'networkidle' })
107+
await waitForServiceWorker(page, baseURL)
108+
// set the ttl in milliseconds
109+
await setConfig({ page, config: { serviceWorkerRegistrationTTL: 1400 } })
110+
111+
await page.goto(`${protocol}//bafybeib3ffl2teiqdncv3mkz4r23b5ctrwkzrrhctdbne6iboayxuxk5ui.ipfs.${rootDomain}/root2/root3/root4/`, { waitUntil: 'networkidle' })
112+
await waitForServiceWorker(page, baseURL)
113+
expect(swResponses.length).toBe(1)
114+
const response = swResponses[swResponses.length - 1]
115+
expect(response?.status()).toBe(200)
116+
expect(response?.headers()['content-type']).toBe('text/html; charset=utf-8')
117+
expect(await response?.text()).toContain('hello')
118+
119+
// wait for the ttl to expire
120+
await page.waitForTimeout(1500)
121+
122+
const response2 = await page.reload({ waitUntil: 'networkidle' })
123+
expect(swResponses.length).toBe(2)
124+
expect(response2?.status()).toBe(200)
125+
expect(response2?.headers()['content-type']).toBe('text/html; charset=utf-8')
126+
expect(await response2?.text()).toContain('hello')
127+
128+
// wait for the TTL invalid setTimeout to run.
129+
await page.waitForTimeout(100)
130+
131+
const noServiceWorkerRegistration = await page.evaluate(async () => {
132+
return await window.navigator.serviceWorker.getRegistration() === undefined
133+
})
134+
135+
expect(noServiceWorkerRegistration).toBe(true)
136+
})
104137
})

0 commit comments

Comments
 (0)