✨ Adding env variable support so that if set to true we exit with sta…

…tus code 0 when any percy related error occurs. (#1647) * adding support for PERCY_EXIT_WITH_ZERO_ON_ERROR env variable that exit with status 0 on any percy error. * adding logs info regarding the status by which the customer command exited * adding tests * adding tests * test fix * test fix
percy · Jul 2, 2024 · 865bab5 · 865bab5
1 parent a1114f1
commit 865bab5
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 10 deletions.
diff --git a/packages/cli-command/src/command.js b/packages/cli-command/src/command.js
@@ -53,8 +53,12 @@ function withBuiltIns(definition) {
 }
 
 // Helper to throw an error with an exit code and optional reason message
-function exit(exitCode, reason = '') {
+function exit(exitCode, reason = '', shouldOverrideExitCode = true) {
+  let percyExitWithZeroOnError = process.env.PERCY_EXIT_WITH_ZERO_ON_ERROR === 'true';
+  exitCode = percyExitWithZeroOnError && shouldOverrideExitCode ? 0 : exitCode;
   let err = reason instanceof Error ? reason : new Error(reason);
+  // Adding additional object so that it can be used in runner function below.
+  err.shouldOverrideExitCode = shouldOverrideExitCode;
   throw Object.assign(err, { exitCode });
 }
 
@@ -155,7 +159,10 @@ export function command(name, definition, callback) {
         err.message ||= `EEXIT: ${err.exitCode}`;
 
         if (definition.exitOnError) {
-          process.exit(err.exitCode);
+          let shouldOverrideExitCode = err.shouldOverrideExitCode !== false;
+          let percyExitWithZeroOnError = process.env.PERCY_EXIT_WITH_ZERO_ON_ERROR === 'true';
+          let exitCode = percyExitWithZeroOnError && shouldOverrideExitCode ? 0 : err.exitCode;
+          process.exit(exitCode);
         }
 
         // re-throw when not exiting

diff --git a/packages/cli-command/test/command.test.js b/packages/cli-command/test/command.test.js
@@ -160,7 +160,22 @@ describe('Command', () => {
     });
 
     await expectAsync(test()).toBeRejected();
-    expect(spy).toHaveBeenCalled();
+    expect(spy).toHaveBeenCalledWith(1);
+  });
+
+  it('handles maybe exiting on unhandled errors and exits with status 0 when PERCY_EXIT_WITH_ZERO_ON_ERROR is set to true', async () => {
+    process.env.PERCY_EXIT_WITH_ZERO_ON_ERROR = 'true';
+    let spy = spyOn(process, 'exit');
+
+    let test = command('test', {
+      exitOnError: true
+    }, () => {
+      throw new Error();
+    });
+
+    await expectAsync(test()).toBeRejected();
+    expect(spy).toHaveBeenCalledWith(0);
+    delete process.env.PERCY_EXIT_WITH_ZERO_ON_ERROR;
   });
 
   it('handles graceful exit errors', async () => {
@@ -180,8 +195,13 @@ describe('Command', () => {
         message = new Error(message);
       }
 
-      exit(code, message);
-      test.never = true;
+      if (code > 400) {
+        exit(code, message, false);
+        test.never = true;
+      } else {
+        exit(code, message);
+        test.never = true;
+      }
     });
 
     await test(['123']).catch(e => {
@@ -197,6 +217,7 @@ describe('Command', () => {
     await test(['456', 'Reason']).catch(e => {
       expect(e).toHaveProperty('message', 'Reason');
       expect(e).toHaveProperty('exitCode', 456);
+      expect(e).toHaveProperty('shouldOverrideExitCode', false);
     });
 
     expect(test).not.toHaveProperty('never');
@@ -214,6 +235,63 @@ describe('Command', () => {
     ]);
   });
 
+  it('exits with status 0 when PERCY_EXIT_WITH_ZERO_ON_ERROR is set to true', async () => {
+    process.env.PERCY_EXIT_WITH_ZERO_ON_ERROR = 'true';
+    let test = command('test', {
+      args: [{
+        name: 'code',
+        parse: Number,
+        required: true
+      }, {
+        name: 'message'
+      }, {
+        shouldOverrideExitCode: 'shouldOverrideExitCode'
+      }]
+    }, ({ args, exit }) => {
+      let { code, message } = args;
+
+      // exercise coverage
+      if (message && code >= 1) {
+        message = new Error(message);
+      }
+
+      if (code > 400) {
+        exit(code, message, false);
+        test.never = true;
+      } else {
+        exit(code, message);
+        test.never = true;
+      }
+    });
+
+    await test(['123']).catch(e => {
+      expect(e).toHaveProperty('exitCode', 0);
+      expect(e).toHaveProperty('shouldOverrideExitCode', true);
+    });
+
+    expect(test).not.toHaveProperty('never');
+    expect(logger.stderr).toEqual([]);
+
+    logger.reset();
+
+    await test(['123', 'Reason']).catch(e => {
+      expect(e).toHaveProperty('message', 'Reason');
+      expect(e).toHaveProperty('exitCode', 0);
+      expect(e).toHaveProperty('shouldOverrideExitCode', true);
+    });
+
+    await test(['401', 'some reason']).catch(e => {
+      expect(e).toHaveProperty('message', 'some reason');
+      expect(e).toHaveProperty('exitCode', 401);
+      expect(e).toHaveProperty('shouldOverrideExitCode', false);
+    });
+
+    expect(test).not.toHaveProperty('never');
+
+    logger.reset();
+    delete process.env.PERCY_EXIT_WITH_ZERO_ON_ERROR;
+  });
+
   it('handles interrupting generator actions', async () => {
     let sleep = (ms, v) => new Promise(r => setTimeout(r, ms, v));
 

diff --git a/packages/cli-exec/src/exec.js b/packages/cli-exec/src/exec.js
@@ -46,14 +46,14 @@ export const exec = command('exec', {
     log.error("You must supply a command to run after '--'");
     log.info('Example:');
     log.info('  $ percy exec -- npm test');
-    exit(1);
+    exit(1, '', false);
   }
 
   // verify the provided command exists
   let { default: which } = await import('which');
 
   if (!which.sync(command, { nothrow: true })) {
-    exit(127, `Command not found "${command}"`);
+    exit(127, `Command not found "${command}"`, false);
   }
 
   // attempt to start percy if enabled
@@ -90,8 +90,9 @@ export const exec = command('exec', {
   // stop percy if running (force stop if there is an error);
   await percy?.stop(!!error);
 
+  log.info(`Command "${[command, ...args].join(' ')}" exited with status: ${status}`);
   // forward any returned status code
-  if (status) exit(status, error);
+  if (status) exit(status, error, false);
 
   // force exit post timeout
   await waitForTimeout(10000);

diff --git a/packages/cli-exec/test/exec.test.js b/packages/cli-exec/test/exec.test.js
@@ -88,7 +88,8 @@ describe('percy exec', () => {
       '[percy] Percy has started!',
       '[percy] Running "node --eval "',
       '[percy] Finalized build #1: https://percy.io/test/test/123',
-      "[percy] Build's CLI logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha"
+      "[percy] Build's CLI logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha",
+      '[percy] Command "node --eval " exited with status: 0'
     ]);
   });
 
@@ -112,7 +113,8 @@ describe('percy exec', () => {
       '[percy] Percy is disabled'
     ]);
     expect(logger.stdout).toEqual([
-      '[percy] Running "node --eval "'
+      '[percy] Running "node --eval "',
+      '[percy] Command "node --eval " exited with status: 0'
     ]);
   });