Skip to content

Refactor validation error message format to use zod-validation-error lib#50

Open
mjewildnhs wants to merge 10 commits intomainfrom
feature/CCM-14201-minor-refactorings
Open

Refactor validation error message format to use zod-validation-error lib#50
mjewildnhs wants to merge 10 commits intomainfrom
feature/CCM-14201-minor-refactorings

Conversation

@mjewildnhs
Copy link
Contributor

@mjewildnhs mjewildnhs commented Mar 9, 2026

Description

Refactor validation error message format to use zod-validation-error lib

Context

To cut down on hand-rolled code we can get cheaply elsewhere.

Type of changes

  • Refactoring (non-breaking change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would change existing functionality)
  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • I am familiar with the contributing guidelines
  • I have followed the code style of the project
  • I have added tests to cover my changes
  • I have updated the documentation accordingly
  • This PR is a result of pair or mob programming
  • If I have used the 'skip-trivy-package' label I have done so responsibly and in the knowledge that this is being fixed as part of a separate ticket/PR.

Sensitive Information Declaration

To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.

  • I confirm that neither PII/PID nor sensitive data are included in this PR and the codebase changes.

rhyscoxnhs and others added 9 commits March 4, 2026 16:41
* CCM-14637 - Set up integration tests

* CCM-14637 - Attempt to use AWS_ACCOUNT_ID env var instead of fetching it via STS
* Fix open handle errors in unit tests

* Switch back to v8 as coverage provider and update to fix bug

* Move index* tests which test cong-loader-service to config-loader-service test

* Mock logger in config-loader / subscription filter tests

* Split subscription filter tests

* Fix restoration of env var in config-loader-service test
@mjewildnhs mjewildnhs requested a review from a team as a code owner March 9, 2026 10:28
@mjewildnhs mjewildnhs requested a review from Copilot March 9, 2026 10:29
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the validation error message formatting in the client-transform-filter-lambda to use the zod-validation-error library instead of hand-rolled path formatting logic. The change removes the custom formatValidationIssuePath function and delegates error message generation to fromZodError().

Changes:

  • Replaced custom formatValidationIssuePath utility and manual zod error iteration with fromZodError() from zod-validation-error in both event and config validators
  • Removed the formatValidationIssuePath function and its tests from the error-handler module
  • Added zod-validation-error@^5.0.0 as a dependency and disabled the unicorn/no-array-reduce ESLint rule

Reviewed changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
package-lock.json Adds zod-validation-error@5.0.0 to the lock file
lambdas/client-transform-filter-lambda/package.json Adds zod-validation-error@^5.0.0 dependency
lambdas/client-transform-filter-lambda/src/services/validators/event-validator.ts Replaces manual zod error formatting with fromZodError()
lambdas/client-transform-filter-lambda/src/services/validators/config-validator.ts Replaces manual zod issue mapping with single fromZodError() call
lambdas/client-transform-filter-lambda/src/services/error-handler.ts Removes unused formatValidationIssuePath function
lambdas/client-transform-filter-lambda/src/__tests__/validators/event-validator.test.ts Updates expected error messages to match zod-validation-error format
lambdas/client-transform-filter-lambda/src/__tests__/validators/config-validator.test.ts Adds test for new error message format
lambdas/client-transform-filter-lambda/src/__tests__/services/error-handler.test.ts Removes tests for deleted formatValidationIssuePath
lambdas/client-transform-filter-lambda/src/__tests__/index.test.ts Updates integration test to match new error format
eslint.config.mjs Disables unicorn/no-array-reduce rule

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


expect(() => validateStatusPublishEvent(invalidEvent)).toThrow(
"data.timestamp must be a valid RFC 3339 timestamp",
'Validation error: Data.timestamp must be a valid RFC 3339 timestamp at "timestamp"',
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The zod-validation-error library auto-capitalizes the first letter of the custom error message, turning "data.timestamp must be a valid RFC 3339 timestamp" into "Data.timestamp must be a valid RFC 3339 timestamp". This makes "Data" ambiguous — it looks like a class/type name rather than the intended JSON path data.timestamp. The same happens for data.channels.

Consider updating the custom messages in the schema (e.g., to "Timestamp must be a valid RFC 3339 timestamp") to avoid the awkward capitalization of the data. path prefix, since fromZodError already appends the path information via at "timestamp".

Suggested change
'Validation error: Data.timestamp must be a valid RFC 3339 timestamp at "timestamp"',
'Validation error: Timestamp must be a valid RFC 3339 timestamp at "timestamp"',

Copilot uses AI. Check for mistakes.
Comment on lines +134 to +137
const errorMessage = fromZodError(result.error).message;
throw new ConfigValidationError([
{ path: "config", message: errorMessage },
]);
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring collapses all validation issues into a single ValidationIssue with a hardcoded path: "config", losing per-issue path granularity. Previously, each zod issue was mapped to its own ValidationIssue with a specific path (e.g., [0].Name, [0].Targets[0].InvocationEndpoint), which was logged in config-loader.ts at line 21 (validationErrors: error.issues).

Now the logged output for a config with multiple errors will be a single issue [{ path: "config", message: "Validation error: ... ; ..." }] instead of an array of separate, individually-addressed issues. This degrades the observability of config validation failures.

Consider either keeping the per-issue structure (mapping fromZodError's details back to individual issues) or simplifying ConfigValidationError to just hold a message string instead of an issues array, which would better reflect the new single-issue semantics.

Copilot uses AI. Check for mistakes.
Base automatically changed from feature/CCM-14201 to main March 10, 2026 15:38
@rhyscoxnhs rhyscoxnhs requested review from a team as code owners March 10, 2026 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants