Multi step error fix (#2107)

* Make stopAsError report failures.

The old version was kind of broken that returned partly result json.

This fix makes sure the test return as a failure (as documented) and
remove all result data (as documented).

sitespeedio/sitespeed.io#4113

lib/core/engine/command/measure.js

@@ -205,19 +205,22 @@ export class Measure {
         } else {
           this.result.at(-1).error.push(message);
         }
-      } else {
-        //  error not associated to a start/run so far
-        log.error('No page that could be associated with the error:' + message);
       }
     }
+    // Add the error in the generic failure
+    if (this.result.failureMessages) {
+      this.result.failureMessages.push(message);
+    } else {
+      this.result.failureMessages = [message];
+    }
   }
 
   /**
    * Have a consistent way of adding an failure
    * @private
    */
   _failure(message) {
-    log.debug('Mark run as failure with message %s', message);
+    log.info('Mark run as failure with message: %s', message);
     this.result.markedAsFailure = 1;
     if (this.result.failureMessages) {
       this.result.failureMessages.push(message);
@@ -378,14 +381,22 @@ export class Measure {
    *
    * @async
    * @param {string} errorMessage - The message about the error. This will end up on the HTML report for sitespeed.io so give it a good message so you know what's gone wrong.
-   *  @param {string} optionalURL - The URL of the page that you wanted to test. If you don't know the URL (you clicked on a link etc) skip it.
    * @returns {Promise} A promise that resolves when the stop process has completed.
    * @since 21.2.0
    */
-  async stopAsError(errorMessage, optionalURL) {
-    this._error(errorMessage);
+  async stopAsError(errorMessage) {
+    this._failure(errorMessage);
     this.areWeMeasuring = false;
-    return this.stop(optionalURL, false);
+    // Remove the last result
+    this.result.pop();
+
+    await this._stopVideo();
+
+    if (this.options.tcpdump) {
+      await this.tcpDump.stop();
+    }
+
+    return;
   }
   /**
    * Stops the measurement process, collects metrics, and handles any post-measurement tasks.
@@ -397,7 +408,7 @@ export class Measure {
    * @throws {Error} Throws an error if there are issues in stopping the measurement or collecting data.
    * @returns {Promise} A promise that resolves with the collected metrics data.
    */
-  async stop(testedStartUrl, collectMeasurements = true) {
+  async stop(testedStartUrl) {
     log.debug('Stop measuring');
     // If we don't have a URL (tested using clicking on link etc) get the URL from the browser
     let url =
@@ -454,6 +465,7 @@ export class Measure {
     ) {
       res.alias = this.options.urlMetaData[url];
     }
+
     this.result[this.numberOfMeasuredPages] = merge(
       this.result[this.numberOfMeasuredPages],
       res
@@ -463,7 +475,7 @@ export class Measure {
       await this.tcpDump.stop();
     }
 
-    return collectMeasurements === false ? undefined : this.collect(url);
+    return this.collect(url);
   }
 
   /**

lib/core/engine/index.js

@@ -321,6 +321,7 @@ export class Engine {
         if (data.error) {
           errorsOutsideTheBrowser.push(...data.error);
         }
+
         // Catch failures like starting the browser
         if (data.markedAsFailure) {
           failures.push(...data.failureMessages);

lib/video/screenRecording/desktop/desktopRecorder.js

@@ -53,6 +53,12 @@ export class DesktopRecorder {
   async stop(destination) {
     log.debug('Stop screen recording');
     await _stop(this.recording);
+
+    // This was a test with and error and probably not a navigation
+    // The user script use stopAsError
+    if (!destination) {
+      return;
+    }
     // FIXME update to rename/move file
     // The destination file could exixt of we use --resultDir
     // so make sure we remove it first

lib/video/video.js

@@ -58,14 +58,18 @@ export class Video {
    */
   async stop(url) {
     this.isRecording = false;
-    this.videoPath = join(
-      this.storageManager.directory,
-      pathToFolder(url, this.options),
-      'video',
-      this.index + '.mp4'
-    );
-    await this.setupDirs(this.index, url);
-    return this.recorder.stop(this.videoPath);
+    if (url === undefined) {
+      return this.recorder.stop(this.videoPath);
+    } else {
+      this.videoPath = join(
+        this.storageManager.directory,
+        pathToFolder(url, this.options),
+        'video',
+        this.index + '.mp4'
+      );
+      await this.setupDirs(this.index, url);
+      return this.recorder.stop(this.videoPath);
+    }
   }
 
   async cleanup() {