feat(api): better exception throwing support

Summary: Close #84 Reviewed By: tulga1970 Differential Revision: D46813356 fbshipit-source-id: 07aed540d05e8e90dd622cb9d4d104770bea17f9
facebook · Jul 5, 2023 · b5eaf8b · b5eaf8b
1 parent 961316c
commit b5eaf8b
Show file tree

Hide file tree

Showing 7 changed files with 88 additions and 31 deletions.
diff --git a/packages/api/src/API.ts b/packages/api/src/API.ts
@@ -15,6 +15,7 @@ import type {
   ISerializedInfo,
   IScenario,
   XvfbType,
+  Nullable,
   Optional,
 } from '@memlab/core';
 
@@ -416,38 +417,39 @@ export async function testInBrowser(options: APIOptions = {}): Promise<void> {
   }
 
   const testPlanner = options.testPlanner ?? defaultTestPlanner;
-  let interactionManager: E2EInteractionManager | null = null;
-  let xvfb: XvfbType | null = null;
+  let interactionManager: Nullable<E2EInteractionManager> = null;
+  let xvfb: Nullable<XvfbType> = null;
+  let browser: Nullable<Browser> = null;
+  let page: Nullable<Page> = null;
+  let maybeError: Nullable<Error> = null;
   try {
     xvfb = Xvfb.startIfEnabled();
-    const browser = await APIUtils.getBrowser();
+    browser = await APIUtils.getBrowser();
     const pages = await browser.pages();
-    const page = pages.length > 0 ? pages[0] : await browser.newPage();
+    page = pages.length > 0 ? pages[0] : await browser.newPage();
+    // create and configure web page interaction manager
     interactionManager = new E2EInteractionManager(page, browser);
-
     if (options.evalInBrowserAfterInitLoad) {
       interactionManager.setEvalFuncAfterInitLoad(
         options.evalInBrowserAfterInitLoad,
       );
     }
-
     const visitPlan = testPlanner.getVisitPlan();
+    // setup page configuration
     config.setDevice(visitPlan.device);
-
     autoDismissDialog(page);
     await initBrowserInfoInConfig(browser);
-
     browserInfo.monitorWebConsole(page);
-
     await setupPage(page, options);
-
+    // interact with the web page and take heap snapshots
     await interactionManager.visitAndGetSnapshots(options);
-    await utils.closePuppeteer(browser, [page]);
   } catch (ex) {
-    const error = utils.getError(ex);
-    utils.checkUninstalledLibrary(error);
-    info.error(error.message);
+    maybeError = utils.getError(ex);
+    utils.checkUninstalledLibrary(maybeError);
   } finally {
+    if (browser && page) {
+      await utils.closePuppeteer(browser, [page]);
+    }
     if (interactionManager) {
       interactionManager.clearCDPSession();
     }
@@ -458,5 +460,8 @@ export async function testInBrowser(options: APIOptions = {}): Promise<void> {
         }
       });
     }
+    if (maybeError != null) {
+      utils.haltOrThrow(maybeError);
+    }
   }
 }
diff --git a/packages/api/src/__tests__/API/E2EFindMemoryLeaksWithSnapshotFiles.test.ts b/packages/api/src/__tests__/API/E2EFindMemoryLeaksWithSnapshotFiles.test.ts
@@ -10,11 +10,12 @@
 
 /* eslint-disable @typescript-eslint/ban-ts-comment */
 
+import type {Page} from 'puppeteer';
 import {fileManager} from '@memlab/core';
 
 import path from 'path';
 import fs from 'fs-extra';
-import {takeSnapshots, findLeaksBySnapshotFilePaths} from '../../index';
+import {run, takeSnapshots, findLeaksBySnapshotFilePaths} from '../../index';
 import {scenario, testSetup, testTimeout} from './lib/E2ETestSettings';
 
 beforeEach(testSetup);
@@ -79,3 +80,45 @@ test(
   },
   testTimeout,
 );
