diff --git a/.changeset/diff-parser-error-empty-message.md b/.changeset/diff-parser-error-empty-message.md new file mode 100644 index 000000000..81e2d45bd --- /dev/null +++ b/.changeset/diff-parser-error-empty-message.md @@ -0,0 +1,10 @@ +--- +'@asyncapi/cli': patch +--- + +Fix `asyncapi diff` printing a bare `ValidationError:` with no diagnostic body +when one of the supplied documents fails to parse. The parse-failure branch in +`parseDocuments` used the `'invalid-file'` ValidationError type (the file-not- +found template) instead of `'parser-error'`, and `buildError` produced an +empty string for non-ParserError error shapes such as plain strings or generic +`Error` objects. Both paths now surface a meaningful, non-empty message. diff --git a/src/apps/cli/commands/diff.ts b/src/apps/cli/commands/diff.ts index c76687c8a..9e18bb983 100644 --- a/src/apps/cli/commands/diff.ts +++ b/src/apps/cli/commands/diff.ts @@ -247,7 +247,7 @@ export default class Diff extends Command { if (!firstResult.success || !secondResult.success) { this.error( new ValidationError({ - type: 'invalid-file', + type: 'parser-error', filepath: firstDocument.getFilePath() || secondDocument.getFilePath(), err: firstResult.error || secondResult.error, }), diff --git a/src/errors/validation-error.ts b/src/errors/validation-error.ts index 71dc7b7e1..2582a44d6 100644 --- a/src/errors/validation-error.ts +++ b/src/errors/validation-error.ts @@ -30,40 +30,69 @@ export class ValidationError extends Error { } private buildError(err: any) { - const errorsInfo: Array = []; + const errorsInfo = collectParserErrorInfo(err); - if (err.title) { - errorsInfo.push(err.title); + if (errorsInfo.length > 0) { + this.message = errorsInfo.join('\n'); + return; } - if (err.detail) { - errorsInfo.push(err.details); - } + // Fallback for error shapes that don't follow the @asyncapi/parser + // ParserError contract (plain strings, generic Errors, undefined): surface + // the most informative non-empty value we can find so callers never see a + // bare "ValidationError:" with no diagnostic body. + this.message = + (typeof err === 'string' && err) || + err?.message || + err?.title || + 'Unknown parser error'; + } +} - if (err.validationErrors) { - for (const e of err.validationErrors) { - const errorHasTitle = !!e.title; - const errorHasLocation = !!e.location; - /* - * All the conditions below are needed since validationErrors (from ParserError) come from Parser JS library, - * so we cannot assure that all the fields or properties are always provided in the error. There might be cases - * that even title is not provided. - */ - if (errorHasTitle && errorHasLocation) { - errorsInfo.push( - `${e.title} ${e.location.startLine}:${e.location.startColumn}`, - ); - continue; - } - if (errorHasTitle) { - errorsInfo.push(`${e.title}`); - continue; - } - if (errorHasLocation) { - errorsInfo.push(`${e.location.startLine}:${e.location.startColumn}`); - } +function collectParserErrorInfo(err: any): string[] { + const errorsInfo: string[] = []; + + if (!err) { + return errorsInfo; + } + + if (err.title) { + errorsInfo.push(err.title); + } + + if (err.detail) { + errorsInfo.push(err.details); + } + + if (err.validationErrors) { + for (const e of err.validationErrors) { + const formatted = formatValidationError(e); + if (formatted) { + errorsInfo.push(formatted); } } - this.message = errorsInfo.join('\n'); } + + return errorsInfo; +} + +/* + * Format a single ParserError validationErrors entry. All fields are + * defensive: validationErrors come from the @asyncapi/parser JS library and + * are not guaranteed to provide a title or a location. + */ +function formatValidationError(e: any): string | undefined { + const errorHasTitle = !!e.title; + const errorHasLocation = !!e.location; + + if (errorHasTitle && errorHasLocation) { + return `${e.title} ${e.location.startLine}:${e.location.startColumn}`; + } + if (errorHasTitle) { + return `${e.title}`; + } + if (errorHasLocation) { + return `${e.location.startLine}:${e.location.startColumn}`; + } + return undefined; } diff --git a/test/integration/diff.test.ts b/test/integration/diff.test.ts index 5befd309c..1a6c38a7b 100644 --- a/test/integration/diff.test.ts +++ b/test/integration/diff.test.ts @@ -577,4 +577,19 @@ describe('diff', () => { }); }); }); + + describe('when one of the documents cannot be parsed', () => { + test + .stderr() + .stdout() + .command(['diff', './test/fixtures/specification.yml', './test/fixtures/specification-invalid.yml']) + .it('surfaces a non-empty ValidationError message instead of a bare "ValidationError:"', (ctx, done) => { + expect(ctx.stderr).to.contain('ValidationError'); + // The historical bug emitted a literal "ValidationError:" with no + // diagnostic body; ensure at least one informative token follows. + expect(ctx.stderr).to.not.match(/ValidationError:\s*\n/); + expect(ctx.stderr).to.contain('parse'); + done(); + }); + }); }); diff --git a/test/unit/errors/validation-error.test.ts b/test/unit/errors/validation-error.test.ts new file mode 100644 index 000000000..e7203bd13 --- /dev/null +++ b/test/unit/errors/validation-error.test.ts @@ -0,0 +1,88 @@ +import { expect } from 'chai'; +import { ValidationError } from '../../../src/errors/validation-error'; + +describe('ValidationError', () => { + describe('type: parser-error', () => { + it('formats validationErrors with title and location', () => { + const error = new ValidationError({ + type: 'parser-error', + err: { + title: 'Validation failed', + validationErrors: [ + { title: 'Missing required field', location: { startLine: 3, startColumn: 5 } }, + { title: 'Invalid type' }, + ], + }, + }); + + expect(error.message).to.contain('Validation failed'); + expect(error.message).to.contain('Missing required field 3:5'); + expect(error.message).to.contain('Invalid type'); + }); + + it('falls back to the string err itself when no ParserError fields are present', () => { + // Validation service returns errors as plain strings (e.g. "Failed to + // parse document"). Without the fallback, buildError produced an empty + // message and the user saw a bare "ValidationError:" with no body. + const error = new ValidationError({ + type: 'parser-error', + err: 'Failed to parse document', + }); + + expect(error.message).to.equal('Failed to parse document'); + }); + + it('falls back to err.message for generic Error objects', () => { + const inner = new Error('Something went wrong'); + const error = new ValidationError({ + type: 'parser-error', + err: inner, + }); + + expect(error.message).to.equal('Something went wrong'); + }); + + it('falls back to a sentinel string when err is undefined', () => { + const error = new ValidationError({ + type: 'parser-error', + err: undefined, + }); + + expect(error.message).to.equal('Unknown parser error'); + }); + + it('falls back to err.title for objects without validationErrors', () => { + const error = new ValidationError({ + type: 'parser-error', + err: { title: 'A short title' }, + }); + + expect(error.message).to.equal('A short title'); + }); + }); + + describe('type: invalid-file', () => { + it('renders the file-not-found message and ignores err', () => { + const error = new ValidationError({ + type: 'invalid-file', + filepath: '/tmp/missing.yaml', + err: { title: 'should not appear' }, + }); + + expect(error.message).to.equal( + 'There is no file or context with name "/tmp/missing.yaml".', + ); + }); + }); + + describe('type: invalid-syntax-file', () => { + it('renders the syntax error template', () => { + const error = new ValidationError({ + type: 'invalid-syntax-file', + filepath: '/tmp/bad.yaml', + }); + + expect(error.message).to.equal('Syntax Error in "/tmp/bad.yaml".'); + }); + }); +});