From 19029fc2fdee71766226757497a2ae8be3535ad9 Mon Sep 17 00:00:00 2001 From: Steve King Date: Sun, 28 Aug 2022 07:51:00 +0100 Subject: [PATCH] Abort plugin (#848) * Create the `abort` plugin to support killing any active / future tasks for a `simple-git` instance --- .changeset/gold-mugs-explain.md | 5 ++ docs/PLUGIN-ABORT-CONTROLLER.md | 44 +++++++++++ packages/test-utils/index.ts | 1 + .../test-utils/src/create-abort-controller.ts | 45 +++++++++++ simple-git/package.json | 2 +- simple-git/readme.md | 13 ++-- simple-git/src/lib/git-factory.ts | 2 + simple-git/src/lib/plugins/abort-plugin.ts | 33 +++++++++ simple-git/src/lib/plugins/index.ts | 1 + .../src/lib/plugins/simple-git-plugin.ts | 6 ++ .../src/lib/runners/git-executor-chain.ts | 32 +++++++- simple-git/src/lib/types/index.ts | 2 + .../test/integration/plugin.abort.spec.ts | 53 +++++++++++++ ...ugin.spec.ts => plugin.completion.spec.ts} | 0 ...plugin.spec.ts => plugin.progress.spec.ts} | 0 ...-plugin.spec.ts => plugin.timeout.spec.ts} | 0 .../test/unit/__mocks__/mock-child-process.ts | 9 ++- simple-git/test/unit/plugin.abort.spec.ts | 74 +++++++++++++++++++ ...ts => plugin.completion-detection.spec.ts} | 0 yarn.lock | 10 +-- 20 files changed, 318 insertions(+), 14 deletions(-) create mode 100644 .changeset/gold-mugs-explain.md create mode 100644 docs/PLUGIN-ABORT-CONTROLLER.md create mode 100644 packages/test-utils/src/create-abort-controller.ts create mode 100644 simple-git/src/lib/plugins/abort-plugin.ts create mode 100644 simple-git/test/integration/plugin.abort.spec.ts rename simple-git/test/integration/{completion-plugin.spec.ts => plugin.completion.spec.ts} (100%) rename simple-git/test/integration/{progress-plugin.spec.ts => plugin.progress.spec.ts} (100%) rename simple-git/test/integration/{timeout-plugin.spec.ts => plugin.timeout.spec.ts} (100%) create mode 100644 simple-git/test/unit/plugin.abort.spec.ts rename simple-git/test/unit/{completion-detection-plugin.spec.ts => plugin.completion-detection.spec.ts} (100%) diff --git a/.changeset/gold-mugs-explain.md b/.changeset/gold-mugs-explain.md new file mode 100644 index 00000000..e4dac21f --- /dev/null +++ b/.changeset/gold-mugs-explain.md @@ -0,0 +1,5 @@ +--- +'simple-git': minor +--- + +Create the abort plugin to allow cancelling all pending and future tasks. diff --git a/docs/PLUGIN-ABORT-CONTROLLER.md b/docs/PLUGIN-ABORT-CONTROLLER.md new file mode 100644 index 00000000..ee382df7 --- /dev/null +++ b/docs/PLUGIN-ABORT-CONTROLLER.md @@ -0,0 +1,44 @@ +## Using an AbortController to terminate tasks + +The easiest way to send a `SIGKILL` to the `git` child processes created by `simple-git` is to use an `AbortController` +in the constructor options for `simpleGit`: + +```typescript +import { simpleGit, GitPluginError, SimpleGit } from 'simple-git'; + +const controller = new AbortController(); + +const git: SimpleGit = simpleGit({ + baseDir: '/some/path', + abort: controller.signal, +}); + +try { + await git.pull(); +} +catch (err) { + if (err instanceof GitPluginError && err.plugin === 'abort') { + // task failed because `controller.abort` was called while waiting for the `git.pull` + } +} +``` + +### Examples: + +#### Share AbortController across many instances + +Run the same operation against multiple repositories, cancel any pending operations when the first has been completed. + +```typescript +const repos = [ + '/path/to/repo-a', + '/path/to/repo-b', + '/path/to/repo-c', +]; + +const controller = new AbortController(); +const result = await Promise.race( + repos.map(baseDir => simpleGit({ baseDir, abort: controller.signal }).fetch()) +); +controller.abort(); +``` diff --git a/packages/test-utils/index.ts b/packages/test-utils/index.ts index 21f06c2a..76135e09 100644 --- a/packages/test-utils/index.ts +++ b/packages/test-utils/index.ts @@ -1,3 +1,4 @@ +export * from './src/create-abort-controller'; export * from './src/create-test-context'; export * from './src/expectations'; export * from './src/instance'; diff --git a/packages/test-utils/src/create-abort-controller.ts b/packages/test-utils/src/create-abort-controller.ts new file mode 100644 index 00000000..23508cf1 --- /dev/null +++ b/packages/test-utils/src/create-abort-controller.ts @@ -0,0 +1,45 @@ +import { setMaxListeners } from 'events'; + +export function createAbortController() { + if (typeof AbortController === 'undefined') { + return createMockAbortController() as { controller: AbortController; abort: AbortSignal }; + } + + const controller = new AbortController(); + setMaxListeners(1000, controller.signal); + return { + controller, + abort: controller.signal, + mocked: false, + }; +} + +function createMockAbortController(): unknown { + let aborted = false; + const handlers: Set<() => void> = new Set(); + const abort = { + addEventListener(type: 'abort', handler: () => void) { + if (type !== 'abort') throw new Error('Unsupported event name'); + handlers.add(handler); + }, + removeEventListener(type: 'abort', handler: () => void) { + if (type !== 'abort') throw new Error('Unsupported event name'); + handlers.delete(handler); + }, + get aborted() { + return aborted; + }, + }; + + return { + controller: { + abort() { + if (aborted) throw new Error('abort called when already aborted'); + aborted = true; + handlers.forEach((h) => h()); + }, + }, + abort, + mocked: true, + }; +} diff --git a/simple-git/package.json b/simple-git/package.json index 5ce66822..99849179 100644 --- a/simple-git/package.json +++ b/simple-git/package.json @@ -24,7 +24,7 @@ "@simple-git/test-utils": "^1.0.0", "@types/debug": "^4.1.5", "@types/jest": "^27.0.3", - "@types/node": "^14.14.10", + "@types/node": "^16", "esbuild": "^0.14.10", "esbuild-node-externals": "^1.4.1", "jest": "^27.4.5", diff --git a/simple-git/readme.md b/simple-git/readme.md index fcf4032e..0bd2ca81 100644 --- a/simple-git/readme.md +++ b/simple-git/readme.md @@ -93,19 +93,22 @@ await git.pull(); ## Configuring Plugins -- [Completion Detection](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-COMPLETION-DETECTION.md) +- [AbortController](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-ABORT-CONTROLLER.md) + Terminate pending and future tasks in a `simple-git` instance (requires node >= 16). + +- [Completion Detection](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-COMPLETION-DETECTION.md) Customise how `simple-git` detects the end of a `git` process. -- [Error Detection](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-ERRORS.md) +- [Error Detection](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-ERRORS.md) Customise the detection of errors from the underlying `git` process. -- [Progress Events](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-PROGRESS-EVENTS.md) +- [Progress Events](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-PROGRESS-EVENTS.md) Receive progress events as `git` works through long-running processes. -- [Spawned Process Ownership](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-SPAWN-OPTIONS.md) +- [Spawned Process Ownership](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-SPAWN-OPTIONS.md) Configure the system `uid` / `gid` to use for spawned `git` processes. -- [Timeout](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-TIMEOUT.md) +- [Timeout](https://github.com/steveukx/git-js/blob/main/docs/PLUGIN-TIMEOUT.md) Automatically kill the wrapped `git` process after a rolling timeout. ## Using Task Promises diff --git a/simple-git/src/lib/git-factory.ts b/simple-git/src/lib/git-factory.ts index f7c3041f..7034da00 100644 --- a/simple-git/src/lib/git-factory.ts +++ b/simple-git/src/lib/git-factory.ts @@ -2,6 +2,7 @@ import { SimpleGitFactory } from '../../typings'; import * as api from './api'; import { + abortPlugin, commandConfigPrefixingPlugin, completionDetectionPlugin, errorDetectionHandler, @@ -55,6 +56,7 @@ export function gitInstanceFactory( } plugins.add(completionDetectionPlugin(config.completion)); + config.abort && plugins.add(abortPlugin(config.abort)); config.progress && plugins.add(progressMonitorPlugin(config.progress)); config.timeout && plugins.add(timeoutPlugin(config.timeout)); config.spawnOptions && plugins.add(spawnOptionsPlugin(config.spawnOptions)); diff --git a/simple-git/src/lib/plugins/abort-plugin.ts b/simple-git/src/lib/plugins/abort-plugin.ts new file mode 100644 index 00000000..4771a3fb --- /dev/null +++ b/simple-git/src/lib/plugins/abort-plugin.ts @@ -0,0 +1,33 @@ +import { SimpleGitOptions } from '../types'; +import { SimpleGitPlugin } from './simple-git-plugin'; +import { GitPluginError } from '../errors/git-plugin-error'; + +export function abortPlugin(signal: SimpleGitOptions['abort']) { + if (!signal) { + return; + } + + const onSpawnAfter: SimpleGitPlugin<'spawn.after'> = { + type: 'spawn.after', + action(_data, context) { + function kill() { + context.kill(new GitPluginError(undefined, 'abort', 'Abort signal received')); + } + + signal.addEventListener('abort', kill); + + context.spawned.on('close', () => signal.removeEventListener('abort', kill)); + }, + }; + + const onSpawnBefore: SimpleGitPlugin<'spawn.before'> = { + type: 'spawn.before', + action(_data, context) { + if (signal.aborted) { + context.kill(new GitPluginError(undefined, 'abort', 'Abort already signaled')); + } + }, + }; + + return [onSpawnBefore, onSpawnAfter]; +} diff --git a/simple-git/src/lib/plugins/index.ts b/simple-git/src/lib/plugins/index.ts index 0cfb54eb..bf89bdc9 100644 --- a/simple-git/src/lib/plugins/index.ts +++ b/simple-git/src/lib/plugins/index.ts @@ -1,3 +1,4 @@ +export * from './abort-plugin'; export * from './command-config-prefixing-plugin'; export * from './completion-detection.plugin'; export * from './error-detection.plugin'; diff --git a/simple-git/src/lib/plugins/simple-git-plugin.ts b/simple-git/src/lib/plugins/simple-git-plugin.ts index 7e40d6ba..d49f83e4 100644 --- a/simple-git/src/lib/plugins/simple-git-plugin.ts +++ b/simple-git/src/lib/plugins/simple-git-plugin.ts @@ -15,6 +15,12 @@ export interface SimpleGitPluginTypes { data: Partial; context: SimpleGitTaskPluginContext & {}; }; + 'spawn.before': { + data: void; + context: SimpleGitTaskPluginContext & { + kill(reason: Error): void; + }; + }; 'spawn.after': { data: void; context: SimpleGitTaskPluginContext & { diff --git a/simple-git/src/lib/runners/git-executor-chain.ts b/simple-git/src/lib/runners/git-executor-chain.ts index 80c42dee..73fb4434 100644 --- a/simple-git/src/lib/runners/git-executor-chain.ts +++ b/simple-git/src/lib/runners/git-executor-chain.ts @@ -191,10 +191,26 @@ export class GitExecutorChain implements SimpleGitExecutor { const stdOut: Buffer[] = []; const stdErr: Buffer[] = []; - let rejection: Maybe; - logger.info(`%s %o`, command, args); logger('%O', spawnOptions); + + let rejection = this._beforeSpawn(task, args); + if (rejection) { + return done({ + stdOut, + stdErr, + exitCode: 9901, + rejection, + }); + } + + this._plugins.exec('spawn.before', undefined, { + ...pluginContext(task, args), + kill(reason) { + rejection = reason || rejection; + }, + }); + const spawned = spawn(command, args, spawnOptions); spawned.stdout!.on( @@ -235,6 +251,18 @@ export class GitExecutorChain implements SimpleGitExecutor { }); }); } + + private _beforeSpawn(task: SimpleGitTask, args: string[]) { + let rejection: Maybe; + this._plugins.exec('spawn.before', undefined, { + ...pluginContext(task, args), + kill(reason) { + rejection = reason || rejection; + }, + }); + + return rejection; + } } function pluginContext(task: SimpleGitTask, commands: string[]) { diff --git a/simple-git/src/lib/types/index.ts b/simple-git/src/lib/types/index.ts index aeb2e672..0368b01d 100644 --- a/simple-git/src/lib/types/index.ts +++ b/simple-git/src/lib/types/index.ts @@ -64,6 +64,8 @@ export interface GitExecutorResult { } export interface SimpleGitPluginConfig { + abort: AbortSignal; + /** * Configures the events that should be used to determine when the unederlying child process has * been terminated. diff --git a/simple-git/test/integration/plugin.abort.spec.ts b/simple-git/test/integration/plugin.abort.spec.ts new file mode 100644 index 00000000..de598032 --- /dev/null +++ b/simple-git/test/integration/plugin.abort.spec.ts @@ -0,0 +1,53 @@ +import { promiseError } from '@kwsites/promise-result'; +import { + assertGitError, + createAbortController, + createTestContext, + newSimpleGit, + SimpleGitTestContext, + wait, +} from '@simple-git/test-utils'; + +import { GitPluginError } from '../..'; + +describe('timeout', () => { + let context: SimpleGitTestContext; + + beforeEach(async () => (context = await createTestContext())); + + it('kills processes on abort signal', async () => { + const { controller, abort } = createAbortController(); + + const threw = promiseError(newSimpleGit(context.root, { abort }).init()); + + await wait(0); + controller.abort(); + + assertGitError(await threw, 'Abort signal received', GitPluginError); + }); + + it('share AbortController across many instances', async () => { + const { controller, abort } = createAbortController(); + const upstream = await newSimpleGit(__dirname).revparse('--git-dir'); + + const repos = await Promise.all('abcdef'.split('').map((p) => context.dir(p))); + + await Promise.all( + repos.map((baseDir) => { + const git = newSimpleGit({ baseDir, abort }); + if (baseDir.endsWith('a')) { + return promiseError(git.init().then(() => controller.abort())); + } + + return promiseError(git.clone(upstream, baseDir)); + }) + ); + + const results = await Promise.all( + repos.map((baseDir) => newSimpleGit(baseDir).checkIsRepo()) + ); + + expect(results).toContain(false); + expect(results).toContain(true); + }); +}); diff --git a/simple-git/test/integration/completion-plugin.spec.ts b/simple-git/test/integration/plugin.completion.spec.ts similarity index 100% rename from simple-git/test/integration/completion-plugin.spec.ts rename to simple-git/test/integration/plugin.completion.spec.ts diff --git a/simple-git/test/integration/progress-plugin.spec.ts b/simple-git/test/integration/plugin.progress.spec.ts similarity index 100% rename from simple-git/test/integration/progress-plugin.spec.ts rename to simple-git/test/integration/plugin.progress.spec.ts diff --git a/simple-git/test/integration/timeout-plugin.spec.ts b/simple-git/test/integration/plugin.timeout.spec.ts similarity index 100% rename from simple-git/test/integration/timeout-plugin.spec.ts rename to simple-git/test/integration/plugin.timeout.spec.ts diff --git a/simple-git/test/unit/__mocks__/mock-child-process.ts b/simple-git/test/unit/__mocks__/mock-child-process.ts index 8717c8e7..26459ab9 100644 --- a/simple-git/test/unit/__mocks__/mock-child-process.ts +++ b/simple-git/test/unit/__mocks__/mock-child-process.ts @@ -32,7 +32,14 @@ class MockEventTargetImpl implements MockEventTarget { this.getHandlers(event).forEach((handler) => handler(data)); }; - public kill = jest.fn(); + public kill = jest.fn((_signal = 'SIGINT') => { + if (this.$emitted('exit')) { + throw new Error('MockEventTarget:kill called on process after exit'); + } + + this.$emit('exit', 1); + this.$emit('close', 1); + }); public off = jest.fn((event: string, handler: Function) => { this.delHandler(event, handler); diff --git a/simple-git/test/unit/plugin.abort.spec.ts b/simple-git/test/unit/plugin.abort.spec.ts new file mode 100644 index 00000000..08733b74 --- /dev/null +++ b/simple-git/test/unit/plugin.abort.spec.ts @@ -0,0 +1,74 @@ +import { promiseError } from '@kwsites/promise-result'; +import { + assertExecutedTasksCount, + assertGitError, + createAbortController, + newSimpleGit, + wait, +} from './__fixtures__'; +import { GitPluginError } from '../..'; + +describe('plugin.abort', function () { + it('aborts an active child process', async () => { + const { controller, abort } = createAbortController(); + const git = newSimpleGit({ abort }); + + const queue = promiseError(git.raw('foo')); + await wait(); + + assertExecutedTasksCount(1); + controller.abort(); + + assertGitError(await queue, 'Abort signal received', GitPluginError); + }); + + it('aborts all active promises', async () => { + const { controller, abort } = createAbortController(); + const git = newSimpleGit({ abort }); + const all = Promise.all([ + git.raw('a').catch((e) => e), + git.raw('b').catch((e) => e), + git.raw('c').catch((e) => e), + ]); + + await wait(); + assertExecutedTasksCount(3); + controller.abort(); + + expect(await all).toEqual([ + expect.any(GitPluginError), + expect.any(GitPluginError), + expect.any(GitPluginError), + ]); + }); + + it('aborts all steps in chained promises', async () => { + const { controller, abort } = createAbortController(); + const git = newSimpleGit({ abort }); + const a = git.raw('a'); + const b = a.raw('b'); + const c = b.raw('c'); + + const all = Promise.all([a.catch((e) => e), b.catch((e) => e), c.catch((e) => e)]); + + await wait(); + assertExecutedTasksCount(1); + controller.abort(); + + expect(await all).toEqual([ + expect.any(GitPluginError), + expect.any(GitPluginError), + expect.any(GitPluginError), + ]); + assertExecutedTasksCount(1); + }); + + it('aborts before attempting to spawn', async () => { + const { controller, abort } = createAbortController(); + controller.abort(); + + const git = newSimpleGit({ abort }); + assertGitError(await promiseError(git.raw('a')), 'Abort already signaled', GitPluginError); + assertExecutedTasksCount(0); + }); +}); diff --git a/simple-git/test/unit/completion-detection-plugin.spec.ts b/simple-git/test/unit/plugin.completion-detection.spec.ts similarity index 100% rename from simple-git/test/unit/completion-detection-plugin.spec.ts rename to simple-git/test/unit/plugin.completion-detection.spec.ts diff --git a/yarn.lock b/yarn.lock index 78523c42..8c17adfa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2970,10 +2970,10 @@ resolved "https://registry.yarnpkg.com/@types/node/-/node-12.20.45.tgz#f4980d177999299d99cd4b290f7f39366509a44f" integrity sha512-1Jg2Qv5tuxBqgQV04+wO5u+wmSHbHgpORCJdeCLM+E+YdPElpdHhgywU+M1V1InL8rfOtpqtOjswk+uXTKwx7w== -"@types/node@^14.14.10": - version "14.14.10" - resolved "https://registry.yarnpkg.com/@types/node/-/node-14.14.10.tgz#5958a82e41863cfc71f2307b3748e3491ba03785" - integrity sha512-J32dgx2hw8vXrSbu4ZlVhn1Nm3GbeCFNw2FWL8S5QKucHGY0cyNwjdQdO+KMBZ4wpmC7KhLCiNsdk1RFRIYUQQ== +"@types/node@^16": + version "16.11.56" + resolved "https://registry.yarnpkg.com/@types/node/-/node-16.11.56.tgz#dcbb617669481e158e0f1c6204d1c768cd675901" + integrity sha512-aFcUkv7EddxxOa/9f74DINReQ/celqH8DiB3fRYgVDM2Xm5QJL8sl80QKuAnGvwAsMn+H3IFA6WCrQh1CY7m1A== "@types/normalize-package-data@^2.4.0": version "2.4.1" @@ -8113,7 +8113,7 @@ typedarray@^0.0.6: resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777" integrity sha1-hnrHTjhkGHsdPUfZlqeOxciDB3c= -typescript@4.7.4, typescript@^4.1.2: +typescript@^4.1.2: version "4.7.4" resolved "https://registry.yarnpkg.com/typescript/-/typescript-4.7.4.tgz#1a88596d1cf47d59507a1bcdfb5b9dfe4d488235" integrity sha512-C0WQT0gezHuw6AdY1M2jxUO83Rjf0HP7Sk1DtXj6j1EwkQNZrHAg2XPWlq62oqEhYvONq5pkC2Y9oPljWToLmQ==