Repository: elastic/synthetics (ts)

Issue:
propagate errors from `beforeAll` and `afterAll` hooks
+ Now error in the `beforeAll` and `afterAll` hooks would be captured and reported as error, but they will not be associated with the reporters in the correct way. 
+ Without associating these errors, the Uptime UI will have no information about what happened during the context of a single execution. 

We have to figure out a way to solve this problem. 

Pull request description:
fix: capture beforeAll hook errors
+ fix #280 
+ We capture the beforeAll hook errors and any error that happens in any one of the hooks would be captured and all journeys that run on the current invocation will report that error event with `failed` status to be able to captured as part of the Uptime UI. 
+ `afterAll` errors behaves the same - we report them in the stderror logs which will be captured by Heartbeat. 

Patch:
diff --git a/src/core/runner.ts b/src/core/runner.ts
index 2872cf5..649a22a 100644
--- a/src/core/runner.ts
+++ b/src/core/runner.ts
@@ -131,6 +131,7 @@ export default class Runner extends EventEmitter {
   journeys: Journey[] = [];
   hooks: SuiteHooks = { beforeAll: [], afterAll: [] };
   screenshotPath = join(CACHE_PATH, 'screenshots');
+  hookError: Error | undefined;
 
   static async createContext(options: RunOptions): Promise<JourneyContext> {
     const start = monotonicTimeInSeconds();
@@ -291,13 +292,12 @@ export default class Runner extends EventEmitter {
     result: JourneyContext & JourneyResult,
     options: RunOptions
   ) {
-    const { pluginManager, start, params, status, error } = result;
+    const { pluginManager, start, status, error } = result;
     const pluginOutput = await pluginManager.output();
     this.emit('journey:end', {
       journey,
       status,
       error,
-      params,
       start,
       end: monotonicTimeInSeconds(),
       options,
@@ -313,6 +313,33 @@ export default class Runner extends EventEmitter {
     }
   }
 
+  /**
+   * Simulate a journey run to capture errors in the beforeAll hook
+   */
+  async runFakeJourney(journey: Journey, options: RunOptions) {
+    const start = monotonicTimeInSeconds();
+    this.emit('journey:start', {
+      journey,
+      timestamp: getTimestamp(),
+      params: options.params,
+    });
+    const result: JourneyResult = {
+      status: 'failed',
+      error: this.hookError,
+    };
+    this.emit('journey:end', {
+      journey,
+      start,
+      options,
+      end: monotonicTimeInSeconds(),
+      ...result,
+    });
+    if (options.reporter === 'json') {
+      await once(this, 'journey:end:reported');
+    }
+    return result;
+  }
+
   async runJourney(journey: Journey, options: RunOptions) {
     const result: JourneyResult = {
       status: 'succeeded',
@@ -376,7 +403,8 @@ export default class Runner extends EventEmitter {
     await this.runBeforeAllHook({
       env: options.environment,
       params: options.params,
-    });
+    }).catch(e => (this.hookError = e));
+
     const { dryRun, match, tags } = options;
     for (const journey of this.journeys) {
       /**
@@ -389,7 +417,9 @@ export default class Runner extends EventEmitter {
       if (!journey.isMatch(match, tags)) {
         continue;
       }
-      const journeyResult = await this.runJourney(journey, options);
+      const journeyResult: JourneyResult = this.hookError
+        ? await this.runFakeJourney(journey, options)
+        : await this.runJourney(journey, options);
       result[journey.name] = journeyResult;
     }
     await Gatherer.stop();
diff --git a/src/reporters/json.ts b/src/reporters/json.ts
index f169df5..08d13bc 100644
--- a/src/reporters/json.ts
+++ b/src/reporters/json.ts
@@ -332,7 +332,6 @@ export async function gatherScreenshots(
   screenshotsPath: string,
   callback: (step: Step, data: string) => Promise<void>
 ) {
-  const screenshots: Array<ScreenshotOutput> = [];
   if (isDirectory(screenshotsPath)) {
     await totalist(screenshotsPath, async (_, absPath) => {
       try {
@@ -344,7 +343,6 @@ export async function gatherScreenshots(
       }
     });
   }
-  return screenshots;
 }
 
 export default class JSONReporter extends BaseReporter {
@@ -425,6 +423,9 @@ export default class JSONReporter extends BaseReporter {
           await gatherScreenshots(
             join(CACHE_PATH, 'screenshots'),
             async (step, data) => {
+              if (!data) {
+                return;
+              }
               if (ssblocks) {
                 await this.writeScreenshotBlocks(journey, step, data);
               } else {


Test patch:
diff --git a/__tests__/core/runner.test.ts b/__tests__/core/runner.test.ts
index a60b6ed..d4ba7d9 100644
--- a/__tests__/core/runner.test.ts
+++ b/__tests__/core/runner.test.ts
@@ -147,6 +147,23 @@ describe('runner', () => {
     });
   });
 
+  it('run journey - failed on beforeAll', async () => {
+    const error = new Error('Broken beforeAll hook');
+    runner.addHook('beforeAll', () => {
+      throw error;
+    });
+    runner.addJourney(new Journey({ name: 'j1' }, () => step('step1', noop)));
+    runner.addJourney(new Journey({ name: 'j2' }, () => step('step1', noop)));
+    const result = await runner.run({
+      wsEndpoint,
+      outfd: fs.openSync(dest, 'w'),
+    });
+    expect(result).toEqual({
+      j1: { status: 'failed', error },
+      j2: { status: 'failed', error },
+    });
+  });
+
   it('run step', async () => {
     const j1 = journey('j1', async ({ page }) => {
       step('step1', async () => {