Skip to content

Rjf/one or many error#487

Merged
robfitzgerald merged 4 commits intomainfrom
rjf/one-or-many-error
Mar 27, 2026
Merged

Rjf/one or many error#487
robfitzgerald merged 4 commits intomainfrom
rjf/one-or-many-error

Conversation

@robfitzgerald
Copy link
Copy Markdown
Collaborator

@robfitzgerald robfitzgerald commented Mar 27, 2026

this PR addresses a limitation in the OneOrMany type during deserialization errors. if the inner type T fails to deserialize, the OneOrMany deserialization error gobbles up the inner error message and only returns (for, in this case, a config file named config_file.toml):

failure building compass app: while reading config_file.toml: data did not match any variant of untagged enum OneOrMany

which is the default deserialization error of an #[serde(untagged)] enum. this PR implements a custom deserializer that will not fail to propagate the inner error message. tests are added that demonstrate the improvement.

Copy link
Copy Markdown
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 improves config deserialization diagnostics by replacing OneOrMany<T>’s derived Serde behavior with a custom Deserialize implementation that preserves inner T deserialization errors, and adds unit tests to validate the improved error messages.

Changes:

  • Implement a custom serde::de::Visitor + Deserialize for OneOrMany<T> to propagate inner errors.
  • Remove derived Deserialize/#[serde(untagged)] usage in favor of the custom deserializer.
  • Add unit tests covering single vs many cases and error-message propagation.

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

@robfitzgerald robfitzgerald requested a review from nreinicke March 27, 2026 20:43
Copy link
Copy Markdown
Collaborator

@nreinicke nreinicke left a comment

Choose a reason for hiding this comment

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

This all makes sense to me

@robfitzgerald robfitzgerald merged commit afc7b72 into main Mar 27, 2026
5 checks passed
@robfitzgerald robfitzgerald deleted the rjf/one-or-many-error branch March 27, 2026 22:05
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