diff --git a/packages/common/src/project/load.spec.ts b/packages/common/src/project/load.spec.ts new file mode 100644 index 0000000000..7f0363a893 --- /dev/null +++ b/packages/common/src/project/load.spec.ts @@ -0,0 +1,290 @@ +// Copyright 2020-2025 SubQuery Pte Ltd authors & contributors +// SPDX-License-Identifier: GPL-3.0 + +import {validateCommonProjectManifest} from './load'; + +describe('validateCommonProjectManifest', () => { + /** + * Example of error output format improvement: + * + * BEFORE (old format using toString()): + * project validation failed. + * An instance of CommonProjectManifestV1_0_0Impl has failed the validation: + * - property version has failed the following constraints: isString + * - property schema has failed the following constraints: nested property schema must be either object or array + * + * AFTER (new structured format): + * project validation failed. + * - version: version must be a string + * - schema: schema must be an object + * - network: network must be an object + * - runner: runner must be an object + * - dataSources: dataSources must be an array + */ + it('should throw error with structured format for missing required fields', () => { + const invalidManifest = { + specVersion: '1.0.0', + // Missing required fields: version, schema, network, runner, dataSources + }; + + expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); + + try { + validateCommonProjectManifest(invalidManifest); + fail('Expected validation to throw an error'); + } catch (error: any) { + const errorMessage = error.message; + + // Check that error message starts with the expected prefix + expect(errorMessage).toContain('project validation failed.'); + + // Check that error message contains structured format with property names + // The new format should include property names and constraints + expect(errorMessage).toMatch(/ - \w+:/); + + // Verify that multiple properties are listed + const lines = errorMessage.split('\n').filter((line: string) => line.trim().startsWith('-')); + expect(lines.length).toBeGreaterThan(0); + + // Each line should follow the format: " - propertyName: constraint message" + lines.forEach((line: string) => { + expect(line).toMatch(/^\s+-\s+\w+:\s+.+$/); + }); + + // Verify specific properties are mentioned + expect(errorMessage).toMatch(/version|schema|network|runner|dataSources/); + } + }); + + it('should throw error with property-specific constraints for invalid specVersion', () => { + const invalidManifest = { + specVersion: '2.0.0', // Invalid: must be '1.0.0' + version: '1.0.0', + schema: {file: 'schema.graphql'}, + network: {chainId: '0x123'}, + runner: { + node: {name: '@subql/node', version: '1.0.0'}, + query: {name: '@subql/query', version: '1.0.0'}, + }, + dataSources: [], + }; + + expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); + + try { + validateCommonProjectManifest(invalidManifest); + fail('Expected validation to throw an error'); + } catch (error: any) { + const errorMessage = error.message; + + // Should contain specVersion property in error + expect(errorMessage).toContain('specVersion'); + + // Should contain constraint information in structured format + expect(errorMessage).toMatch(/specVersion:\s*.+/); + + // Verify the format: " - specVersion: constraint message" + const specVersionLine = errorMessage.split('\n').find((line: string) => line.includes('specVersion')); + expect(specVersionLine).toMatch(/^\s+-\s+specVersion:\s+.+$/); + } + }); + + it('should throw error with structured format for invalid runner query name', () => { + const invalidManifest = { + specVersion: '1.0.0', + version: '1.0.0', + schema: {file: 'schema.graphql'}, + network: {chainId: '0x123'}, + runner: { + node: {name: '@subql/node', version: '1.0.0'}, + query: {name: '@subql/invalid-query', version: '1.0.0'}, // Invalid query name + }, + dataSources: [], + }; + + expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); + + try { + validateCommonProjectManifest(invalidManifest); + fail('Expected validation to throw an error'); + } catch (error: any) { + const errorMessage = error.message; + + // Should contain query property in error (nested in runner.query.name) + expect(errorMessage).toContain('query'); + + // Error should be structured with property path + expect(errorMessage).toMatch(/query/); + + // Verify structured format for nested property + const queryLine = errorMessage.split('\n').find((line: string) => line.includes('query')); + if (queryLine) { + expect(queryLine).toMatch(/^\s+-\s+query/); + } + } + }); + + it('should throw error with structured format for missing chainId', () => { + const invalidManifest = { + specVersion: '1.0.0', + version: '1.0.0', + schema: {file: 'schema.graphql'}, + network: { + // Missing required chainId + endpoint: 'wss://example.com', + }, + runner: { + node: {name: '@subql/node', version: '1.0.0'}, + query: {name: '@subql/query', version: '1.0.0'}, + }, + dataSources: [], + }; + + expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); + + try { + validateCommonProjectManifest(invalidManifest); + fail('Expected validation to throw an error'); + } catch (error: any) { + const errorMessage = error.message; + + // Should contain chainId or network property in error + expect(errorMessage).toMatch(/chainId|network/); + + // Should be in structured format + expect(errorMessage).toMatch(/ - \w+:/); + + // Verify the error format shows property name and constraint + const chainIdLine = errorMessage.split('\n').find((line: string) => + line.includes('chainId') || line.includes('network') + ); + if (chainIdLine) { + expect(chainIdLine).toMatch(/^\s+-\s+\w+:\s+.+$/); + } + } + }); + + it('should demonstrate improved error message format with actual output', () => { + const invalidManifest = { + specVersion: '1.0.0', + version: '', // Empty string should fail validation + schema: {file: 'schema.graphql'}, + network: {chainId: '0x123'}, + runner: { + node: {name: '@subql/node', version: '1.0.0'}, + query: {name: '@subql/query', version: '1.0.0'}, + }, + dataSources: [], + }; + + try { + validateCommonProjectManifest(invalidManifest); + fail('Expected validation to throw an error'); + } catch (error: any) { + const errorMessage = error.message; + + // The new format should be clear and structured + // Example output: + // project validation failed. + // - version: version must be a string + + expect(errorMessage).toContain('project validation failed.'); + expect(errorMessage).toContain('version'); + + // Verify the structured format is present + const versionErrorLine = errorMessage.split('\n').find((line: string) => + line.includes('version') && line.includes(':') + ); + expect(versionErrorLine).toBeDefined(); + expect(versionErrorLine).toMatch(/^\s+-\s+version:\s+.+$/); + } + }); + + it('should not throw error for valid manifest', () => { + const validManifest = { + specVersion: '1.0.0', + version: '1.0.0', + schema: {file: 'schema.graphql'}, + network: { + chainId: '0x123', + endpoint: 'wss://example.com', + }, + runner: { + node: {name: '@subql/node', version: '1.0.0'}, + query: {name: '@subql/query', version: '1.0.0'}, + }, + dataSources: [], + }; + + expect(() => validateCommonProjectManifest(validManifest)).not.toThrow(); + }); + + it('should format errors for nested child objects with array indices', () => { + // This test demonstrates how errors are formatted for deeply nested objects, + // such as an invalid filter on a mapping handler. + // The error path should use bracket notation for array indices: dataSources[0].mapping.handlers[1].filter + const invalidManifest = { + specVersion: '1.0.0', + version: '1.0.0', + schema: {file: 'schema.graphql'}, + network: { + chainId: '0x123', + endpoint: 'wss://example.com', + }, + runner: { + node: {name: '@subql/node', version: '1.0.0'}, + query: {name: '@subql/query', version: '1.0.0'}, + }, + dataSources: [ + { + kind: 'substrate/Runtime', + mapping: { + file: 'dist/index.js', + handlers: [ + { + handler: 'handleBlock', + kind: 'substrate/BlockHandler', + }, + { + handler: 'handleEvent', + kind: 'substrate/EventHandler', + filter: { + // Invalid: specVersion should be an array of 2 numbers, not a single number + specVersion: 123, + }, + }, + ], + }, + }, + ], + }; + + expect(() => validateCommonProjectManifest(invalidManifest)).toThrow(); + + try { + validateCommonProjectManifest(invalidManifest); + fail('Expected validation to throw an error'); + } catch (error: any) { + const errorMessage = error.message; + + // Error should contain the structured format + expect(errorMessage).toContain('project validation failed.'); + + // For nested array errors, the path should use bracket notation + // Example: dataSources[0].mapping.handlers[1].filter.specVersion + // Verify that array indices are formatted with brackets + expect(errorMessage).toMatch(/\[\d+\]/); + + // The error message should be structured and readable with full example + // Expected format: " - dataSources[0].mapping.handlers[1].filter.specVersion: " + const errorLines = errorMessage.split('\n').filter((line: string) => line.trim().startsWith('-')); + expect(errorLines.length).toBeGreaterThan(0); + + // Verify at least one line contains array bracket notation and follows the expected format + const arrayErrorLine = errorLines.find((line: string) => /\[\d+\]/.test(line)); + expect(arrayErrorLine).toBeDefined(); + expect(arrayErrorLine).toMatch(/^\s+-\s+.+\[\d+\].+:\s+.+$/); + } + }); +}); + diff --git a/packages/common/src/project/load.ts b/packages/common/src/project/load.ts index 67e0d401cf..91da153d56 100644 --- a/packages/common/src/project/load.ts +++ b/packages/common/src/project/load.ts @@ -10,7 +10,7 @@ import yaml from 'js-yaml'; import {gte} from 'semver'; import {CommonProjectManifestV1_0_0Impl} from '../'; import {NETWORK_FAMILY, runnerMapping} from '../constants'; -import {DEFAULT_MANIFEST, DEFAULT_TS_MANIFEST, extensionIsYamlOrJSON} from './utils'; +import {DEFAULT_MANIFEST, DEFAULT_TS_MANIFEST, extensionIsYamlOrJSON, formatValidationErrors} from './utils'; export function loadFromJsonOrYaml(file: string): unknown { const {ext} = path.parse(file); if (!extensionIsYamlOrJSON(ext)) { @@ -91,8 +91,7 @@ export function validateCommonProjectManifest(raw: unknown): void { const projectManifest = plainToClass(CommonProjectManifestV1_0_0Impl, raw); const errors = validateSync(projectManifest, {whitelist: true}); if (errors?.length) { - // TODO: print error details - const errorMsgs = errors.map((e) => e.toString()).join('\n'); + const errorMsgs = formatValidationErrors(errors).join('\n'); throw new Error(`project validation failed.\n${errorMsgs}`); } } diff --git a/packages/common/src/project/utils.ts b/packages/common/src/project/utils.ts index 451bc3042b..248c7666c1 100644 --- a/packages/common/src/project/utils.ts +++ b/packages/common/src/project/utils.ts @@ -16,6 +16,7 @@ import { registerDecorator, validateSync, ValidationArguments, + ValidationError, ValidationOptions, ValidatorConstraint, ValidatorConstraintInterface, @@ -199,10 +200,41 @@ export async function delay(sec: number): Promise { }); } +/** + * Recursively formats validation errors into a structured format. + * Handles nested errors (errors with children) by recursively processing them. + * Formats array indices with brackets for better readability (e.g., dataSources[0].mapping.handlers[1].filter). + */ +export function formatValidationErrors(errors: ValidationError[], parentPath = ''): string[] { + const errorMessages: string[] = []; + + for (const error of errors) { + // Check if property is a numeric string (array index) + const isArrayIndex = /^\d+$/.test(error.property); + const propertyPath = parentPath + ? isArrayIndex + ? `${parentPath}[${error.property}]` + : `${parentPath}.${error.property}` + : error.property; + + if (error.constraints && Object.keys(error.constraints).length > 0) { + const constraints = Object.values(error.constraints).join(', '); + errorMessages.push(` - ${propertyPath}: ${constraints}`); + } + + // Recursively handle nested errors + if (error.children && error.children.length > 0) { + errorMessages.push(...formatValidationErrors(error.children, propertyPath)); + } + } + + return errorMessages; +} + export function validateObject(object: any, errorMessage = 'failed to validate object.'): void { const errors = validateSync(object, {whitelist: true, forbidNonWhitelisted: true}); if (errors?.length) { - const errorMsgs = errors.map((e) => e.toString()).join('\n'); + const errorMsgs = formatValidationErrors(errors).join('\n'); throw new Error(`${errorMessage}\n${errorMsgs}`); } } diff --git a/packages/common/src/project/versioned/base.ts b/packages/common/src/project/versioned/base.ts index 473908c13b..0b6d06a078 100644 --- a/packages/common/src/project/versioned/base.ts +++ b/packages/common/src/project/versioned/base.ts @@ -21,7 +21,7 @@ import { validateSync, } from 'class-validator'; import yaml from 'js-yaml'; -import {IsEndBlockGreater, toJsonObject} from '../utils'; +import {IsEndBlockGreater, toJsonObject, formatValidationErrors} from '../utils'; import {ParentProjectModel} from './v1_0_0/models'; export abstract class ProjectManifestBaseImpl @@ -53,7 +53,7 @@ export abstract class ProjectManifestBaseImpl validate(): void { const errors = validateSync(this.deployment, {whitelist: true, forbidNonWhitelisted: true}); if (errors?.length) { - const errorMsgs = errors.map((e) => e.toString()).join('\n'); + const errorMsgs = formatValidationErrors(errors).join('\n'); throw new Error(`Failed to parse project. Please see below for more information.\n${errorMsgs}`); } }