Repository: elastic/synthetics (ts)

Issue:
Set the default navigation timeout to 50 seconds
We have been seeing numerous sites genuinely failing to load within the current default of 30 seconds, which leads to failures.  As such, the default timeout for navigation timeouts should be changed to 50 seconds.

Pull request description:
feat: allow setting navigation and action timeouts
+ fix #738
+ PR changes the default navigation timeout for all journeys to `50 seconds` instead of `30 seconds`. Same applies to all actions as well which is controlled by `actionTimeout`. 
+ Allow setting the timeouts mainly `navigationTimeout` and `actionTimeout` via the `playwrightOptions` configuration. These are  prioritized over the default timeout of `50 seconds` which is set by the agent.
+ This is backwards compatible meaning, individual method timeouts would still be preferred over both of these timeouts. Ex: `page.go("url", {timeout: 2000})` would be preferred over `playwrightOptions.navigationTimeout` and `default 50 seconds` timeout. 

Patch:
diff --git a/src/cli.ts b/src/cli.ts
index 9c2aaa4..a87a40a 100644
--- a/src/cli.ts
+++ b/src/cli.ts
@@ -79,7 +79,6 @@ program
   )
   .option('--inline', 'Run inline journeys from heartbeat')
   .option('-r, --require <modules...>', 'module(s) to preload')
