Repository: steveukx/git-js
Pull request #604 (2021-03-12 09:05:36)

Feature/error detection plugin
By default, `simple-git` will determine that a `git` task has resulted in an error when the process exit
code is anything other than `0` and there has been some data sent to the `stdErr` stream. Error handlers
will be passed the content of both `stdOut` and `stdErr` concatenated together. 

To change any of this behaviour, configure the `simple-git` with the `errors` plugin with a function to be
called after every task has been run and should return either `undefined` when the task is treated as
a success, or a `Buffer` or `Error` when the task should be treated as a failure.

When the default error handler (or any other plugin) has thrown an error, the first argument to the error
detection plugin is the original error. Either return that error directly to allow it to bubble up to the
task's error handlers, or implement your own error detection as below:

```typescript
import simpleGit from 'simple-git';
const git = simpleGit({
   errors(error, result) {
      // optionally pass through any errors reported before this plugin runs
      if (error) return error;
      // customise the `errorCode` values to treat as success
      if (result.errorCode === 0) {
         return;
      }
      // the default error messages include both stdOut and stdErr, but that
      // can be changed here, or completely replaced with some other content
      return Buffer.concat([...result.stdOut, ...result.stdErr]);
   }
})
```


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 508fbd5..1abfdd9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,12 +1,5 @@
 # Change History & Release Notes
 
