Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show more informative error when dynamic.dict fails #741

Closed
wants to merge 2 commits into from

Conversation

sporto
Copy link
Contributor

@sporto sporto commented Nov 16, 2024

#740

This makes the decoding error a bit more informative by showing the index of the failed value or key.
As the key can decode to anything (not necessarily a string). It is not possible to show the name of the key.

@@ -1132,14 +1132,14 @@ pub fn dict_test() {
|> dynamic.from
|> dynamic.dict(dynamic.int, dynamic.int)
|> should.equal(
Error([DecodeError(expected: "Int", found: "String", path: ["keys"])]),
Error([DecodeError(expected: "Int", found: "String", path: ["keys[0]"])]),
Copy link
Member

@lpil lpil Nov 17, 2024

Choose a reason for hiding this comment

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

This isn't the syntax for indexing in this module, and you can't index into the keys with 0, they are not ordered.

Copy link
Contributor Author

@sporto sporto Nov 17, 2024

Choose a reason for hiding this comment

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

Hi

Not sure what do you mean with the syntax comment.

The values[1] is just meant to show the index of the failed input. Changed the test to reflect this.

The intention is to have some way to find the problematic input in the original data source.

e.g. given

{
  "dog": { "id": 1 },
  "cat": { "id": 2 },
  "fish": { "id": null },
  "rabbit": { "id": 4 },
}

I will like to be able to see an error showing values[2] which points me to fish.

Not sure however if the index will be correct when parsing something like this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The indexing only works on erlang. We need a different strategy.

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.

2 participants