Skip to content

Commit

Permalink
fix(🐛): Handle page and snapshot errors (#562)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Wil Wilsman authored Sep 9, 2020
1 parent 86e6fae commit 29dad28
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
11 changes: 9 additions & 2 deletions src/services/agent-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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))

Expand Down
13 changes: 7 additions & 6 deletions src/services/asset-discovery-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit 29dad28

Please sign in to comment.