-  .option('--no-headless', 'run browser in headful mode')
   .option('--sandbox', 'enable chromium sandboxing')
   .option('--rich-events', 'Mimics a heartbeat run')
   .option(
diff --git a/src/common_types.ts b/src/common_types.ts
index 10c4192..e886308 100644
--- a/src/common_types.ts
+++ b/src/common_types.ts
@@ -221,7 +221,6 @@ export type CliArgs = BaseArgs & {
   pattern?: string;
   inline?: boolean;
   require?: Array<string>;
-  headless?: boolean;
   sandbox?: boolean;
   richEvents?: boolean;
   capability?: Array<string>;
@@ -255,7 +254,11 @@ export type ProjectSettings = {
 };
 
 export type PlaywrightOptions = LaunchOptions &
-  BrowserContextOptions & { testIdAttribute?: string };
+  BrowserContextOptions & {
+    testIdAttribute?: string;
+    actionTimeout?: number;
+    navigationTimeout?: number;
+  };
 export type SyntheticsConfig = {
   params?: Params;
   playwrightOptions?: PlaywrightOptions;
diff --git a/src/core/gatherer.ts b/src/core/gatherer.ts
index f78a2e2..2ac9f75 100644
--- a/src/core/gatherer.ts
+++ b/src/core/gatherer.ts
@@ -34,6 +34,9 @@ import { PluginManager } from '../plugins';
 import { log } from './logger';
 import { Driver, NetworkConditions, RunOptions } from '../common_types';
 
+// Default timeout for Playwright actions and Navigations
+const DEFAULT_TIMEOUT = 50000;
+
 /**
  * Purpose of the Gatherer is to set up the necessary browser driver
  * related capabilities for the runner to run all journeys
@@ -63,8 +66,15 @@ export class Gatherer {
       ...playwrightOptions,
       userAgent: await Gatherer.getUserAgent(playwrightOptions?.userAgent),
     });
-    Gatherer.setNetworkConditions(context, networkConditions);
+    // Set timeouts for actions and navigations
+    context.setDefaultTimeout(
+      playwrightOptions?.actionTimeout ?? DEFAULT_TIMEOUT
+    );
+    context.setDefaultNavigationTimeout(
+      playwrightOptions?.navigationTimeout ?? DEFAULT_TIMEOUT
+    );
 
+    Gatherer.setNetworkConditions(context, networkConditions);
     if (playwrightOptions?.testIdAttribute) {
       selectors.setTestIdAttribute(playwrightOptions.testIdAttribute);
     }
diff --git a/src/options.ts b/src/options.ts
index 8734707..9f6260e 100644
--- a/src/options.ts
+++ b/src/options.ts
@@ -101,7 +101,6 @@ export function normalizeOptions(cliArgs: CliArgs): RunOptions {
   );
   options.playwrightOptions = {
     ...playwrightOpts,
-    headless: cliArgs.headless,
     chromiumSandbox: cliArgs.sandbox ?? playwrightOpts?.chromiumSandbox,
     ignoreHTTPSErrors:
       cliArgs.ignoreHttpsErrors ?? playwrightOpts?.ignoreHTTPSErrors,


Test patch:
diff --git a/__tests__/core/gatherer.test.ts b/__tests__/core/gatherer.test.ts
index 8992e11..367c073 100644
--- a/__tests__/core/gatherer.test.ts
+++ b/__tests__/core/gatherer.test.ts
@@ -267,6 +267,52 @@ describe('Gatherer', () => {
     });
   });
 
+  describe('Timeouts', () => {
+    it('set navigation timeout', async () => {
+      const driver = await Gatherer.setupDriver({
+        wsEndpoint,
+        playwrightOptions: { navigationTimeout: 1, actionTimeout: 2 },
+      });
+      server.route('/hang.html', () => {});
+      let error: any = null;
+      await driver.page
+        .goto(server.PREFIX + '/hang.html')
+        .catch(e => (error = e));
+      expect(error.message).toContain('page.goto: Timeout 1ms exceeded.');
+      await Gatherer.dispose(driver);
+      await Gatherer.stop();
+    });
+
+    it('prefer timeout over navigation timeout', async () => {
+      const driver = await Gatherer.setupDriver({
+        wsEndpoint,
+        playwrightOptions: { navigationTimeout: 3 },
+      });
+      server.route('/hang.html', () => {});
+      let error: any = null;
+      await driver.page
+        .goto(server.PREFIX + '/hang.html', { timeout: 1 })
+        .catch(e => (error = e));
+      expect(error.message).toContain('page.goto: Timeout 1ms exceeded.');
+      await Gatherer.dispose(driver);
+      await Gatherer.stop();
+    });
+
+    it('set action timeout', async () => {
+      const driver = await Gatherer.setupDriver({
+        wsEndpoint,
+        playwrightOptions: { actionTimeout: 2 },
+      });
+      await driver.page.setContent(
+        '<button disabled><span>Click Span</span></button>'
+      );
+      const error = await driver.page.click('text=Click Span').catch(e => e);
+      expect(error.message).toContain('page.click: Timeout 2ms exceeded.');
+      await Gatherer.dispose(driver);
+      await Gatherer.stop();
+    });
+  });
+
   describe('API Request Context', () => {
     it('exposes request', async () => {
       const driver = await Gatherer.setupDriver({ wsEndpoint });
diff --git a/__tests__/options.test.ts b/__tests__/options.test.ts
index 51e67a9..621e126 100644
--- a/__tests__/options.test.ts
+++ b/__tests__/options.test.ts
@@ -33,7 +33,9 @@ describe('options', () => {
       params: {
         foo: 'bar',
       },
-      headless: true,
+      playwrightOptions: {
+        headless: false,
+      },
       sandbox: false,
       screenshots: 'on',
       dryRun: true,
@@ -59,7 +61,7 @@ describe('options', () => {
         defaultBrowserType: 'chromium',
         deviceScaleFactor: 4.5,
         hasTouch: true,
-        headless: true,
+        headless: false,
         ignoreHTTPSErrors: undefined,
         isMobile: true,
         userAgent:
diff --git a/__tests__/utils/test-config.ts b/__tests__/utils/test-config.ts
index a2848dd..079be90 100644
--- a/__tests__/utils/test-config.ts
+++ b/__tests__/utils/test-config.ts
@@ -27,7 +27,7 @@ import { ChildProcess, spawn } from 'child_process';
 import { join } from 'path';
 import { Monitor } from '../../src/dsl/monitor';
 
-export const wsEndpoint = process.env.WSENDPOINT;
+export const wsEndpoint = process.env.WSENDPOINT || '';
 
 const FIXTURES_DIR = join(__dirname, '..', 'fixtures');
 export function createTestMonitor(filename: string, type = 'browser') {
@@ -93,9 +93,9 @@ export class CLIMock {
     );
 
     if (this.stdinStr) {
-      this.process.stdin.setDefaultEncoding('utf8');
-      this.process.stdin.write(this.stdinStr);
-      this.process.stdin.end();
+      this.process.stdin?.setDefaultEncoding('utf8');
+      this.process.stdin?.write(this.stdinStr);
+      this.process.stdin?.end();
     }
 
     const dataListener = data => {
@@ -106,21 +106,21 @@ export class CLIMock {
       this.chunks.push(this.data);
       if (this.waitForPromise && this.data.includes(this.waitForText)) {
         if (!this.debug) {
-          this.process.stdout.off('data', dataListener);
+          this.process.stdout?.off('data', dataListener);
         }
         this.waitForPromise();
       }
     };
-    this.process.stdout.on('data', dataListener);
+    this.process.stdout?.on('data', dataListener);
 
     this.exitCode = new Promise(res => {
-      this.process.stderr.on('data', data => {
+      this.process.stderr?.on('data', data => {
         this.stderrStr += data;
         if (this.debug) {
           console.log('CLIMock.stderr:', data.toString());
         }
       });
-      this.process.on('exit', code => res(code));
+      this.process.on('exit', code => res(code ?? 0));
     });
 
     return this;