From 43745210a5dc62023063c329090608f2b4d64f7d Mon Sep 17 00:00:00 2001
From: Max Schmitt <max@schmitt.mx>
Date: Wed, 3 Apr 2024 18:47:03 +0200
Subject: [PATCH] fix(trace-viewer): exit if given trace.zip does not exist
 (#30222)

---
 .../src/server/trace/viewer/traceViewer.ts       | 16 +++++-----------
 tests/installation/playwright-cli.spec.ts        | 10 ++++++++++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts
index 8bf26d3db1..3f1511cef6 100644
--- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts
+++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts
@@ -53,20 +53,16 @@ export type TraceViewerAppOptions = {
   persistentContextOptions?: Parameters<BrowserType['launchPersistentContext']>[2];
 };
 
-async function validateTraceUrls(traceUrls: string[]) {
+function validateTraceUrls(traceUrls: string[]) {
   for (const traceUrl of traceUrls) {
     let traceFile = traceUrl;
     // If .json is requested, we'll synthesize it.
     if (traceUrl.endsWith('.json'))
       traceFile = traceUrl.substring(0, traceUrl.length - '.json'.length);
 
-    if (!traceUrl.startsWith('http://') && !traceUrl.startsWith('https://') && !fs.existsSync(traceFile) && !fs.existsSync(traceFile + '.trace')) {
-      // eslint-disable-next-line no-console
-      console.error(`Trace file ${traceUrl} does not exist!`);
-      return false;
-    }
+    if (!traceUrl.startsWith('http://') && !traceUrl.startsWith('https://') && !fs.existsSync(traceFile) && !fs.existsSync(traceFile + '.trace'))
+      throw new Error(`Trace file ${traceUrl} does not exist!`);
   }
-  return true;
 }
 
 export async function startTraceViewerServer(options?: TraceViewerServerOptions): Promise<HttpServer> {
@@ -146,8 +142,7 @@ export async function installRootRedirect(server: HttpServer, traceUrls: string[
 }
 
 export async function runTraceViewerApp(traceUrls: string[], browserName: string, options: TraceViewerServerOptions & { headless?: boolean }, exitOnClose?: boolean) {
-  if (!validateTraceUrls(traceUrls))
-    return;
+  validateTraceUrls(traceUrls);
   const server = await startTraceViewerServer(options);
   await installRootRedirect(server, traceUrls, options);
   const page = await openTraceViewerApp(server.urlPrefix(), browserName, options);
@@ -157,8 +152,7 @@ export async function runTraceViewerApp(traceUrls: string[], browserName: string
 }
 
 export async function runTraceInBrowser(traceUrls: string[], options: TraceViewerServerOptions) {
-  if (!validateTraceUrls(traceUrls))
-    return;
+  validateTraceUrls(traceUrls);
   const server = await startTraceViewerServer(options);
   await installRootRedirect(server, traceUrls, options);
   await openTraceInBrowser(server.urlPrefix());
diff --git a/tests/installation/playwright-cli.spec.ts b/tests/installation/playwright-cli.spec.ts
index 9b2b34cb74..02e65a0fe8 100755
--- a/tests/installation/playwright-cli.spec.ts
+++ b/tests/installation/playwright-cli.spec.ts
@@ -58,4 +58,14 @@ test('cli should work', async ({ exec, tmpWorkspace }) => {
     await exec('npx playwright screenshot about:blank two.png');
     await fs.promises.stat(path.join(tmpWorkspace, 'two.png'));
   });
+
+  await test.step('show-trace', async () => {
+    const result = await exec('npx playwright show-trace i-do-not-exist.zip', { expectToExitWithError: true });
+    expect(result).toContain(`Trace file i-do-not-exist.zip does not exist`);
+  });
+
+  await test.step('show-report', async () => {
+    const result = await exec('npx playwright show-report', { expectToExitWithError: true });
+    expect(result).toContain(`No report found at "${path.join(fs.realpathSync(tmpWorkspace), 'playwright-report')}"`);
+  });
 });