Merge pull request #604 from steveukx/feature/error-detection-plugin

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]);
+   }
+})
+```

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.
 

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);
 }

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 {

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
+         };
+      },
+   };
+
+}

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';

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;

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,43 +97,42 @@ 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),
                   ));
                },
                fail
             );
          }
 
-         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;
                }

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;

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));
 }

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);

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;