Skip to content

Commit 0fadef5

Browse files
committed
ui: move caching to the extension server code
I was being silly - there's no reason to do the caching in the service worker, we can just do it in the extension server code directly which completely negates the need to do any subtle workaround CORS etc.
1 parent c3cc182 commit 0fadef5

2 files changed

Lines changed: 49 additions & 72 deletions

File tree

ui/src/core_plugins/dev.perfetto.ExtensionServers/extension_server.ts

Lines changed: 49 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ import {joinPath} from './url_utils';
3838

3939
const FETCH_TIMEOUT_MS = 10000; // 10 seconds
4040

41-
function extensionHeaders(): Record<string, string> {
42-
// TODO(lalitm): readd caching once we've figured out a more correct solution.
43-
return {};
44-
}
41+
// Cache name used by the client-side extension cache. Responses are stored
42+
// here keyed by URL so that when the network is unreachable we can serve
43+
// the last-known-good data without involving the service worker or any
44+
// custom request headers.
45+
const EXT_CACHE_NAME = 'extension-servers';
4546

4647
interface FetchRequest {
4748
url: string;
@@ -66,7 +67,6 @@ export function buildFetchRequest(
6667
const headers: Record<string, string> = {
6768
Accept: 'application/vnd.github.raw+json',
6869
Authorization: `token ${server.auth.pat}`,
69-
...extensionHeaders(),
7070
};
7171
return {url, init: {method: 'GET', headers}};
7272
}
@@ -76,10 +76,7 @@ export function buildFetchRequest(
7676
const url =
7777
`https://raw.githubusercontent.com/${server.repo}` +
7878
`/${encodeURIComponent(server.ref)}/${encodedPath}`;
79-
const headers: Record<string, string> = {
80-
...extensionHeaders(),
81-
};
82-
return {url, init: {method: 'GET', headers}};
79+
return {url, init: {method: 'GET'}};
8380
}
8481

8582
// HTTPS servers — normalize URL in case https:// is missing.
@@ -88,9 +85,7 @@ export function buildFetchRequest(
8885
baseUrl = `https://${baseUrl}`;
8986
}
9087
const url = `${baseUrl.replace(/\/+$/, '')}/${path}`;
91-
const headers: Record<string, string> = {
92-
...extensionHeaders(),
93-
};
88+
const headers: Record<string, string> = {};
9489
if (server.auth.type === 'https_basic') {
9590
const credentials = `${server.auth.username}:${server.auth.password}`;
9691
headers['Authorization'] = `Basic ${base64Encode(utf8Encode(credentials))}`;
@@ -137,6 +132,46 @@ function refreshSsoCookie(url: string): Promise<boolean> {
137132
});
138133
}
139134

