Skip to content

Conversation

@jdegoes
Copy link
Member

@jdegoes jdegoes commented Jan 29, 2026

Summary

This PR unifies all error types under SchemaError, ensuring that when combining JsonSelection or DynamicValueSelection via ++, errors are properly aggregated instead of being lost.

Closes #868

Problem

When combining two selections that both contain errors using ++, the previous implementation would:

  • For JsonError: Concatenate messages with ; separator, losing path info from the second error
  • For DynamicValueError: Same string concatenation issue

Solution

Replace both JsonError and DynamicValueError with SchemaError, which already supports error aggregation via its ++ operator that maintains a list of Single errors.

Changes

SchemaError enhancements

  • Add SchemaError.Message case class for simple message+path errors
  • Add SchemaError.message(details, path) constructor
  • Add SchemaError.apply(details) convenience constructor
  • Add atField, atIndex, atKey, atCase methods that prepend paths to all errors
  • Add remapSource helper for updating paths in error aggregation

JsonSelection & DynamicValueSelection

  • Replace Either[JsonError, ...] with Either[SchemaError, ...]
  • Replace Either[DynamicValueError, ...] with Either[SchemaError, ...]
  • Update ++ operator to aggregate errors when both sides fail

Files deleted

  • JsonError.scala - replaced by SchemaError
  • DynamicValueError.scala - replaced by SchemaError
  • DynamicValueErrorSpec.scala - replaced by SchemaErrorPathSpec.scala

Test updates

  • Add SchemaErrorPathSpec with comprehensive tests for path methods
  • Update all tests using JsonError/DynamicValueError to use SchemaError
  • Add tests verifying error aggregation in ++ operator

Testing

  • All tests pass with 100% statement coverage
  • Tested on both Scala 3.3.7 and Scala 2.13.18

This PR unifies all error types under SchemaError, ensuring that when
combining JsonSelection or DynamicValueSelection via ++, errors are
properly aggregated instead of being lost.

## Changes

### SchemaError enhancements
- Add SchemaError.Message case class for simple message+path errors
- Add SchemaError.message(details, path) constructor
- Add SchemaError.apply(details) convenience constructor
- Add atField, atIndex, atKey, atCase methods that prepend to all errors
- Add remapSource helper for updating paths in error aggregation

### JsonSelection & DynamicValueSelection
- Replace Either[JsonError, ...] with Either[SchemaError, ...]
- Replace Either[DynamicValueError, ...] with Either[SchemaError, ...]
- Update ++ operator to aggregate errors when both sides fail
- Previously: only first error was kept
- Now: both errors are aggregated into a single SchemaError

### Files deleted
- JsonError.scala - replaced by SchemaError
- DynamicValueError.scala - replaced by SchemaError
- DynamicValueErrorSpec.scala - replaced by SchemaErrorPathSpec.scala

### Test updates
- Add SchemaErrorPathSpec with comprehensive tests for path methods
- Update all tests using JsonError/DynamicValueError to use SchemaError
- Add tests verifying error aggregation in ++ operator

Closes #868

Amp-Thread-ID: https://ampcode.com/threads/T-019c078a-eb23-745d-94ed-d97ec21956d8
Co-authored-by: Amp <amp@ampcode.com>
Copilot AI review requested due to automatic review settings January 29, 2026 02:48
Copy link
Contributor

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 unifies JSON and DynamicValue error handling under SchemaError, so that selection and decoding operations can aggregate multiple errors (with paths) instead of losing detail via string concatenation.

Changes:

  • Replace JsonError/DynamicValueError with SchemaError across JSON and DynamicValue selection, navigation, and decoding APIs, including JsonSelection, DynamicValueSelection, Json, DynamicValue, and JsonDecoder.
  • Extend SchemaError with message-based errors, path-prepend helpers (atField, atIndex, atKey, atCase), and a source-remapping helper so aggregated errors keep accurate paths.
  • Update and extend tests: add SchemaErrorPathSpec, adapt JSON/DynamicValue selection specs to SchemaError, and add dedicated tests for error aggregation (++) and exception behavior, while removing the now-obsolete error classes and specs.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
