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

improve error message for explicitly empty appliesTo #1335

Open
2 tasks
cdisselkoen opened this issue Nov 22, 2024 · 4 comments
Open
2 tasks

improve error message for explicitly empty appliesTo #1335

cdisselkoen opened this issue Nov 22, 2024 · 4 comments
Labels
papercut Small annoyances in the Cedar SDK. Lower priority fixes than bugs. Smaller than a fature request

Comments

@cdisselkoen
Copy link
Contributor

Category

Error message improvements

Describe the feature you'd like to request

For the following schema:

action Read appliesTo {
  principal: [],
  resource: [],
};

The error message when parsing this is currently (as of 4.2.2)

  × error parsing schema: An empty list was passed

This could be more clear. It also doesn't have any locating information, like a source location, or an indication of which action had the parsing problem.

Describe alternatives you've considered

N/A

Additional context

No response

Is this something that you'd be interested in working on?

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change
@cdisselkoen cdisselkoen added pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. feature-request This issue requets a substantial new feature labels Nov 22, 2024
@john-h-kastner-aws john-h-kastner-aws added papercut Small annoyances in the Cedar SDK. Lower priority fixes than bugs. Smaller than a fature request and removed pending-triage The cedar maintainers haven't looked at this yet. Automicaly added to all new issues. feature-request This issue requets a substantial new feature labels Nov 25, 2024
@shaobo-he-aws
Copy link
Contributor

It seems a deeper issue is that parse errors thrown by lalrpop do not get access to the source string and hence cannot properly implement the label method.

@john-h-kastner-aws
Copy link
Contributor

john-h-kastner-aws commented Nov 27, 2024

See #948 and #977 for missing source locations in cedar schema parsing errors

@cdisselkoen
Copy link
Contributor Author

We did adjust the lalrpop for the policy format to provide the source string (#516). Perhaps we need the same change in the schema-format lalrpop file.

@shaobo-he-aws
Copy link
Contributor

We did adjust the lalrpop for the policy format to provide the source string (#516). Perhaps we need the same change in the schema-format lalrpop file.

There's certain part missing in the Cedar schema parser. I'm looking into it and see if we can reuse code for both parsers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
papercut Small annoyances in the Cedar SDK. Lower priority fixes than bugs. Smaller than a fature request
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

3 participants