From a1313727e87db8c999b3ce46fd78ea4074f193b7 Mon Sep 17 00:00:00 2001
From: Ross Wollman <rwoll@users.noreply.github.com>
Date: Thu, 26 Aug 2021 11:26:08 -0700
Subject: [PATCH] fix(har): HAR artifacts need to be marked remote (#8462)

This is a follow-up fix to microsoft/playwright#8385.

Testing options are limited right now, but this change was confirmed
with a client running on my physical machine and a LaunchServer running
in a Docker container.

Before this change, the har.spec.ts only passed when the client and
server were on the some filesystem.

microsoft/playwright#8450 will likely give us options to test this in an
automated way in the official CI suite.
---
 src/client/browserContext.ts      |  7 ++++++-
 tests/browsertype-connect.spec.ts | 21 +++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/client/browserContext.ts b/src/client/browserContext.ts
index 6d2377e745..000f404f02 100644
--- a/src/client/browserContext.ts
+++ b/src/client/browserContext.ts
@@ -35,6 +35,7 @@ import * as structs from '../../types/structs';
 import { CDPSession } from './cdpSession';
 import { Tracing } from './tracing';
 import type { BrowserType } from './browserType';
+import { Artifact } from './artifact';
 
 export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel, channels.BrowserContextInitializer> implements api.BrowserContext {
   _pages = new Set<Page>();
@@ -346,7 +347,11 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel,
         await this._browserType?._onWillCloseContext?.(this);
         if (this._options.recordHar)  {
           const har = await this._channel.harExport();
-          await har.artifact.saveAs({ path: this._options.recordHar.path });
+          const artifact = Artifact.from(har.artifact);
+          if (this.browser()?._remoteType)
+            artifact._isRemote = true;
+          await artifact.saveAs(this._options.recordHar.path);
+          await artifact.delete();
         }
         await channel.close();
         await this._closedPromise;
diff --git a/tests/browsertype-connect.spec.ts b/tests/browsertype-connect.spec.ts
index 12caa547e4..4a0f6f8c36 100644
--- a/tests/browsertype-connect.spec.ts
+++ b/tests/browsertype-connect.spec.ts
@@ -454,3 +454,24 @@ test('should be able to connect when the wsEndpont is passed as the first argume
   expect(await page.evaluate('1 + 2')).toBe(3);
   await browser.close();
 });
+
+test('should save har', async ({browserType, startRemoteServer, server}, testInfo) => {
+  const remoteServer = await startRemoteServer();
+  const browser = await browserType.connect(remoteServer.wsEndpoint());
+  const harPath = testInfo.outputPath('test.har');
+  const context = await browser.newContext({
+    recordHar: {
+      path: harPath,
+    }
+  });
+  const page = await context.newPage();
+  await page.goto(server.EMPTY_PAGE);
+  await context.close();
+  await browser.close();
+
+  const log = JSON.parse(fs.readFileSync(harPath).toString())['log'];
+  expect(log.entries.length).toBe(1);
+  const entry = log.entries[0];
+  expect(entry.pageref).toBe(log.pages[0].id);
+  expect(entry.request.url).toBe(server.EMPTY_PAGE);
+});