schema/shared/src/main/scala/zio/blocks/schema/json/JsonSelection.scala Switches JsonSelection to Either[SchemaError, Vector[Json]] and updates combinators (including ++) and extraction methods to propagate and aggregate SchemaErrors.
schema/shared/src/main/scala/zio/blocks/schema/DynamicValueSelection.scala Migrates DynamicValueSelection to Either[SchemaError, Chunk[DynamicValue]] and updates ++, type-directed extraction, and unsafe helpers to work with SchemaError aggregation.
schema/shared/src/main/scala/zio/blocks/schema/SchemaError.scala Adds SchemaError.Message, convenience constructors (message, apply(String)), path-prepend methods for all aggregated errors, and a remapSource helper used by those methods.
schema/shared/src/main/scala/zio/blocks/schema/json/JsonDecoder.scala Changes all decoder signatures to return Either[SchemaError, A], threads SchemaError through collection and composite decoders, and forwards schema-derived codec errors as SchemaError; note: in eitherDecoder, the Right-branch still tags errors with .atField("Left"), which mislabels the error path and should use "Right" instead.
schema/shared/src/main/scala/zio/blocks/schema/json/Json.scala Replaces JsonError with SchemaError for navigation (get), path-based mutation (modifyOrFail, setOrFail, deleteOrFail, insertOrFail), folds, parses, and reconstruction helpers, ensuring codec errors are surfaced as SchemaError.
schema/shared/src/main/scala/zio/blocks/schema/DynamicValue.scala Replaces DynamicValueError with SchemaError throughout navigation, path-based updates, folds, and KV reconstruction, using SchemaError.message(..., path) to record precise paths.
schema/shared/src/main/scala/zio/blocks/schema/json/JsonError.scala Deleted; functionality is replaced by SchemaError.Message and the enriched SchemaError API.
schema/shared/src/main/scala/zio/blocks/schema/DynamicValueError.scala Deleted; replaced by SchemaError.Message and unified SchemaError handling for DynamicValue operations.
schema/shared/src/test/scala/zio/blocks/schema/json/JsonSpec.scala Updates tests from JsonError to SchemaError, adjusts path assertions to the new SchemaError shape, adds suites validating SchemaError exception behavior and ++ aggregation in the JSON context.
schema/shared/src/test/scala/zio/blocks/schema/json/JsonSelectionSpec.scala Migrates selection tests to SchemaError, including new tests verifying ++ aggregates errors and that type-directed extraction methods (as, unwrap, unwrapAll) now use SchemaError.
schema/shared/src/test/scala/zio/blocks/schema/DynamicValueSelectionSpec.scala Adapts DynamicValueSelection tests to SchemaError, adds coverage for error aggregation in ++, and keeps unsafe methods asserting that SchemaError is thrown.
schema/shared/src/test/scala/zio/blocks/schema/DynamicValueSpec.scala Switches fold failure tests from DynamicValueError to SchemaError to match the new API.
schema/shared/src/test/scala/zio/blocks/schema/SchemaErrorPathSpec.scala New spec comprehensively testing SchemaError message formatting, ++ aggregation, and path-prepend behavior (atField, atIndex, atKey, atCase) across single and aggregated errors.
schema/shared/src/test/scala/zio/blocks/schema/DynamicValueErrorSpec.scala Removed; its coverage is replaced by SchemaErrorPathSpec and the DynamicValue/selection tests that now validate SchemaError behavior.

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

@@ -389,9 +389,9 @@ object JsonDecoder {
case Left(error) => return new Left(error.atField("Left"))
Copy link

Copilot AI Jan 29, 2026

Choose a reason for hiding this comment

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

In the eitherDecoder implementation, the error path for the Right-branch currently uses error.atField("Left"), which incorrectly attributes decoding failures under the Right key to the Left field. This will produce misleading error paths when decoding fails in the Right case. To keep error reporting accurate, the Right-branch should tag errors with the "Right" field name instead.

Suggested change
case Left(error) => return new Left(error.atField("Left"))
case Left(error) => return new Left(error.atField("Right"))

Copilot uses AI. Check for mistakes.
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.

Ensure ++ on JsonSelection & DynamicValueSelection does not lose errors

2 participants