Skip to content

Commit

Permalink
Escalate various log messages (#1138)
Browse files Browse the repository at this point in the history
If result validation was explicitly requested but skipped, then show a warning
fixes #1137

some log messages were raised to be warnings, in case they lead to an error.

some wordings were changed

---------

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
  • Loading branch information
jkowalleck committed Dec 11, 2023
1 parent d629598 commit 1256457
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 19 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/nodejs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,8 @@ jobs:
path: ${{ env.DIST_DIR }}
- name: dogfooding
run: >
node -- bin/cyclonedx-npm-cli.js
node -- bin/cyclonedx-npm-cli.js
-vvv
--ignore-npm-errors
--omit=dev
--validate
Expand Down
7 changes: 7 additions & 0 deletions HISTORY.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.

## unreleased

* Change
* If BOM result validation was explicitly requested but skipped, then a warning is shown ([#1137] via [#1138])
* Log messages that explain fatal errors were are (via [#1138])

[#1137]: https://github.com/CycloneDX/cyclonedx-node-npm/issues/1137
[#1138]: https://github.com/CycloneDX/cyclonedx-node-npm/pull/1138

## 1.15.0 - 2023-12-10

* Changed
Expand Down
6 changes: 3 additions & 3 deletions src/builders.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ export class BomBuilder {

private getNpmVersion (npmRunner: runFunc, process_: NodeJS.Process): string {
let version: string
this.console.info('INFO | detect NPM version ...')
this.console.info('INFO | detecting NPM version ...')
try {
version = npmRunner(['--version'], {
env: process_.env,
Expand Down Expand Up @@ -166,7 +166,7 @@ export class BomBuilder {
}
}

this.console.info('INFO | gather dependency tree ...')
this.console.info('INFO | gathering dependency tree ...')
this.console.debug('DEBUG | npm-ls: run npm with %j in %j', args, projectDir)
let npmLsReturns: Buffer
try {
Expand Down Expand Up @@ -207,7 +207,7 @@ export class BomBuilder {
}

buildFromNpmLs (data: any, npmVersion?: string): Models.Bom {
this.console.info('INFO | build BOM ...')
this.console.info('INFO | building BOM ...')

// region all components & dependencies

Expand Down
35 changes: 21 additions & 14 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ interface CommandOptions {
outputReproducible: boolean
outputFormat: OutputFormat
outputFile: string
validate: boolean
validate: boolean | undefined
mcType: Enums.ComponentType
verbose: number
}
Expand Down Expand Up @@ -146,7 +146,7 @@ function makeCommand (process: NodeJS.Process): Command {
'--validate',
'Validate resulting BOM before outputting. ' +
'Validation is skipped, if requirements not met. See the README.'
).default(true)
).default(undefined)
).addOption(
new Option(
'--no-validate',
Expand Down Expand Up @@ -211,28 +211,29 @@ export async function run (process: NodeJS.Process): Promise<number> {
if (!existsSync(packageFile)) {
throw new Error(`missing project's manifest file: ${packageFile}`)
}
myConsole.debug('DEBUG | packageFile: %s', packageFile)
myConsole.debug('DEBUG | packageFile:', packageFile)
const projectDir = dirname(packageFile)
myConsole.info('INFO | projectDir: %s', projectDir)
myConsole.info('INFO | projectDir:', projectDir)

if (existsSync(resolve(projectDir, 'npm-shrinkwrap.json'))) {
myConsole.debug('DEBUG | detected a npm shrinkwrap file')
myConsole.info('INFO | detected a npm shrinkwrap file')
} else if (existsSync(resolve(projectDir, 'package-lock.json'))) {
myConsole.debug('DEBUG | detected a package lock file')
myConsole.info('INFO | detected a package lock file')
} else if (!options.packageLockOnly && existsSync(resolve(projectDir, 'node_modules'))) {
myConsole.debug('DEBUG | detected a node_modules dir')
myConsole.info('INFO | detected a `node_modules` dir')
// npm7 and later also might put a `node_modules/.package-lock.json` file
} else {
myConsole.log('LOG | No evidence: no package lock file nor npm shrinkwrap file')
myConsole.warn('WARN | ? Did you forget to run `npm install` on your project accordingly ?')
myConsole.error('ERROR | No evidence: no package lock file nor npm shrinkwrap file')
if (!options.packageLockOnly) {
myConsole.log('LOG | No evidence: no node_modules dir')
myConsole.error('ERROR | No evidence: no `node_modules` dir')
}
myConsole.info('INFO | ? Did you forget to run `npm install` on your project accordingly ?')
throw new Error('missing evidence')
}

const extRefFactory = new Factories.FromNodePackageJson.ExternalReferenceFactory()

myConsole.log('LOG | gathering BOM data ...')
const bom = new BomBuilder(
new Builders.FromNodePackageJson.ToolBuilder(extRefFactory),
new Builders.FromNodePackageJson.ComponentBuilder(
Expand Down Expand Up @@ -271,14 +272,14 @@ export async function run (process: NodeJS.Process): Promise<number> {
break
}

myConsole.log('LOG | serialize BOM')
myConsole.log('LOG | serializing BOM ...')
const serialized = serializer.serialize(bom, {
sortLists: options.outputReproducible,
space: 2
})

if (options.validate) {
myConsole.log('LOG | try validate BOM result ...')
if (options.validate ?? true) {
myConsole.log('LOG | try validating BOM result ...')
try {
const validationErrors = await validator.validate(serialized)
if (validationErrors === null) {
Expand All @@ -293,7 +294,13 @@ export async function run (process: NodeJS.Process): Promise<number> {
}
} catch (err) {
if (err instanceof Validation.MissingOptionalDependencyError) {
myConsole.info('INFO | skipped validate BOM:', err.message)
if (options.validate === true) {
// if explicitly requested to validate, then warn about skip
myConsole.warn('WARN | skipped validating BOM:', err.message)
// @TODO breaking change: forward error, do not skip/continue
} else {
myConsole.info('INFO | skipped validating BOM:', err.message)
}
} else {
myConsole.error('ERROR | unexpected error')
throw err
Expand Down
1 change: 0 additions & 1 deletion tests/integration/synthetics/cli.run.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,6 @@ describe('cli.run()', () => {
'-vvv',
'--ignore-npm-errors',
'--output-reproducible',
'--validate',
// no intention to test all the spec-versions nor all the output-formats - this would be not our scope.
'--spec-version', '1.4',
'--output-format', 'JSON',
Expand Down

0 comments on commit 1256457

Please sign in to comment.