From 96973ad617cacfe5c014fdcd4a54c95d941ee240 Mon Sep 17 00:00:00 2001 From: Dan Allen Date: Wed, 22 May 2024 01:42:15 -0600 Subject: [PATCH] resolves #5 add on_failure key to run entry to control behavior when command fails --- CHANGELOG.adoc | 1 + .../ROOT/pages/configuration-keys.adoc | 12 ++++ packages/collector-extension/lib/index.js | 14 ++-- .../lib/util/run-command.js | 2 +- .../test/collector-extension-test.js | 72 ++++++++++++++++++- .../fixtures/test-at-root/.gen-failure.js | 7 ++ 6 files changed, 100 insertions(+), 8 deletions(-) create mode 100644 packages/collector-extension/test/fixtures/test-at-root/.gen-failure.js diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 56f677d..c8db70f 100644 --- a/CHANGELOG.adoc +++ b/CHANGELOG.adoc @@ -9,6 +9,7 @@ For a detailed view of what's changed, refer to the {url-repo}/commits[commit hi === Added * add an optional `clean` configuration key on each collector entry to specify a directory to clean (i.e., remove) (#10) +* add `on_failure` key to run entry to allow failed command to be logged or ignored instead of thrown (#5) === Changed diff --git a/docs/modules/ROOT/pages/configuration-keys.adoc b/docs/modules/ROOT/pages/configuration-keys.adoc index a075799..4a180f1 100644 --- a/docs/modules/ROOT/pages/configuration-keys.adoc +++ b/docs/modules/ROOT/pages/configuration-keys.adoc @@ -124,6 +124,13 @@ This setting effectively disables looking for a global command on the current us If the command ends with `.js`, the Node.js binary used to run Antora is also used to run the command. +The `on_failure` key can be used to control what happens when the command fails. +A failed command is one that exits with a non-zero exit value. +If the value is `ignore`, the failure is silently ignored. +If the value is `log`, the failure is logged at the error level. +The level can be tuned by appending it as a property on the log keyword (e.g., `log.warn`). +If the value is `throw` (the default), the error bubbles, which causes the executio of Antora to immediately stop, just like any other fatal error. + [#scan-key] == scan key @@ -218,6 +225,11 @@ This key can be set implicitly by prepending the command with `./`. |Not set (false) |Boolean +|`run.on_failure` +|Controls the behavior when the command fails (exits with a non-zero exit value). +|throw +|throw, ignore, log (same as log.error), log. + |`scan.dir` |Defines the location from where the extension collects the generated files after the previous command is complete. The extension then imports the collected files into the bucket in the content aggregate. diff --git a/packages/collector-extension/lib/index.js b/packages/collector-extension/lib/index.js index 1ad3acb..3751847 100644 --- a/packages/collector-extension/lib/index.js +++ b/packages/collector-extension/lib/index.js @@ -21,6 +21,7 @@ const PACKAGE_NAME = require('../package.json').name module.exports.register = function () { this.once('contentAggregated', async ({ playbook, contentAggregate }) => { + let logger const quiet = playbook.runtime?.quiet const cacheDir = ospath.join(getBaseCacheDir(playbook), 'collector') //await fsp.rm(cacheDir, { force: true, recursive: true, force: true }) // Q: should we try to reuse existing cache? @@ -68,7 +69,7 @@ module.exports.register = function () { for (const clean of cleans) { await fsp.rm(clean.dir, { recursive: true, force: true }) } - for (const { dir: cwd, command, local } of runs) { + for (const { dir: cwd, command, local, onFailure = 'throw' } of runs) { if (command) { let cmd = command const opts = { cwd, output: true, quiet } @@ -84,10 +85,15 @@ module.exports.register = function () { await runCommand(cmd, [], opts) } catch (err) { const loc = worktree || url - const flag = worktree ? ' ' : remote && worktree === false ? ` ` : '' + const qualifier = worktree ? ' ' : remote && worktree === false ? ` ` : '' const pathInfo = startPath ? ` | start path: ${startPath}` : '' - const ctx = ` in ${loc} (${reftype}: ${refname}${flag}${pathInfo})` - throw Object.assign(err, { message: `(${PACKAGE_NAME}): ${err.message.replace(/$/m, ctx)}` }) + err.message = err.message.replace(/$/m, ` in ${loc} (${reftype}: ${refname}${qualifier}${pathInfo})`) + if (onFailure === 'throw') { + throw Object.assign(err, { message: `(${PACKAGE_NAME}): ${err.message}` }) + } else if (onFailure === 'log' || onFailure?.startsWith('log.')) { + const logLevel = onFailure.split('log.')[1] || 'error' + if ((logger ??= this.getLogger(PACKAGE_NAME)).isLevelEnabled(logLevel)) logger[logLevel](err) + } } } } diff --git a/packages/collector-extension/lib/util/run-command.js b/packages/collector-extension/lib/util/run-command.js index 7c5717a..7db1aac 100644 --- a/packages/collector-extension/lib/util/run-command.js +++ b/packages/collector-extension/lib/util/run-command.js @@ -107,7 +107,7 @@ async function runCommand (cmd, argv = [], opts = {}) { resolve(Buffer.from(stdout.join(''))) } } else { - let msg = `Command failed: ${ps.spawnargs.join(' ')}` + let msg = `Command failed with exit code ${code}: ${ps.spawnargs.join(' ')}` if (stderr.length) msg += '\n' + stderr.join('') reject(new Error(msg)) } diff --git a/packages/collector-extension/test/collector-extension-test.js b/packages/collector-extension/test/collector-extension-test.js index 6facd40..0f1ccca 100644 --- a/packages/collector-extension/test/collector-extension-test.js +++ b/packages/collector-extension/test/collector-extension-test.js @@ -29,6 +29,7 @@ const REPOS_DIR = ospath.join(WORK_DIR, 'repos') describe('collector extension', () => { let gitServer let gitServerPort + let messages const cleanWorkDir = async (opts = {}) => { await fsp.rm(WORK_DIR, { recursive: true, force: true }) @@ -60,6 +61,10 @@ describe('collector extension', () => { await fsp.rm(WORK_DIR, { recursive: true, force: true }) }) + beforeEach(async () => { + messages = [] + }) + afterEach(async () => { await cleanWorkDir() }) @@ -80,6 +85,22 @@ describe('collector extension', () => { once (eventName, fn) { this[eventName] = fn }, + getLogger: (name) => + new Proxy( + {}, + { + get (target, property) { + if (property === 'isLevelEnabled') return () => true + return (msg, err) => { + if (msg instanceof Error) { + err = msg + msg = undefined + } + messages.push({ name, level: property, msg, err }) + } + }, + } + ), }) const createRepository = async ({ repoName, fixture = repoName, branches, tags, startPath, collectorConfig }) => { @@ -1139,7 +1160,7 @@ describe('collector extension', () => { it('should throw error if command is not found', async () => { const collectorConfig = { run: { command: 'no-such-command' } } const expectedMessage = windows - ? 'Command failed: ' + ? 'Command failed' : '(@antora/collector-extension): Command not found: no-such-command in ' + `http://localhost:${gitServerPort}/test-at-root/.git (branch: main)` expect(await trapAsyncError(() => runScenario({ repoName: 'test-at-root', collectorConfig }))).to.throw( @@ -1151,7 +1172,7 @@ describe('collector extension', () => { it('should throw error if command is not found for origin with start path', async () => { const collectorConfig = { run: { command: 'no-such-command' } } const expectedMessage = windows - ? 'Command failed: ' + ? 'Command failed' : '(@antora/collector-extension): Command not found: no-such-command in ' + `http://localhost:${gitServerPort}/test-at-start-path/.git (branch: main | start path: docs)` expect( @@ -1162,7 +1183,7 @@ describe('collector extension', () => { it('should throw error if command is not found when using local worktree', async () => { const collectorConfig = { run: { command: 'no-such-command' } } const expectedMessage = windows - ? 'Command failed: ' + ? 'Command failed' : '(@antora/collector-extension): Command not found: no-such-command in ' + `${ospath.join(REPOS_DIR, 'test-at-root')} (branch: main )` expect( @@ -1170,6 +1191,51 @@ describe('collector extension', () => { ).to.throw(Error, expectedMessage) }) + // also verifies that node command is replaced with process.execPath + it('should throw error if command fails to complete successfully', async () => { + const collectorConfig = { run: { command: 'node .gen-failure.js' } } + const expectedMessage = + '(@antora/collector-extension): Command failed with exit code 1: ' + + (windows ? '' : `${process.execPath} .gen-failure.js in `) + expect(await trapAsyncError(() => runScenario({ repoName: 'test-at-root', collectorConfig }))).to.throw( + Error, + expectedMessage + ) + }) + + it('should ignore error if command fails when on_failure is ignore', async () => { + const collectorConfig = { run: { command: 'no-such-command', on_failure: 'ignore' } } + expect(await trapAsyncError(() => runScenario({ repoName: 'test-at-root', collectorConfig }))).to.not.throw() + }) + + it('should log error if command fails when on_failure is log', async () => { + const collectorConfig = { run: { command: 'no-such-command', on_failure: 'log' } } + const expectedMessage = windows + ? 'Command failed' + : `Command not found: no-such-command in http://localhost:${gitServerPort}/test-at-root/.git (branch: main)` + expect(await trapAsyncError(() => runScenario({ repoName: 'test-at-root', collectorConfig }))).to.not.throw() + expect(messages).to.have.lengthOf(1) + const message = messages[0] + expect(message).to.contain({ name: '@antora/collector-extension', level: 'error', msg: undefined }) + const err = message.err + expect(err).to.be.instanceOf(Error) + expect(err.message).to.include(expectedMessage) + }) + + it('should log at specified level if command fails when on_failure is log.warn', async () => { + const collectorConfig = { run: { command: 'no-such-command', on_failure: 'log.warn' } } + const expectedMessage = windows + ? 'Command failed' + : `Command not found: no-such-command in http://localhost:${gitServerPort}/test-at-root/.git (branch: main)` + expect(await trapAsyncError(() => runScenario({ repoName: 'test-at-root', collectorConfig }))).to.not.throw() + expect(messages).to.have.lengthOf(1) + const message = messages[0] + expect(message).to.contain({ name: '@antora/collector-extension', level: 'warn', msg: undefined }) + const err = message.err + expect(err).to.be.instanceOf(Error) + expect(err.message).to.include(expectedMessage) + }) + it('should not run collector if no origins are found on content aggregate', async () => { await runScenario({ repoName: 'test-at-root', diff --git a/packages/collector-extension/test/fixtures/test-at-root/.gen-failure.js b/packages/collector-extension/test/fixtures/test-at-root/.gen-failure.js new file mode 100644 index 0000000..797ce86 --- /dev/null +++ b/packages/collector-extension/test/fixtures/test-at-root/.gen-failure.js @@ -0,0 +1,7 @@ +'use strict' + +const fsp = require('node:fs/promises') + +;(async () => { + await fsp.writeFile('path/does/not/exist/file.adoc', 'content', 'utf8') +})() -- GitLab