From 29dad28c8aba1c566e599f402c55b504f41e3d4b Mon Sep 17 00:00:00 2001
From: Wil Wilsman <wil@percy.io>
Date: Wed, 9 Sep 2020 14:27:34 -0500
Subject: [PATCH] =?UTF-8?q?fix(=F0=9F=90=9B):=20Handle=20page=20and=20snap?=
 =?UTF-8?q?shot=20errors=20(#562)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Sometimes, due to various reasons, the running browser might disconnect from Puppeteer during asset
discovery causing a `target closed` error. This error can happen anytime CDP sends a message to the
browser, such as enabling JS or setting viewport options. Since it is unhandled it can result in the
local server being unable to respond to requests which can then result in request timeouts.
---
 src/services/agent-service.ts           | 11 +++++++++--
 src/services/asset-discovery-service.ts | 13 +++++++------
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/services/agent-service.ts b/src/services/agent-service.ts
index 7996b610..fa51fa66 100644
--- a/src/services/agent-service.ts
+++ b/src/services/agent-service.ts
@@ -7,7 +7,7 @@ import * as path from 'path'
 import { Configuration } from '../configuration/configuration'
 import { SnapshotConfiguration } from '../configuration/snapshot-configuration'
 import { SnapshotOptions } from '../percy-agent-client/snapshot-options'
-import logger, { createFileLogger, profile } from '../utils/logger'
+import logger, { createFileLogger, addLogDate, profile } from '../utils/logger'
 import { HEALTHCHECK_PATH, SNAPSHOT_PATH, STOP_PATH } from './agent-service-constants'
 import BuildService from './build-service'
 import Constants from './constants'
@@ -33,7 +33,14 @@ export class AgentService {
     this.app.use(bodyParser.json({ limit: '50mb' }))
     this.app.use(express.static(this.publicDirectory))
 
-    this.app.post(SNAPSHOT_PATH, this.handleSnapshot.bind(this))
+    this.app.post(SNAPSHOT_PATH, async (request, response) => {
+      try { await this.handleSnapshot.call(this, request, response) } catch (error) {
+        logger.error(addLogDate(`${error.name} ${error.message}`))
+        logger.debug(addLogDate(error))
+        return response.json({ success: false })
+      }
+    })
+
     this.app.post(STOP_PATH, this.handleStop.bind(this))
     this.app.get(HEALTHCHECK_PATH, this.handleHealthCheck.bind(this))
 
diff --git a/src/services/asset-discovery-service.ts b/src/services/asset-discovery-service.ts
index 0c5a618a..b4d31d9a 100644
--- a/src/services/asset-discovery-service.ts
+++ b/src/services/asset-discovery-service.ts
@@ -215,12 +215,6 @@ export class AssetDiscoveryService extends PercyClientService {
     profile('--> assetDiscoveryService.pool.acquire', { url: rootResourceUrl })
     const page = await pool.acquire()
     profile('--> assetDiscoveryService.pool.acquire')
-    await page.setJavaScriptEnabled(enableJavaScript)
-    await page.setViewport(Object.assign(page.viewport(), { width }))
-    await page.setExtraHTTPHeaders(merge.all([
-      this.configuration['request-headers'],
-      requestHeaders,
-    ]) as {})
 
     page.on('request', async (request) => {
       const requestUrl = request.url()
@@ -296,6 +290,13 @@ export class AssetDiscoveryService extends PercyClientService {
     let maybeResources: any[] = []
 
     try {
+      await page.setJavaScriptEnabled(enableJavaScript)
+      await page.setViewport(Object.assign(page.viewport(), { width }))
+      await page.setExtraHTTPHeaders(merge.all([
+        this.configuration['request-headers'],
+        requestHeaders,
+      ]) as {})
+
       profile('--> assetDiscoveryService.page.goto', { url: rootResourceUrl })
       await page.goto(rootResourceUrl)
       profile('--> assetDiscoveryService.page.goto')