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

Specify explicit error type for all error scenarios #115

Closed
wants to merge 4 commits into from

Conversation

sebastien-rosset
Copy link
Contributor

@sebastien-rosset sebastien-rosset commented Mar 19, 2023

This PR changes the wording of the spec to state the specific error type in all error scenarios.

I have found 34 occurrences of the word "error" in the spec. In most cases, the spec indicates the exact type of error, e.g., invalid-type, unknown-function, invalid-arity. However, there are three occurrences when no explicit error type is mentioned.

In two cases, I think the right error type is invalid-type. In the third case (Slices section), I'm not sure what the error type should be. Maybe invalid-value, but it's not specified anywhere in the spec (I see only three error types defined). Related to this, I have raised #116 to add a section for error handling.

- If the given step is 0, an error MUST be raised.
+ If the given step is 0, an `invalid-value` error MUST be raised.

@sebastien-rosset sebastien-rosset changed the title Use explicit error type Specify explicit error type for all error scenarios Mar 19, 2023
@springcomp
Copy link
Contributor

springcomp commented Mar 21, 2023

invalid-value is indeed a new error type introduced in JMESPath Community that did not originally exists, but I think it’s a good error to report in this case.

Copy link
Member

@jamesls jamesls left a comment

Choose a reason for hiding this comment

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

Looks good, thanks!

@@ -348,7 +348,7 @@ Slice expressions adhere to the following rules:
* If no stop position is given, it is assumed to be the length of the array if
the given step is greater than 0 or 0 if the given step is less than 0.
* If the given step is omitted, it it assumed to be 1.
* If the given step is 0, an error MUST be raised.
* If the given step is 0, an ``invalid-value`` error MUST be raised.
Copy link
Member

@jamesls jamesls Mar 24, 2023

Choose a reason for hiding this comment

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

EDIT: never mind, while the spec never specified the error type, the compliance tests did so I'm ok with updating the spec here.

jamesls added a commit that referenced this pull request Mar 24, 2023
PR #115

* error-handling:
  Use explicit error types
@jamesls
Copy link
Member

jamesls commented Mar 24, 2023

Rebased and merged in 98abdcf, thanks again!

@jamesls jamesls closed this Mar 24, 2023
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.

None yet

3 participants