fix(trace): operate trace uri, not path

packages/playwright-core/src/server/trace/viewer/traceViewer.ts

@@ -55,25 +55,26 @@ export type TraceViewerAppOptions = {
 
 const tracesDirMarker = 'traces.dir';
 
-function validateTraceUrl(traceUrl: string | undefined): string | undefined {
-  if (!traceUrl)
-    return traceUrl;
+function validateTraceUrl(traceFileOrUrl: string | undefined): string | undefined {
+  if (!traceFileOrUrl)
+    return traceFileOrUrl;
 
-  if (traceUrl.startsWith('http://') || traceUrl.startsWith('https://'))
-    return traceUrl;
+  if (traceFileOrUrl.startsWith('http://') || traceFileOrUrl.startsWith('https://'))
+    return traceFileOrUrl;
 
+  let traceFile = traceFileOrUrl;
   // If .json is requested, we'll synthesize it.
-  if (traceUrl.endsWith('.json'))
-    return traceUrl;
+  if (traceFile.endsWith('.json'))
+    return toFilePathUrl(traceFile);
 
   try {
-    const stat = fs.statSync(traceUrl);
+    const stat = fs.statSync(traceFile);
     // If the path is a directory, add 'trace.dir' which has a special handler.
     if (stat.isDirectory())
-      return path.join(traceUrl, tracesDirMarker);
-    return traceUrl;
+      traceFile = path.join(traceFile, tracesDirMarker);
+    return toFilePathUrl(traceFile);
   } catch {
-    throw new Error(`Trace file ${traceUrl} does not exist!`);
+    throw new Error(`Trace file ${traceFileOrUrl} does not exist!`);
   }
 }
 
@@ -270,13 +271,18 @@ function traceDescriptor(traceDir: string, tracePrefix: string | undefined) {
 
   for (const name of fs.readdirSync(traceDir)) {
     if (!tracePrefix || name.startsWith(tracePrefix))
-      result.entries.push({ name, path: path.join(traceDir, name) });
+      result.entries.push({ name, path: toFilePathUrl(path.join(traceDir, name)) });
   }
 
   const resourcesDir = path.join(traceDir, 'resources');
   if (fs.existsSync(resourcesDir)) {
     for (const name of fs.readdirSync(resourcesDir))
-      result.entries.push({ name: 'resources/' + name, path: path.join(resourcesDir, name) });
+      result.entries.push({ name: 'resources/' + name, path: toFilePathUrl(path.join(resourcesDir, name)) });
   }
   return result;
 }
+
+
+function toFilePathUrl(filePath: string): string {
+  return `file?path=${encodeURIComponent(filePath)}`;
+}

packages/playwright-core/src/utils/isomorphic/trace/traceLoader.ts

@@ -27,7 +27,6 @@ export interface TraceLoaderBackend {
   readText(entryName: string): Promise<string | undefined>;
   readBlob(entryName: string): Promise<Blob | undefined>;
   isLive(): boolean;
-  traceURL(): string;
 }
 
 export class TraceLoader {

packages/playwright-core/src/utils/isomorphic/trace/traceModel.ts

@@ -86,14 +86,14 @@ export class TraceModel {
   readonly sources: Map<string, SourceModel>;
   resources: ResourceSnapshot[];
   readonly actionCounters: Map<string, number>;
-  readonly traceUrl: string;
+  readonly traceUri: string;
 
 
-  constructor(traceUrl: string, contexts: ContextEntry[]) {
+  constructor(traceUri: string, contexts: ContextEntry[]) {
     contexts.forEach(contextEntry => indexModel(contextEntry));
     const libraryContext = contexts.find(context => context.origin === 'library');
 
-    this.traceUrl = traceUrl;
+    this.traceUri = traceUri;
     this.browserName = libraryContext?.browserName || '';
     this.sdkLanguage = libraryContext?.sdkLanguage;
     this.channel = libraryContext?.channel;
@@ -114,7 +114,7 @@ export class TraceModel {
     this.hasSource = contexts.some(c => c.hasSource);
     this.hasStepData = contexts.some(context => context.origin === 'testRunner');
     this.resources = [...contexts.map(c => c.resources)].flat();
-    this.attachments = this.actions.flatMap(action => action.attachments?.map(attachment => ({ ...attachment, callId: action.callId, traceUrl })) ?? []);
+    this.attachments = this.actions.flatMap(action => action.attachments?.map(attachment => ({ ...attachment, callId: action.callId, traceUri })) ?? []);
     this.visibleAttachments = this.attachments.filter(attachment => !attachment.name.startsWith('_'));
 
     this.events.sort((a1, a2) => a1.time - a2.time);
@@ -132,7 +132,7 @@ export class TraceModel {
 
   createRelativeUrl(path: string) {
     const url = new URL('http://localhost/' + path);
-    url.searchParams.set('trace', this.traceUrl);
+    url.searchParams.set('trace', this.traceUri);
     return url.toString().substring('http://localhost/'.length);
   }
 

packages/trace-viewer/src/sw/main.ts

@@ -19,7 +19,7 @@ import { TraceLoader } from '@isomorphic/trace/traceLoader';
 import { TraceVersionError } from '@isomorphic/trace/traceModernizer';
 
 import { Progress, splitProgress } from './progress';
-import { FetchTraceLoaderBackend, traceFileURL, ZipTraceLoaderBackend } from './traceLoaderBackends';
+import { FetchTraceLoaderBackend, ZipTraceLoaderBackend } from './traceLoaderBackends';
 
 type Client = {
   id: string;
@@ -74,9 +74,9 @@ function simulateRestart() {
   clientIdToTraceUrls.clear();
 }
 
-async function loadTraceOrError(clientId: string, url: URL, isContextRequest: boolean, progress: Progress): Promise<{ loadedTrace?: LoadedTrace, errorResponse?: Response }> {
+async function loadTraceOrError(clientId: string, url: URL, progress: Progress): Promise<{ loadedTrace?: LoadedTrace, errorResponse?: Response }> {
   try {
-    const loadedTrace = await loadTrace(clientId, url, isContextRequest, progress);
+    const loadedTrace = await loadTrace(clientId, url, progress);
     return { loadedTrace };
   } catch (error) {
     return {
@@ -88,40 +88,39 @@ async function loadTraceOrError(clientId: string, url: URL, isContextRequest: bo
   }
 }
 
-function loadTrace(clientId: string, url: URL, isContextRequest: boolean, progress: Progress): Promise<LoadedTrace> {
-  const traceUrl = url.searchParams.get('trace')!;
-  if (!traceUrl)
+function loadTrace(clientId: string, url: URL, progress: Progress): Promise<LoadedTrace> {
+  const traceUri = url.searchParams.get('trace')!;
+  if (!traceUri)
     throw new Error('trace parameter is missing');
 
-  clientIdToTraceUrls.set(clientId, traceUrl);
-  const omitCache = isContextRequest && isLiveTrace(traceUrl);
-  const loadedTrace = omitCache ? undefined : loadedTraces.get(traceUrl);
+  clientIdToTraceUrls.set(clientId, traceUri);
+  const loadedTrace = loadedTraces.get(traceUri);
   if (loadedTrace)
     return loadedTrace;
-  const promise = innerLoadTrace(traceUrl, progress);
-  loadedTraces.set(traceUrl, promise);
-  promise.catch(() => loadedTraces.delete(traceUrl));
+  const promise = innerLoadTrace(traceUri, progress);
+  loadedTraces.set(traceUri, promise);
+  promise.catch(() => loadedTraces.delete(traceUri));
   return promise;
 }
 
-async function innerLoadTrace(traceUrl: string, progress: Progress): Promise<LoadedTrace> {
+async function innerLoadTrace(traceUri: string, progress: Progress): Promise<LoadedTrace> {
   await gc();
 
   const traceLoader = new TraceLoader();
   try {
     // Allow 10% to hop from sw to page.
     const [fetchProgress, unzipProgress] = splitProgress(progress, [0.5, 0.4, 0.1]);
-    const backend = isLiveTrace(traceUrl) || traceUrl.endsWith('traces.dir') ? new FetchTraceLoaderBackend(traceUrl) : new ZipTraceLoaderBackend(traceUrl, fetchProgress);
+    const backend = isLiveTrace(traceUri) || traceUri.endsWith('traces.dir') ? new FetchTraceLoaderBackend(traceUri) : new ZipTraceLoaderBackend(traceUri, fetchProgress);
     await traceLoader.load(backend, unzipProgress);
   } catch (error: any) {
     // eslint-disable-next-line no-console
     console.error(error);
     if (error?.message?.includes('Cannot find .trace file') && await traceLoader.hasEntry('index.html'))
       throw new Error('Could not load trace. Did you upload a Playwright HTML report instead? Make sure to extract the archive first and then double-click the index.html file or put it on a web server.');
     if (error instanceof TraceVersionError)
-      throw new Error(`Could not load trace from ${traceUrl}. ${error.message}`);
+      throw new Error(`Could not load trace from ${traceUri}. ${error.message}`);
 
-    let message = `Could not load trace from ${traceUrl}. Make sure a valid Playwright Trace is accessible over this url.`;
+    let message = `Could not load trace from ${traceUri}. Make sure a valid Playwright Trace is accessible over this url.`;
 
     const lnaPermission = await navigator.permissions.query({ name: 'local-network-access' as PermissionName }).catch(() => { });
     if (lnaPermission && lnaPermission.state !== 'granted')
@@ -162,7 +161,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
     // Snapshot iframe navigation request.
     if (relativePath?.startsWith('/snapshot/')) {
       // It is Ok to pass noop progress as the trace is likely already loaded.
-      const { errorResponse, loadedTrace } = await loadTraceOrError(event.resultingClientId!, url, false, noopProgress);
+      const { errorResponse, loadedTrace } = await loadTraceOrError(event.resultingClientId!, url, noopProgress);
       if (errorResponse)
         return errorResponse;
       const pageOrFrameId = relativePath.substring('/snapshot/'.length);
@@ -181,7 +180,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
     if (!client)
       return new Response('Sub-resource without a client', { status: 500 });
 
-    const { snapshotServer } = await loadTrace(client.id, new URL(client.url), false, clientProgress(client));
+    const { snapshotServer } = await loadTrace(client.id, new URL(client.url), clientProgress(client));
     if (!snapshotServer)
       return new Response(null, { status: 404 });
 
@@ -201,8 +200,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
     if (!client)
       return new Response('Sub-resource without a client', { status: 500 });
 
-    const isContextRequest = relativePath === '/contexts';
-    const { errorResponse, loadedTrace } = await loadTraceOrError(client.id, url, isContextRequest, clientProgress(client));
+    const { errorResponse, loadedTrace } = await loadTraceOrError(client.id, url, clientProgress(client));
     if (errorResponse)
       return errorResponse;
 
@@ -231,13 +229,7 @@ async function doFetch(event: FetchEvent): Promise<Response> {
     }
   }
 
-  // Pass through to the server for file requests.
-  if (relativePath?.startsWith('/file/')) {
-    const path = url.searchParams.get('path')!;
-    return await fetch(traceFileURL(path));
-  }
-
-  // Static content for sub-resource.
+  // Pass through to the server for file requests and static content.
   return fetch(event.request);
 }
 
@@ -279,8 +271,10 @@ function clientProgress(client: Client): Progress {
 
 function noopProgress(done: number, total: number): undefined { }
 
-function isLiveTrace(traceUrl: string): boolean {
-  return traceUrl.endsWith('.json');
+function isLiveTrace(traceUri: string): boolean {
+  const url = new URL(traceUri, 'http://localhost');
+  const path = url.searchParams.get('path');
+  return !!path?.endsWith('.json');
 }
 
 self.addEventListener('fetch', function(event: FetchEvent) {

packages/trace-viewer/src/sw/traceLoaderBackends.ts

@@ -26,14 +26,12 @@ type Progress = (done: number, total: number) => undefined;
 export class ZipTraceLoaderBackend implements TraceLoaderBackend {
   private _zipReader: zip.ZipReader<unknown>;
   private _entriesPromise: Promise<Map<string, zip.Entry>>;
-  private _traceURL: string;
 
-  constructor(traceURL: string, progress: Progress) {
-    this._traceURL = traceURL;
+  constructor(traceUri: string, progress: Progress) {
     zipjs.configure({ baseURL: self.location.href } as any);
 
     this._zipReader = new zipjs.ZipReader(
-        new zipjs.HttpReader(this._resolveTraceURL(traceURL), { mode: 'cors', preventHeadRequest: true } as any),
+        new zipjs.HttpReader(this._resolveTraceURI(traceUri), { mode: 'cors', preventHeadRequest: true } as any),
         { useWebWorkers: false });
     this._entriesPromise = this._zipReader.getEntries({ onprogress: progress }).then(entries => {
       const map = new Map<string, zip.Entry>();
@@ -43,26 +41,16 @@ export class ZipTraceLoaderBackend implements TraceLoaderBackend {
     });
   }
 
-  private _resolveTraceURL(traceURL: string): string {
-    let url: string;
-    if (traceURL.startsWith('http') || traceURL.startsWith('blob')) {
-      url = traceURL;
-      if (url.startsWith('https://www.dropbox.com/'))
-        url = 'https://dl.dropboxusercontent.com/' + url.substring('https://www.dropbox.com/'.length);
-    } else {
-      url = traceFileURL(traceURL);
-    }
-    return url;
+  private _resolveTraceURI(traceUri: string): string {
+    if (traceUri.startsWith('https://www.dropbox.com/'))
+      return 'https://dl.dropboxusercontent.com/' + traceUri.substring('https://www.dropbox.com/'.length);
+    return traceUri;
   }
 
   isLive() {
     return false;
   }
 
-  traceURL() {
-    return this._traceURL;
-  }
-
   async entryNames(): Promise<string[]> {
     const entries = await this._entriesPromise;
     return [...entries.keys()];
@@ -96,11 +84,9 @@ export class ZipTraceLoaderBackend implements TraceLoaderBackend {
 
 export class FetchTraceLoaderBackend implements TraceLoaderBackend {
   private _entriesPromise: Promise<Map<string, string>>;
-  private _path: string;
 
-  constructor(path: string) {
-    this._path  = path;
-    this._entriesPromise = this._readFile(path).then(async response => {
+  constructor(traceUri: string) {
+    this._entriesPromise = this._readFile(traceUri).then(async response => {
       if (!response)
         throw new Error('File not found');
       const json = await response.json();
@@ -115,10 +101,6 @@ export class FetchTraceLoaderBackend implements TraceLoaderBackend {
     return true;
   }
 
-  traceURL(): string {
-    return this._path;
-  }
-
   async entryNames(): Promise<string[]> {
     const entries = await this._entriesPromise;
     return [...entries.keys()];
@@ -141,20 +123,16 @@ export class FetchTraceLoaderBackend implements TraceLoaderBackend {
 
   private async _readEntry(entryName: string): Promise<Response | undefined> {
     const entries = await this._entriesPromise;
-    const fileName = entries.get(entryName);
-    if (!fileName)
+    const fileUri = entries.get(entryName);
+    if (!fileUri)
       return;
-    return this._readFile(fileName);
+    return this._readFile(fileUri);
   }
 
-  private async _readFile(path: string): Promise<Response | undefined> {
-    const response = await fetch(traceFileURL(path));
+  private async _readFile(uri: string): Promise<Response | undefined> {
+    const response = await fetch(uri);
     if (response.status === 404)
       return;
     return response;
   }
 }
-
-export function traceFileURL(path: string): string {
-  return `file?path=${encodeURIComponent(path)}`;
-}

packages/trace-viewer/src/ui/snapshotTab.tsx

@@ -58,7 +58,7 @@ export const SnapshotTabsView: React.FunctionComponent<{
   }, [action]);
   const { snapshotInfoUrl, snapshotUrl, popoutUrl } = React.useMemo(() => {
     const snapshot = snapshots[snapshotTab];
-    return model && snapshot ? extendSnapshot(model.traceUrl, snapshot, shouldPopulateCanvasFromScreenshot) : { snapshotInfoUrl: undefined, snapshotUrl: undefined, popoutUrl: undefined };
+    return model && snapshot ? extendSnapshot(model.traceUri, snapshot, shouldPopulateCanvasFromScreenshot) : { snapshotInfoUrl: undefined, snapshotUrl: undefined, popoutUrl: undefined };
   }, [snapshots, snapshotTab, shouldPopulateCanvasFromScreenshot, model]);
 
   const snapshotUrls = React.useMemo((): SnapshotUrls | undefined => snapshotInfoUrl !== undefined ? { snapshotInfoUrl, snapshotUrl, popoutUrl } : undefined, [snapshotInfoUrl, snapshotUrl, popoutUrl]);
@@ -416,9 +416,9 @@ export function collectSnapshots(action: ActionTraceEvent | undefined): Snapshot
 
 const isUnderTest = new URLSearchParams(window.location.search).has('isUnderTest');
 
-export function extendSnapshot(traceUrl: string, snapshot: Snapshot, shouldPopulateCanvasFromScreenshot: boolean): SnapshotUrls {
+export function extendSnapshot(traceUri: string, snapshot: Snapshot, shouldPopulateCanvasFromScreenshot: boolean): SnapshotUrls {
   const params = new URLSearchParams();
-  params.set('trace', traceUrl);
+  params.set('trace', traceUri);
   params.set('name', snapshot.snapshotName);
   if (isUnderTest)
     params.set('isUnderTest', 'true');
@@ -436,7 +436,7 @@ export function extendSnapshot(traceUrl: string, snapshot: Snapshot, shouldPopul
 
   const popoutParams = new URLSearchParams();
   popoutParams.set('r', snapshotUrl);
-  popoutParams.set('trace', traceUrl);
+  popoutParams.set('trace', traceUri);
   const popoutUrl = new URL(`snapshot.html?${popoutParams.toString()}`, window.location.href).toString();
   return { snapshotInfoUrl, snapshotUrl, popoutUrl };
 }

packages/trace-viewer/src/ui/sourceTab.tsx

@@ -59,7 +59,7 @@ function useSources(stack: StackFrame[] | undefined, selectedFrame: number, sour
         if (!response || response.status === 404)
           response = await fetch(`file?path=${encodeURIComponent(file)}`);
         if (response.status >= 400)
-          source.content = `<Unable to read "${file}">`;
+          source.content = ``;
         else
           source.content = await response.text();
       } catch {