135+
// Network-first fetch with client-side Cache API fallback.
136+
// On success the response body is cached keyed by URL. On network failure
137+
// or timeout, the last-known-good cached response is returned instead.
138+
//
139+
// HTTP errors (e.g. 403, 404) are intentionally NOT served from cache:
140+
// they indicate a real server-side rejection and the caller needs to see
141+
// them (e.g. the SSO 403-retry logic in fetchJson).
142+
async function fetchWithCache(
143+
url: string,
144+
init: RequestInit,
145+
): Promise<Response> {
146+
// Open the cache once; if the Cache API is unavailable (e.g. in tests)
147+
// we fall back to plain fetch with no caching.
148+
let cache: Cache | undefined;
149+
try {
150+
cache = await caches.open(EXT_CACHE_NAME);
151+
} catch {
152+
// Cache API unavailable — proceed without caching.
153+
}
154+
155+
try {
156+
const response = await fetchWithTimeout(url, init, FETCH_TIMEOUT_MS);
157+
if (response.ok) {
158+
// Cache successful responses for offline use.
159+
await cache?.put(url, response.clone());
160+
} else {
161+
// Invalidate on HTTP errors (e.g. 403 revoked access, 404 deleted
162+
// resource). The caller still sees the error so it can react
163+
// (e.g. SSO 403-retry in fetchJson).
164+
await cache?.delete(url);
165+
}
166+
return response;
167+
} catch (networkErr) {
168+
// Network or timeout error — serve from cache if available.
169+
const cached = await cache?.match(url);
170+
if (cached) return cached;
171+
throw networkErr;
172+
}
173+
}
174+
140175
async function fetchJson<T extends z.ZodTypeAny>(
141176
server: UserInput,
142177
path: string,
@@ -145,7 +180,7 @@ async function fetchJson<T extends z.ZodTypeAny>(
145180
const req = buildFetchRequest(server, path);
146181
let response: Response;
147182
try {
148-
response = await fetchWithTimeout(req.url, req.init, FETCH_TIMEOUT_MS);
183+
response = await fetchWithCache(req.url, req.init);
149184
} catch (e) {
150185
return errResult(`Failed to fetch ${req.url}: ${e}`);
151186
}
@@ -164,7 +199,7 @@ async function fetchJson<T extends z.ZodTypeAny>(
164199
const refreshed = await refreshSsoCookie(baseUrl.replace(/\/+$/, ''));
165200
if (refreshed) {
166201
try {
167-
response = await fetchWithTimeout(req.url, req.init, FETCH_TIMEOUT_MS);
202+
response = await fetchWithCache(req.url, req.init);
168203
} catch (e) {
169204
return errResult(`Failed to fetch ${req.url} after SSO refresh: ${e}`);
170205
}

ui/src/service_worker/service_worker.ts

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -45,17 +45,12 @@ export {};
4545

4646
const LOG_TAG = `ServiceWorker: `;
4747
const CACHE_NAME = 'ui-perfetto-dev';
48-
const EXT_CACHE_NAME = 'extension-servers';
49-
const EXTENSION_HEADER = 'X-Perfetto-Extension';
5048
const OPEN_TRACE_PREFIX = '/_open_trace'
5149

5250
// If the fetch() for the / doesn't respond within 3s, return a cached version.
5351
// This is to avoid that a user waits too much if on a flaky network.
5452
const INDEX_TIMEOUT_MS = 3000;
5553

56-
// Timeout for extension server fetches.
57-
const EXT_FETCH_TIMEOUT_MS = 10000;
58-
5954
// Use more relaxed timeouts when caching the subresources for the new version
6055
// in the background.
6156
const INSTALL_TIMEOUT_MS = 30000;
@@ -216,12 +211,6 @@ self.addEventListener('fetch', (event) => {
216211
return;
217212
}
218213

219-
// Extension server requests: bypass normal handling, cache responses.
220-
if (event.request.headers.get(EXTENSION_HEADER)) {
221-
event.respondWith(handleExtensionRequest(event.request));
222-
return;
223-
}
224-
225214
// The early return here will cause the browser to fall back on standard
226215
// network-based fetch.
227216
if (!shouldHandleHttpRequest(event.request)) {
@@ -248,53 +237,6 @@ function shouldHandleHttpRequest(req: Request): boolean {
248237
return req.method === 'GET' && url.origin === self.location.origin;
249238
}
250239

251-
// Handles extension server requests marked with X-Perfetto-Extension.
252-
// Strips the marker header, fetches with network-first strategy, and caches
253-
// successful responses. On network failure, serves from cache.
254-
async function handleExtensionRequest(req: Request): Promise<Response> {
255-
// Strip marker header before forwarding to the network.
256-
const headers = new Headers(req.headers);
257-
headers.delete(EXTENSION_HEADER);
258-
const cleanReq = new Request(req.url, {
259-
method: req.method,
260-
headers,
261-
mode: req.mode,
262-
credentials: req.credentials,
263-
});
264-
265-
try {
266-
const response = await fetchWithTimeout(cleanReq, EXT_FETCH_TIMEOUT_MS);
267-
// fetchWithTimeout only resolves for ok responses — cache it.
268-
try {
269-
const cache = await caches.open(EXT_CACHE_NAME);
270-
await cache.put(req.url, response.clone());
271-
} catch {
272-
// Cache write failure is non-fatal.
273-
}
274-
return response;
275-
} catch (e) {
276-
// NetworkError or TimeoutError — serve from cache.
277-
// HTTP errors (403, 404) are plain Error — return directly.
278-
if (!(e instanceof NetworkError) && !(e instanceof TimeoutError)) {
279-
return new Response(String(e), {status: 502, statusText: 'Bad Gateway'});
280-
}
281-
try {
282-
const cache = await caches.open(EXT_CACHE_NAME);
283-
const cached = await cache.match(req.url);
284-
if (cached) {
285-
console.info(LOG_TAG + `serving ${req.url} from extension cache`);
286-
return cached;
287-
}
288-
} catch {
289-
// Cache read failure.
290-
}
291-
return new Response('Extension server unreachable and no cached data', {
292-
status: 503,
293-
statusText: 'Service Unavailable',
294-
});
295-
}
296-
}
297-
298240
async function handleHttpRequest(req: Request): Promise<Response> {
299241
if (!shouldHandleHttpRequest(req)) {
300242
throw new Error(LOG_TAG + `${req.url} shouldn't have been handled`);

0 commit comments

Comments
 (0)