fix(semver): 🐞 handle child process error gracefully

Fixes #196

packages/semver/src/executors/version/index.e2e.spec.ts

@@ -49,7 +49,11 @@ describe('@jscutlery/semver:version', () => {
   let result: { success: boolean };
   let testingWorkspace: TestingWorkspace;
 
-  beforeAll(() => jest.spyOn(console, 'info').mockImplementation());
+  beforeAll(() => {
+    jest.spyOn(console, 'warn').mockImplementation();
+    jest.spyOn(console, 'info').mockImplementation();
+  });
+
   afterAll(() => (console.info as jest.Mock).mockRestore());
 
   describe('package "a" with (--sync-versions=false)', () => {

packages/semver/src/executors/version/index.spec.ts

@@ -1,7 +1,7 @@
 import { logger } from '@nrwl/devkit';
 import { ExecutorContext } from '@nrwl/tao/src/shared/workspace';
 import { execFile } from 'child_process';
-import { of } from 'rxjs';
+import { of, throwError } from 'rxjs';
 import * as standardVersion from 'standard-version';
 import * as changelog from 'standard-version/lib/lifecycles/changelog';
 import { callbackify } from 'util';
@@ -55,6 +55,7 @@ describe('@jscutlery/semver:version', () => {
     });
 
     jest.spyOn(logger, 'info');
+    jest.spyOn(logger, 'error');
 
     mockChangelog.mockResolvedValue(undefined);
     mockTryBump.mockReturnValue(of('2.1.0'));
@@ -113,14 +114,17 @@ describe('@jscutlery/semver:version', () => {
     });
 
     it('should run standard-version with a custom tag', async () => {
-      const { success } = await version({...options, versionTagPrefix: 'custom-tag-prefix/${target}-' }, context);
+      const { success } = await version(
+        { ...options, versionTagPrefix: 'custom-tag-prefix/${target}-' },
+        context
+      );
 
       expect(success).toBe(true);
       expect(standardVersion).toBeCalledWith(
         expect.objectContaining({
           header: expect.any(String),
           dryRun: false,
-          tagPrefix: "custom-tag-prefix/a-",
+          tagPrefix: 'custom-tag-prefix/a-',
         })
       );
     });
@@ -276,14 +280,9 @@ describe('@jscutlery/semver:version', () => {
 
   describe('Git push', () => {
     it('should push to Git', async () => {
-      mockTryPushToGitRemote.mockReturnValue(
-        of({ stderr: '', stdout: 'success' })
-      );
+      mockTryPushToGitRemote.mockReturnValue(of('success'));
 
-      const { success } = await version(
-        { ...options, push: true },
-        context
-      );
+      const { success } = await version({ ...options, push: true }, context);
 
       expect(success).toBe(true);
       expect(mockTryPushToGitRemote).toHaveBeenCalledWith(
@@ -295,6 +294,19 @@ describe('@jscutlery/semver:version', () => {
       );
     });
 
+    it('should handle Git failure', async () => {
+      mockTryPushToGitRemote.mockReturnValue(
+        throwError(new Error('Something went wrong'))
+      );
+
+      const { success } = await version({ ...options, push: true }, context);
+
+      expect(success).toBe(false);
+      expect(logger.error).toBeCalledWith(
+        expect.stringContaining('Error: Something went wrong')
+      );
+    });
+
     it('should not push to Git by default', async () => {
       await version(options, context);
       expect(mockTryPushToGitRemote).not.toHaveBeenCalled();

packages/semver/src/executors/version/index.ts

@@ -30,7 +30,7 @@ export default function version(
 ): Promise<{ success: boolean }> {
   const workspaceRoot = context.root;
   const preset = 'angular';
-  const tagPrefix = versionTagPrefix !== undefined 
+  const tagPrefix = versionTagPrefix !== undefined
     ? resolveTagTemplate(
       versionTagPrefix,
       { target: context.projectName, projectName: context.projectName }

packages/semver/src/executors/version/utils/exec-async.spec.ts

@@ -35,17 +35,12 @@ describe('_execAsync (Promise)', () => {
   });
 
   it('should handle failure', async () => {
-    try {
-      await  _execAsync('exit 1');
-      fail();
-    } catch (error) {
-      expect(error).toEqual(
-        expect.objectContaining({
-          cmd: 'exit 1',
-          stderr: '',
-          stdout: '',
-        })
-      );
-    }
+    await expect(_execAsync('exit 1')).rejects.toEqual(
+      expect.objectContaining({
+        error: expect.objectContaining({ cmd: 'exit 1' }),
+        stderr: '',
+        stdout: '',
+      })
+    );
   });
 });

packages/semver/src/executors/version/utils/exec-async.ts

@@ -1,18 +1,20 @@
-import { exec } from 'child_process';
+import { exec, ExecException } from 'child_process';
 import { defer } from 'rxjs';
 
+export type ChildProcessResponse = { error?: ExecException | null, stderr: string; stdout: string };
+
 export function execAsync(cmd: string, args: string[]) {
   return defer(() => _execAsync(cmd, args));
 }
 
-export function _execAsync(cmd: string, args: string[] = []): Promise<{ stderr: string; stdout: string }> {
+export function _execAsync(cmd: string, args: string[] = []): Promise<ChildProcessResponse> {
   return new Promise((resolve, reject) => {
     exec(
       `${cmd} ${args.length > 0 ? args.join(' '): ''}`.trim(),
       { cwd: process.cwd() },
       (error, stdout, stderr) => {
         if (error) {
-          reject({ ...error, stdout, stderr });
+          reject({ error, stdout, stderr });
           return;
         }
 

packages/semver/src/executors/version/utils/git.spec.ts

@@ -122,36 +122,30 @@ describe('git', () => {
       it(`should throw if Git push failed`, async () => {
         jest
           .spyOn(cp, 'execAsync')
-          .mockReturnValue(throwError({ stderr: 'failed', stdout: '' }));
+          .mockReturnValue(
+            throwError({ stderr: 'Something went wrong', stdout: '' })
+          );
 
-        try {
-          await tryPushToGitRemote({
+        await expect(
+          tryPushToGitRemote({
             remote: 'origin',
             branch: 'master',
             noVerify: false,
-          }).toPromise();
-          fail();
-        } catch (error) {
-          expect(cp.execAsync).toBeCalledTimes(1);
-          expect(error).toEqual(
-            expect.objectContaining({ stderr: 'failed', stdout: '' })
-          );
-        }
+          }).toPromise()
+        ).rejects.toEqual(new Error('Something went wrong'));
+        expect(cp.execAsync).toBeCalledTimes(1);
       });
 
       it('should fail if options are undefined', async () => {
-        try {
-          await tryPushToGitRemote({
+        await expect(
+          tryPushToGitRemote({
             /* eslint-disable @typescript-eslint/no-explicit-any */
             remote: undefined as any,
             branch: undefined as any,
             /* eslint-enable @typescript-eslint/no-explicit-any */
             noVerify: false,
-          }).toPromise();
-          fail();
-        } catch (error) {
-          expect(error.message).toContain('Missing Git options');
-        }
+          }).toPromise()
+        ).rejects.toEqual(expect.any(Error));
       });
     });
   });
@@ -169,7 +163,11 @@ describe('git', () => {
 
       expect(cp.execAsync).toBeCalledWith(
         'git',
-        expect.arrayContaining(['add', 'packages/demo/file.txt', 'packages/demo/other-file.ts'])
+        expect.arrayContaining([
+          'add',
+          'packages/demo/file.txt',
+          'packages/demo/other-file.ts',
+        ])
       );
     });
 
@@ -187,7 +185,7 @@ describe('git', () => {
     });
   });
 
-    describe('getFirstCommitRef', () => {
+  describe('getFirstCommitRef', () => {
     it('should get last git commit', async () => {
       jest
         .spyOn(cp, 'execAsync')

packages/semver/src/executors/version/utils/git.ts

@@ -1,6 +1,6 @@
 import * as gitRawCommits from 'git-raw-commits';
-import { defer, Observable, of, throwError, EMPTY } from 'rxjs';
-import { catchError, last, scan, startWith, switchMap } from 'rxjs/operators';
+import { defer, EMPTY, Observable, throwError } from 'rxjs';
+import { catchError, last, map, scan, startWith } from 'rxjs/operators';
 
 import { execAsync } from './exec-async';
 
@@ -38,10 +38,7 @@ export function tryPushToGitRemote({
   remote: string;
   branch: string;
   noVerify: boolean;
-}): Observable<{
-  stderr: string;
-  stdout: string;
-}> {
+}): Observable<string> {
   return defer(() => {
     if (remote == null || branch == null) {
       return throwError(
@@ -71,13 +68,18 @@ export function tryPushToGitRemote({
         ) {
           console.warn('git push --atomic failed, attempting non-atomic push');
 
-          return execAsync('git', ['push', ...gitPushOptions, remote, branch]);
+          return execAsync('git', [
+            'push',
+            ...gitPushOptions,
+            remote,
+            branch,
+          ]).pipe(catchError((error) => throwError(new Error(error.stderr))));
         }
 
-        return throwError(error);
+        return throwError(new Error(error.stderr));
       })
     );
-  });
+  }).pipe(map((process) => process.stdout));
 }
 
 export function addToStage({
@@ -86,18 +88,22 @@ export function addToStage({
 }: {
   paths: string[];
   dryRun: boolean;
-}): Observable<{ stderr: string; stdout: string }> {
+}): Observable<string> {
   if (paths.length === 0) {
     return EMPTY;
   }
 
   const gitAddOptions = [...(dryRun ? ['--dry-run'] : []), ...paths];
-  return execAsync('git', ['add', ...gitAddOptions]);
+  return execAsync('git', ['add', ...gitAddOptions]).pipe(
+    map((process) => process.stdout),
+    catchError((error) => throwError(new Error(error.stderr)))
+  );
 }
 
 export function getFirstCommitRef(): Observable<string> {
   return execAsync('git', ['rev-list', '--max-parents=0', 'HEAD']).pipe(
-    /**                                 Remove line breaks. */
-    switchMap(({ stdout }) => of(stdout.replace(/\r?\n|\r/, '')))
+    /**                                Remove line breaks. */
+    map(({ stdout }) => stdout.replace(/\r?\n|\r/, '')),
+    catchError((error) => throwError(new Error(error.stderr)))
   );
 }