+
+test(
+  'takeSnapshot API allows to throw and catch exceptions from scenario',
+  async () => {
+    const scenarioThatThrows = {...scenario};
+    const errorMessage = 'throw from scenario.action';
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    scenarioThatThrows.action = async (_page: Page): Promise<void> => {
+      throw new Error(errorMessage);
+    };
+
+    expect.assertions(1);
+    await expect(
+      takeSnapshots({
+        scenario: scenarioThatThrows,
+        evalInBrowserAfterInitLoad: injectDetachedDOMElements,
+      }),
+    ).rejects.toThrow(errorMessage);
+  },
+  testTimeout,
+);
+
+test(
+  'run API allows to throw and catch exceptions from scenario',
+  async () => {
+    const scenarioThatThrows = {...scenario};
+    const errorMessage = 'throw from scenario.action';
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    scenarioThatThrows.action = async (_page: Page): Promise<void> => {
+      throw new Error(errorMessage);
+    };
+
+    expect.assertions(1);
+    await expect(
+      run({
+        scenario: scenarioThatThrows,
+        evalInBrowserAfterInitLoad: injectDetachedDOMElements,
+      }),
+    ).rejects.toThrow(errorMessage);
+  },
+  testTimeout,
+);
diff --git a/packages/core/src/lib/Config.ts b/packages/core/src/lib/Config.ts
@@ -163,7 +163,7 @@ export class MemLabConfig {
   qes: QuickExperiment[];
   isOndemand: boolean;
   useExternalSnapshot: boolean;
-  externalRunMetaFile: string;
+  externalRunMetaTemplateFile: string;
   externalSnapshotVisitOrderFile: string;
   externalSnapshotDir: Nullable<string>;
   externalSnapshotFilePaths: string[];

diff --git a/packages/core/src/lib/FileManager.ts b/packages/core/src/lib/FileManager.ts
@@ -636,7 +636,7 @@ export class FileManager {
     config.metricsOutDir = joinAndProcessDir(options, loggerOutDir, 'metrics');
     config.reportScreenshotFile = path.join(outDir, 'report.png');
 
-    config.externalRunMetaFile = this.getRunMetaExternalTemplateFile();
+    config.externalRunMetaTemplateFile = this.getRunMetaExternalTemplateFile();
     config.externalSnapshotVisitOrderFile =
       this.getSnapshotSequenceExternalTemplateFile();
 

diff --git a/packages/core/src/lib/RunInfoUtils.ts b/packages/core/src/lib/RunInfoUtils.ts
@@ -18,12 +18,17 @@ import utils from './Utils';
 import {setInternalValue} from './InternalValueSetter';
 
 export class RunMetaInfoManager {
-  getRunMetaFilePath(options?: {workDir?: Optional<string>}): string {
+  getRunMetaFilePath(options?: {
+    workDir?: Optional<string>;
+    readonly?: Optional<boolean>;
+  }): string {
     if (options?.workDir != null) {
       return fileManager.getRunMetaFile({workDir: options.workDir});
     }
-    if (config.useExternalSnapshot) {
-      return config.externalRunMetaFile;
+    if (options?.readonly && config.useExternalSnapshot) {
+      // only returns the template file if the
+      // run meta file is used for readonly purpose
+      return config.externalRunMetaTemplateFile;
     }
     if (config.runMetaFile != null) {
       return config.runMetaFile;
@@ -64,7 +69,9 @@ export class RunMetaInfoManager {
     metaFile?: Optional<string>;
     workDir?: Optional<string>;
   }): RunMetaInfo {
-    const file = options?.metaFile || this.getRunMetaFilePath(options);
+    const file =
+      options?.metaFile ||
+      this.getRunMetaFilePath({readonly: true, ...options});
     try {
       return this.loadRunMetaInfoFromFile(file);
     } catch (_) {
@@ -78,7 +85,9 @@ export class RunMetaInfoManager {
     metaFile?: Optional<string>;
     workDir?: Optional<string>;
   }): Nullable<RunMetaInfo> {
-    const file = options?.metaFile || this.getRunMetaFilePath(options);
+    const file =
+      options?.metaFile ||
+      this.getRunMetaFilePath({readonly: true, ...options});
     try {
       return this.loadRunMetaInfoFromFile(file);
     } catch (_) {

diff --git a/packages/e2e/src/lib/E2EUtils.ts b/packages/e2e/src/lib/E2EUtils.ts
@@ -179,7 +179,7 @@ async function applyAsyncWithGuard(
   try {
     ret = await f.apply(self, args);
   } catch (ex) {
-    exceptionHandler(utils.getError(ex));
+    await exceptionHandler(utils.getError(ex));
   }
   return ret;
 }

diff --git a/website/docs/api/modules/api_src.md b/website/docs/api/modules/api_src.md
@@ -26,7 +26,7 @@ Options for configuring browser interaction run, all fields are optional
 | `workDir?` | `string` | specify the working directory where you want memlab to dump heap snapshots and other meta data of the test run. If no working directory is provided, memlab will generate a random temp directory under the operating system's default directory for temporary files. Note: It's the caller's responsibility to make sure the specified working directory exists. |
 
  * **Source**:
-    * api/src/API.ts:43
+    * api/src/API.ts:44
 
 ___
 
@@ -40,7 +40,7 @@ A data structure holding the result of the [run](api_src.md#run) API call.
 | `runResult` | [`BrowserInteractionResultReader`](../classes/api_src.BrowserInteractionResultReader.md) | a utility for reading browser interaction results from disk |
 
  * **Source**:
-    * api/src/API.ts:84
+    * api/src/API.ts:85
 
 ## Functions
 
@@ -71,7 +71,7 @@ const {analyze, takeSnapshots, StringAnalysis} = require('@memlab/api');
 ```
 
  * **Source**:
-    * api/src/API.ts:285
+    * api/src/API.ts:286
 
 ___
 
@@ -97,7 +97,7 @@ const {findLeaks, takeSnapshots} = require('@memlab/api');
 ```
 
  * **Source**:
-    * api/src/API.ts:223
+    * api/src/API.ts:224
 
 ___
 
@@ -116,7 +116,7 @@ the `--baseline`, `--target`, and `--final` flags in CLI.
  * **Returns**: `Promise`<`ISerializedInfo`[]\> | leak traces detected and clustered from the browser interaction
 
  * **Source**:
-    * api/src/API.ts:244
+    * api/src/API.ts:245
 
 ___
 
@@ -144,7 +144,7 @@ const {run} = require('@memlab/api');
 ```
 
  * **Source**:
-    * api/src/API.ts:161
+    * api/src/API.ts:162
 
 ___
 
@@ -169,7 +169,7 @@ const {takeSnapshots} = require('@memlab/api');
 ```
 
  * **Source**:
-    * api/src/API.ts:192
+    * api/src/API.ts:193
 
 ___
 
@@ -195,4 +195,4 @@ const {warmupAndTakeSnapshots} = require('@memlab/api');
 ```
 
  * **Source**:
-    * api/src/API.ts:127
+    * api/src/API.ts:128