Skip to content

fix(trace): Reload traces immediately on service worker start #37551

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(trace): Reload traces immediately on service worker start #37551

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
71 changes: 34 additions & 37 deletions packages/trace-viewer/src/sw/main.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,36 @@ self.addEventListener('activate', function(event: any) {

const scopePath = new URL(self.registration.scope).pathname;
const loadedTraces = new Map<string, { traceModel: TraceModel, snapshotServer: SnapshotServer }>();
const clientIdToTraceUrls = new Map<string, { limit: number | undefined, traceUrls: Set<string>, traceViewerServer: TraceViewerServer }>();
const clientIdToTraceUrls = new Map<string, { clientUrl: string | undefined, limit: number | undefined, traceUrls: Set<string>, traceViewerServer: TraceViewerServer }>();

// Service worker was restarted upon subresource fetch.
// It was stopped because ping did not keep it alive since the tab itself was throttled.
async function loadTracesFromIndexDB() {
const clients = await loadClientParams();
for (const [clientId, params] of Object.entries(clients ?? {})) {
for (const traceUrl of params.traceUrls)
await loadTrace(traceUrl, null, { id: clientId, url: params.clientUrl }, params.limit, () => {});
}
}

let loadTracesPromise = loadTracesFromIndexDB();
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want all the traces to be loaded for inactive clients. I think we want to move the if (!clientIdToTraceUrls.has(event.clientId)) { block of the original code up in doFetch and omit it for /context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. Although that would mean that if the first request after a reload is a /trace/snapshot call, the trace is not reloaded (since this is a different client worker (event.clientId) then the one that called /contexts) and we still return 404. Right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also we have to think about scenarios like:

  • If one client re-connects, its traces are loaded again. But what if it then calls /ping, it will gc and remove traces (params in IndexDB) for all other clients.

Sidenote: How important is the gc? Isn't the browser already taking care of this by shutting down the SW? (we can always do the await self.clients.matchAll(); check whenever the SW restarts)

Copy link
Member

@pavelfeldman pavelfeldman Sep 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The GC is so that you could open 100 traces one after another in the same window (UI Mode) without exploding out of memory.

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. Although that would mean that if the first request after a reload is a /trace/snapshot call, the trace is not reloaded (since this is a different client worker (event.clientId) then the one that called /contexts) and we still return 404. Right?

Why is it a different client worker? I assume event.clientId are preserved, so should return the right value.

Also we have to think about scenarios like: If one client re-connects, its traces are loaded again. But what if it then calls /ping, it will gc and remove traces (params in IndexDB) for all other clients.

Why does gc remove traces for all other clients? I would assume that once the sw is restarted, self.clients.matchAll() would return them and no gc will happen?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why is it a different client worker? I assume event.clientId are preserved, so should return the right value.

I should have been more clear here. What I meant is that each tracer viewer has two clients: one for the trace viewer itself (which also loads the trace) and one for the iframe (which fetches /snapshot). They have different event.clientIds (but indeed on a restart their Ids remain the same).

But I actually don't think it is currently possible for the /snapshot call to arrive first after a SW restart (its clientId is not linked to a saved trace). Since when selecting an action in the trace viewer, always first a /snapshotInfo call will be made (which reloads the trace). Then after that the /snapshot call is made, but at that time the trace has thus already been reloaded.


function simulateServiceWorkerRestart() {
async function simulateServiceWorkerRestart() {
loadedTraces.clear();
clientIdToTraceUrls.clear();
loadTracesPromise = loadTracesFromIndexDB();
}

async function loadTrace(traceUrl: string, traceFileName: string | null, client: any | undefined, limit: number | undefined, progress: (done: number, total: number) => undefined): Promise<TraceModel> {
await gc();
async function loadTrace(traceUrl: string, traceFileName: string | null, client: { id?: string, url?: string } | undefined, limit: number | undefined, progress: (done: number, total: number) => undefined): Promise<TraceModel> {
const clientId = client?.id ?? '';
let data = clientIdToTraceUrls.get(clientId);
if (!data) {
const clientURL = new URL(client?.url ?? self.registration.scope);
const traceViewerServerBaseUrl = new URL(clientURL.searchParams.get('server') ?? '../', clientURL);
data = { limit, traceUrls: new Set(), traceViewerServer: new TraceViewerServer(traceViewerServerBaseUrl) };
data = { clientUrl: client?.url, limit, traceUrls: new Set(), traceViewerServer: new TraceViewerServer(traceViewerServerBaseUrl) };
clientIdToTraceUrls.set(clientId, data);
}
data.traceUrls.add(traceUrl);
await saveClientIdParams();

const traceModel = new TraceModel();
try {
Expand Down Expand Up @@ -100,6 +111,9 @@ async function doFetch(event: FetchEvent): Promise<Response> {
// the https urls.
const isDeployedAsHttps = self.registration.scope.startsWith('https://');

// Wait until the old traces are loaded again to prevent altering (stored) clientIdToTraceUrls in the meantime.
await loadTracesPromise;

if (request.url.startsWith(self.registration.scope)) {
const url = new URL(unwrapPopoutUrl(request.url));
const relativePath = url.pathname.substring(scopePath.length - 1);
Expand All @@ -108,18 +122,20 @@ async function doFetch(event: FetchEvent): Promise<Response> {
return new Response(null, { status: 200 });
}
if (relativePath === '/restartServiceWorker') {
simulateServiceWorkerRestart();
await simulateServiceWorkerRestart();
return new Response(null, { status: 200 });
}

const traceUrl = url.searchParams.get('trace');

if (relativePath === '/contexts') {
try {
await gc();
const limit = url.searchParams.has('limit') ? +url.searchParams.get('limit')! : undefined;
const traceModel = await loadTrace(traceUrl!, url.searchParams.get('traceFileName'), client, limit, (done: number, total: number) => {
client.postMessage({ method: 'progress', params: { done, total } });
});
await saveClientParams();
return new Response(JSON.stringify(traceModel!.contextEntries), {
status: 200,
headers: { 'Content-Type': 'application/json' }
Expand All @@ -132,16 +148,6 @@ async function doFetch(event: FetchEvent): Promise<Response> {
}
}

if (!clientIdToTraceUrls.has(event.clientId)) {
// Service worker was restarted upon subresource fetch.
// It was stopped because ping did not keep it alive since the tab itself was throttled.
const params = await loadClientIdParams(event.clientId);
if (params) {
for (const traceUrl of params.traceUrls)
await loadTrace(traceUrl, null, client, params.limit, () => {});
}
}

if (relativePath.startsWith('/snapshotInfo/')) {
const { snapshotServer } = loadedTraces.get(traceUrl!) || {};
if (!snapshotServer)
Expand Down Expand Up @@ -227,6 +233,7 @@ async function gc() {
// @ts-ignore
if (!clients.find(c => c.id === clientId)) {
clientIdToTraceUrls.delete(clientId);
await saveClientParams();
continue;
}
if (data.limit !== undefined) {
Expand All @@ -241,36 +248,26 @@ async function gc() {
if (!usedTraces.has(traceUrl))
loadedTraces.delete(traceUrl);
}

await saveClientIdParams();
}

// Persist clientIdToTraceUrls to localStorage to avoid losing it when the service worker is restarted.
async function saveClientIdParams() {
const serialized: Record<string, {
limit: number | undefined,
traceUrls: string[]
}> = {};
type StoredClientParams = Record<string, { clientUrl?: string, limit?: number, traceUrls: string[] }>;

// Persist clientIdToTraceUrls to IndexDB to avoid losing it when the service worker is restarted.
async function saveClientParams() {
const params: StoredClientParams = {};
for (const [clientId, data] of clientIdToTraceUrls) {
serialized[clientId] = {
params[clientId] = {
clientUrl: data.clientUrl,
limit: data.limit,
traceUrls: [...data.traceUrls]
};
}

const newValue = JSON.stringify(serialized);
const oldValue = await idbKeyval.get('clientIdToTraceUrls');
if (newValue === oldValue)
return;
idbKeyval.set('clientIdToTraceUrls', newValue);
await idbKeyval.set('clientIdToTraceUrls', params);
}

async function loadClientIdParams(clientId: string): Promise<{ limit: number | undefined, traceUrls: string[] } | undefined> {
const serialized = await idbKeyval.get('clientIdToTraceUrls') as string | undefined;
if (!serialized)
return;
const deserialized = JSON.parse(serialized);
return deserialized[clientId];
async function loadClientParams() {
return await idbKeyval.get<StoredClientParams>('clientIdToTraceUrls');
}

// @ts-ignore
Expand Down
Loading