-### [2.36.2](https://www.github.com/steveukx/git-js/compare/v2.36.1...v2.36.2) (2021-03-11)
-
-
-### Bug Fixes
-
-* Export missing `SimpleGitProgressEvent` ([038870e](https://www.github.com/steveukx/git-js/commit/038870eb9ae35be78c1dd7fe1977ad8ba35913f2)), closes [#601](https://www.github.com/steveukx/git-js/issues/601)
-
 ### [2.36.1](https://www.github.com/steveukx/git-js/compare/v2.36.0...v2.36.1) (2021-03-06)
 
 
diff --git a/docs/PLUGIN-ERRORS.md b/docs/PLUGIN-ERRORS.md
new file mode 100644
index 0000000..b5d062e
--- /dev/null
+++ b/docs/PLUGIN-ERRORS.md
@@ -0,0 +1,33 @@
+## Custom Error Detection
+
+By default, `simple-git` will determine that a `git` task has resulted in an error when the process exit
+code is anything other than `0` and there has been some data sent to the `stdErr` stream. Error handlers
+will be passed the content of both `stdOut` and `stdErr` concatenated together. 
+
+To change any of this behaviour, configure the `simple-git` with the `errors` plugin with a function to be
+called after every task has been run and should return either `undefined` when the task is treated as
+a success, or a `Buffer` or `Error` when the task should be treated as a failure.
+
+When the default error handler (or any other plugin) has thrown an error, the first argument to the error
+detection plugin is the original error. Either return that error directly to allow it to bubble up to the
+task's error handlers, or implement your own error detection as below:
+
+```typescript
+import simpleGit from 'simple-git';
+
+const git = simpleGit({
+   errors(error, result) {
+      // optionally pass through any errors reported before this plugin runs
+      if (error) return error;
+
+      // customise the `errorCode` values to treat as success
+      if (result.errorCode === 0) {
+         return;
+      }
+
+      // the default error messages include both stdOut and stdErr, but that
+      // can be changed here, or completely replaced with some other content
+      return Buffer.concat([...result.stdOut, ...result.stdErr]);
+   }
+})
+```
diff --git a/package.json b/package.json
index 17a542b..6fdcc87 100644
--- a/package.json
+++ b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "simple-git",
   "description": "Simple GIT interface for node.js",
-  "version": "2.36.2",
+  "version": "2.36.1",
   "author": "Steve King <steve@mydev.co>",
   "contributors": [
     {
diff --git a/readme.md b/readme.md
index 578aaae..9d44d81 100644
--- a/readme.md
+++ b/readme.md
@@ -79,6 +79,9 @@ await git.pull();
 
 ## Configuring Plugins
 
+- [Error Detection](./docs/PLUGIN-ERRORS.md)
+  Customise the detection of errors from the underlying `git` process.
+
 - [Progress Events](./docs/PLUGIN-PROGRESS-EVENTS.md)
   Receive progress events as `git` works through long-running processes.
 
diff --git a/src/lib/git-factory.ts b/src/lib/git-factory.ts
index 669a3fd..8216b3c 100644
--- a/src/lib/git-factory.ts
+++ b/src/lib/git-factory.ts
@@ -1,12 +1,19 @@
-const Git = require('../git');
-
 import { SimpleGitFactory } from '../../typings';
 
 import api from './api';
-import { commandConfigPrefixingPlugin, PluginStore, progressMonitorPlugin, timeoutPlugin } from './plugins';
+import {
+   commandConfigPrefixingPlugin,
+   errorDetectionHandler,
+   errorDetectionPlugin,
+   PluginStore,
+   progressMonitorPlugin,
+   timeoutPlugin
+} from './plugins';
 import { createInstanceConfig, folderExists } from './utils';
 import { SimpleGitOptions } from './types';
 
+const Git = require('../git');
+
 /**
  * Adds the necessary properties to the supplied object to enable it for use as
  * the default export of a module.
@@ -47,5 +54,8 @@ export function gitInstanceFactory(baseDir?: string | Partial<SimpleGitOptions>,
    config.progress && plugins.add(progressMonitorPlugin(config.progress));
    config.timeout && plugins.add(timeoutPlugin(config.timeout));
 
+   plugins.add(errorDetectionPlugin(errorDetectionHandler(true)));
+   config.errors && plugins.add(errorDetectionPlugin(config.errors));
+
    return new Git(config, plugins);
 }
diff --git a/src/lib/parsers/parse-branch-delete.ts b/src/lib/parsers/parse-branch-delete.ts
index da0cd5c..15d6d70 100644
--- a/src/lib/parsers/parse-branch-delete.ts
+++ b/src/lib/parsers/parse-branch-delete.ts
@@ -22,8 +22,8 @@ const parsers: LineParser<BranchMultiDeleteResult>[] = [
    }),
 ];
 
-export const parseBranchDeletions: TaskParser<string, BranchMultiDeleteResult> = (stdOut: string) => {
-   return parseStringResponse(new BranchDeletionBatch(), parsers, stdOut);
+export const parseBranchDeletions: TaskParser<string, BranchMultiDeleteResult> = (stdOut, stdErr) => {
+   return parseStringResponse(new BranchDeletionBatch(), parsers, stdOut, stdErr);
 }
 
 export function hasBranchDeletionError(data: string, processExitCode: ExitCodes): boolean {
diff --git a/src/lib/plugins/error-detection.plugin.ts b/src/lib/plugins/error-detection.plugin.ts
new file mode 100644
index 0000000..cf54209
--- /dev/null
+++ b/src/lib/plugins/error-detection.plugin.ts
@@ -0,0 +1,47 @@
+import { GitError } from '../errors/git-error';
+import { GitExecutorResult, SimpleGitPluginConfig } from '../types';
+import { SimpleGitPlugin } from './simple-git-plugin';
+
+type TaskResult = Omit<GitExecutorResult, 'rejection'>;
+
+function isTaskError (result: TaskResult) {
+   return !!(result.exitCode && result.stdErr.length);
+}
+
+function getErrorMessage (result: TaskResult) {
+   return Buffer.concat([...result.stdOut, ...result.stdErr]);
+}
+
+export function errorDetectionHandler (overwrite = false, isError = isTaskError, errorMessage: (result: TaskResult) => Buffer | Error = getErrorMessage) {
+
+   return (error: Buffer | Error | undefined, result: TaskResult) => {
+      if ((!overwrite && error) || !isError(result)) {
+         return error;
+      }
+
+      return errorMessage(result);
+   };
+}
+
+export function errorDetectionPlugin(config: SimpleGitPluginConfig['errors']): SimpleGitPlugin<'task.error'> {
+
+   return {
+      type: 'task.error',
+      action(data, context) {
+         const error = config(data.error, {
+            stdErr: context.stdErr,
+            stdOut: context.stdOut,
+            exitCode: context.exitCode
+         });
+
+         if (Buffer.isBuffer(error)) {
+            return {error: new GitError(undefined, error.toString('utf-8'))};
+         }
+
+         return {
+            error
+         };
+      },
+   };
+
+}
diff --git a/src/lib/plugins/index.ts b/src/lib/plugins/index.ts
index fd14965..f005ffe 100644
--- a/src/lib/plugins/index.ts
+++ b/src/lib/plugins/index.ts
@@ -1,4 +1,5 @@
 export * from './command-config-prefixing-plugin';
+export * from './error-detection.plugin';
 export * from './plugin-store';
 export * from './progress-monitor-plugin';
 export * from './simple-git-plugin';
diff --git a/src/lib/plugins/simple-git-plugin.ts b/src/lib/plugins/simple-git-plugin.ts
index 19355c5..7e564a0 100644
--- a/src/lib/plugins/simple-git-plugin.ts
+++ b/src/lib/plugins/simple-git-plugin.ts
@@ -1,4 +1,5 @@
 import { ChildProcess } from 'child_process';
+import { GitExecutorResult } from '../types';
 
 type SimpleGitTaskPluginContext = {
    readonly method: string;
@@ -16,7 +17,11 @@ export interface SimpleGitPluginTypes {
          spawned: ChildProcess;
          kill (reason: Error): void;
       };
-   }
+   },
+   'task.error': {
+      data: { error?: Error };
+      context: SimpleGitTaskPluginContext & GitExecutorResult;
+   },
 }
 
 export type SimpleGitPluginType = keyof SimpleGitPluginTypes;
diff --git a/src/lib/runners/git-executor-chain.ts b/src/lib/runners/git-executor-chain.ts
index ddae5b3..f4727c9 100644
--- a/src/lib/runners/git-executor-chain.ts
+++ b/src/lib/runners/git-executor-chain.ts
@@ -3,15 +3,7 @@ import { GitError } from '../errors/git-error';
 import { OutputLogger } from '../git-logger';
 import { PluginStore } from '../plugins';
 import { EmptyTask, isBufferTask, isEmptyTask, } from '../tasks/task';
-import {
-   GitExecutorResult,
-   Maybe,
-   outputHandler,
-   RunnableTask,
-   SimpleGitExecutor,
-   SimpleGitTask,
-   TaskResponseFormat
-} from '../types';
+import { GitExecutorResult, Maybe, outputHandler, RunnableTask, SimpleGitExecutor, SimpleGitTask } from '../types';
 import { callTaskParser, first, GitOutputStreams, objectToString } from '../utils';
 import { Scheduler } from './scheduler';
 import { TasksPendingQueue } from './tasks-pending-queue';
@@ -88,7 +80,7 @@ export class GitExecutorChain implements SimpleGitExecutor {
          task,
          this.binary, args, this.outputHandler, logger.step('SPAWN'),
       );
-      const outputStreams = await this.handleTaskData(task, raw, logger.step('HANDLE'));
+      const outputStreams = await this.handleTaskData(task, args, raw, logger.step('HANDLE'));
 
       logger(`passing response to task's parser as a %s`, task.format);
 
@@ -105,27 +97,32 @@ export class GitExecutorChain implements SimpleGitExecutor {
    }
 
    private handleTaskData<R>(
-      {onError, concatStdErr}: SimpleGitTask<R>,
-      {exitCode, rejection, stdOut, stdErr}: GitExecutorResult, logger: OutputLogger): Promise<GitOutputStreams> {
+      task: SimpleGitTask<R>,
+      args: string[],
+      result: GitExecutorResult, logger: OutputLogger): Promise<GitOutputStreams> {
+
+      const {exitCode, rejection, stdOut, stdErr} = result;
 
       return new Promise((done, fail) => {
          logger(`Preparing to handle process response exitCode=%d stdOut=`, exitCode);
 
-         const failed = !!(rejection || (exitCode && stdErr.length));
+         const {error} = this._plugins.exec('task.error', {error: rejection}, {
+            ...pluginContext(task, args),
+            ...result,
+         });
 
-         if (failed && onError) {
+         if (error && task.onError) {
             logger.info(`exitCode=%s handling with custom error handler`);
-            logger(`concatenate stdErr to stdOut: %j`, concatStdErr);
 
-            return onError(
-               exitCode,
-               Buffer.concat([...(concatStdErr ? stdOut : []), ...stdErr]).toString('utf-8'),
-               (result: TaskResponseFormat) => {
+            return task.onError(
+               result,
+               error,
+               (newStdOut) => {
                   logger.info(`custom error handler treated as success`);
-                  logger(`custom error returned a %s`, objectToString(result));
+                  logger(`custom error returned a %s`, objectToString(newStdOut));
 
                   done(new GitOutputStreams(
-                     Buffer.isBuffer(result) ? result : Buffer.from(String(result)),
+                     Array.isArray(newStdOut) ? Buffer.concat(newStdOut) : newStdOut,
                      Buffer.concat(stdErr),
                   ));
                },
@@ -133,15 +130,9 @@ export class GitExecutorChain implements SimpleGitExecutor {
             );
          }
 
-         if (failed) {
+         if (error) {
             logger.info(`handling as error: exitCode=%s stdErr=%s rejection=%o`, exitCode, stdErr.length, rejection);
-            return fail(rejection || Buffer.concat(stdErr).toString('utf-8'));
-         }
-
-         if (concatStdErr) {
-            logger(`concatenating stdErr onto stdOut before processing`);
-            logger(`stdErr: $O`, stdErr);
-            stdOut.push(...stdErr);
+            return fail(error);
          }
 
          logger.info(`retrieving task output complete`);
@@ -210,7 +201,7 @@ export class GitExecutorChain implements SimpleGitExecutor {
          this._plugins.exec('spawn.after', undefined, {
             ...pluginContext(task, args),
             spawned,
-            kill (reason: Error) {
+            kill(reason: Error) {
                if (spawned.killed) {
                   return;
                }
diff --git a/src/lib/tasks/branch.ts b/src/lib/tasks/branch.ts
index ddc6529..cfe7a80 100644
--- a/src/lib/tasks/branch.ts
+++ b/src/lib/tasks/branch.ts
@@ -3,6 +3,7 @@ import { StringTask } from '../types';
 import { GitResponseError } from '../errors/git-response-error';
 import { hasBranchDeletionError, parseBranchDeletions } from '../parsers/parse-branch-delete';
 import { parseBranchSummary } from '../parsers/parse-branch';
+import { bufferToString } from '../utils';
 
 export function containsDeleteBranchCommand(commands: string[]) {
    const deleteCommands = ['-d', '-D', '--delete'];
@@ -51,14 +52,13 @@ export function deleteBranchesTask(branches: string[], forceDelete = false): Str
       parser(stdOut, stdErr) {
          return parseBranchDeletions(stdOut, stdErr);
       },
-      onError(exitCode, error, done, fail) {
-         if (!hasBranchDeletionError(error, exitCode)) {
+      onError({exitCode, stdOut}, error, done, fail) {
+         if (!hasBranchDeletionError(String(error), exitCode)) {
             return fail(error);
          }
 
-         done(error);
+         done(stdOut);
       },
-      concatStdErr: true,
    }
 }
 
@@ -69,14 +69,16 @@ export function deleteBranchTask(branch: string, forceDelete = false): StringTas
       parser(stdOut, stdErr) {
          return parseBranchDeletions(stdOut, stdErr).branches[branch]!;
       },
-      onError(exitCode, error, _, fail) {
-         if (!hasBranchDeletionError(error, exitCode)) {
+      onError({exitCode, stdErr, stdOut}, error, _, fail) {
+         if (!hasBranchDeletionError(String(error), exitCode)) {
             return fail(error);
          }
 
-         throw new GitResponseError(task.parser(error, ''), error);
+         throw new GitResponseError(
+            task.parser(bufferToString(stdOut), bufferToString(stdErr)),
+            String(error)
+         );
       },
-      concatStdErr: true,
    };
 
    return task;
diff --git a/src/lib/tasks/check-is-repo.ts b/src/lib/tasks/check-is-repo.ts
index cb774e5..b9fda5b 100644
--- a/src/lib/tasks/check-is-repo.ts
+++ b/src/lib/tasks/check-is-repo.ts
@@ -7,12 +7,12 @@ export enum CheckRepoActions {
    IS_REPO_ROOT = 'root',
 }
 
-const onError: StringTask<boolean>['onError'] = (exitCode, stdErr, done, fail) => {
-   if (exitCode === ExitCodes.UNCLEAN && isNotRepoMessage(stdErr)) {
-      return done('false');
+const onError: StringTask<boolean>['onError'] = ({exitCode}, error, done, fail) => {
+   if (exitCode === ExitCodes.UNCLEAN && isNotRepoMessage(error)) {
+      return done(Buffer.from('false'));
    }
 
-   fail(stdErr);
+   fail(error);
 }
 
 const parser: StringTask<boolean>['parser'] = (text) => {
@@ -64,6 +64,6 @@ export function checkIsBareRepoTask(): StringTask<boolean> {
 }
 
 
-function isNotRepoMessage(message: string): boolean {
-   return /(Not a git repository|Kein Git-Repository)/i.test(message);
+function isNotRepoMessage(error: Error): boolean {
+   return /(Not a git repository|Kein Git-Repository)/i.test(String(error));
 }
diff --git a/src/lib/tasks/init.ts b/src/lib/tasks/init.ts
index c9a353b..bfe2bc5 100644
--- a/src/lib/tasks/init.ts
+++ b/src/lib/tasks/init.ts
@@ -16,7 +16,6 @@ export function initTask(bare = false, path: string, customArgs: string[]): Stri
 
    return {
       commands,
-      concatStdErr: false,
       format: 'utf-8',
       parser(text: string): InitResult {
          return parseInit(commands.includes('--bare'), path, text);
diff --git a/src/lib/types/index.ts b/src/lib/types/index.ts
index a2d5cc3..1aa3c77 100644
--- a/src/lib/types/index.ts
+++ b/src/lib/types/index.ts
@@ -62,22 +62,43 @@ export interface GitExecutorResult {
    rejection: Maybe<Error>;
 }
 
+export interface SimpleGitPluginConfig {
+
+   /**
+    * Configures the content of errors thrown by the `simple-git` instance for each task
+    */
+   errors(error: Buffer | Error | undefined, result: Omit<GitExecutorResult, 'rejection'>): Buffer | Error | undefined;
+
+   /**
+    * Handler to be called with progress events emitted through the progress plugin
+    */
+   progress(data: SimpleGitProgressEvent): void;
+
+   /**
+    * Configuration for the `timeoutPlugin`
+    */
+   timeout: {
+
+      /**
+       * The number of milliseconds to wait after spawning the process / receiving
+       * content on the stdOut/stdErr streams before forcibly closing the git process.
+       */
+      block: number;
+   };
+}
+
 /**
  * Optional configuration settings to be passed to the `simpleGit`
  * builder.
  */
-export interface SimpleGitOptions {
+export interface SimpleGitOptions extends Partial<SimpleGitPluginConfig> {
    baseDir: string;
    binary: string;
    maxConcurrentProcesses: number;
    config: string[];
-   timeout?: {
-      block: number;
-   };
-
-   progress?(data: SimpleGitProgressEvent): void;
 }
 
 export type Maybe<T> = T | undefined;
+export type MaybeArray<T> = T | T[];
 
 export type Primitives = string | number | boolean;
diff --git a/src/lib/types/tasks.ts b/src/lib/types/tasks.ts
index 711a17b..5a19a38 100644
--- a/src/lib/types/tasks.ts
+++ b/src/lib/types/tasks.ts
@@ -1,3 +1,4 @@
+import { GitExecutorResult } from './index';
 import { EmptyTask } from '../tasks/task';
 
 export type TaskResponseFormat = Buffer | string;
@@ -17,18 +18,11 @@ export interface SimpleGitTaskConfiguration<RESPONSE, FORMAT, INPUT extends Task
    parser: TaskParser<INPUT, RESPONSE>;
 
    onError?: (
-      exitCode: number,
-      error: string,
-      done: (result: INPUT) => void,
-      fail: (error: string) => void,
+      result: GitExecutorResult,
+      error: Error,
+      done: (result: Buffer | Buffer[]) => void,
+      fail: (error: string | Error) => void,
    ) => void;
-
-   /**
-    * @deprecated
-    * Use of `concatStdErr` is now deprecated, to be removed by v2.50.0 (or on upgrading to v3).
-    * Instead, use the `stdErr` argument supplied to the `parser`
-    */
-   concatStdErr?: boolean;
 }
 
 export type StringTask<R> = SimpleGitTaskConfiguration<R, 'utf-8', string>;
diff --git a/src/lib/utils/util.ts b/src/lib/utils/util.ts
index 14bf470..2684f30 100644
--- a/src/lib/utils/util.ts
+++ b/src/lib/utils/util.ts
@@ -137,3 +137,7 @@ export function prefixedArray<T>(input: T[], prefix: T): T[] {
    }
    return output;
 }
+
+export function bufferToString (input: Buffer | Buffer[]): string {
+   return (Array.isArray(input) ? Buffer.concat(input) : input).toString('utf-8');
+}
diff --git a/typings/types.d.ts b/typings/types.d.ts
index 3ac7e1a..ac90481 100644
--- a/typings/types.d.ts
+++ b/typings/types.d.ts
@@ -1,9 +1,7 @@
 export { RemoteWithoutRefs, RemoteWithRefs } from '../src/lib/responses/GetRemoteSummary';
 export { LogOptions, DefaultLogFields } from '../src/lib/tasks/log';
 
-export {
-   outputHandler, Options, TaskOptions, SimpleGitOptions, SimpleGitProgressEvent, SimpleGitTaskCallback
-} from '../src/lib/types';
+export { outputHandler, Options, SimpleGitOptions, TaskOptions, SimpleGitTaskCallback } from '../src/lib/types';
 
 export { CheckRepoActions } from '../src/lib/tasks/check-is-repo';
 export { CleanOptions, CleanMode } from '../src/lib/tasks/clean';


Test patch:
diff --git a/test/unit/plugin.error.spec.ts b/test/unit/plugin.error.spec.ts
new file mode 100644
index 0000000..67da2ad
--- /dev/null
+++ b/test/unit/plugin.error.spec.ts
@@ -0,0 +1,42 @@
+import { promiseError } from '@kwsites/promise-result';
+import { assertGitError, closeWithError, closeWithSuccess, newSimpleGit } from './__fixtures__';
+import { GitError } from '../../src/lib/errors/git-error';
+
+describe('errorDetectionPlugin', () => {
+
+   it('can throw with custom content', async () => {
+      const errors = jest.fn().mockReturnValue(Buffer.from('foo'));
+      const git = newSimpleGit({errors}).init();
+      await closeWithError('err');
+
+      assertGitError(await promiseError(git), 'foo');
+   });
+
+   it('can throw error when otherwise deemed ok', async () => {
+      const errors = jest.fn().mockReturnValue(new Error('FAIL'));
+      const git = newSimpleGit({errors}).init();
+      await closeWithSuccess('OK');
+
+      expect(errors).toHaveBeenCalledWith(undefined, {
+         exitCode: 0,
+         stdErr: [],
+         stdOut: [expect.any(Buffer)],
+      });
+      assertGitError(await promiseError(git), 'FAIL');
+   });
+
+   it('can ignore errors that would otherwise throw', async () => {
+      const errors = jest.fn();
+
+      const git = newSimpleGit({errors}).raw('foo');
+      await closeWithError('OUT', 100);
+
+      expect(errors).toHaveBeenCalledWith(expect.any(GitError), {
+         exitCode: 100,
+         stdOut: [],
+         stdErr: [expect.any(Buffer)],
+      });
+      expect(await promiseError(git)).toBeUndefined();
+   });
+
+});