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

Add additional error context onto schema errors #481

Merged
merged 2 commits into from
Mar 23, 2022

Conversation

josephschorr
Copy link
Member

@josephschorr josephschorr commented Mar 16, 2022

Adds source position information onto the compiled namespace definitions and then uses it in the type system to return errors with more context

Fixes #447 and Fixes AUTHZ-422

@github-actions github-actions bot added area/api v0 Affects the v0 API area/api v1 Affects the v1 API area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Mar 16, 2022
@@ -130,18 +130,18 @@ func TestEditCheck(t *testing.T) {

definition resource {
relation writer: user
permission writer = writer
permission writer = writer
Copy link
Member

Choose a reason for hiding this comment

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

nit: spacing

Copy link
Member Author

Choose a reason for hiding this comment

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

Was actually done on purpose to give a different column index on the error message, but I adjusted to make less odd looking as bit

internal/namespace/typesystem.go Show resolved Hide resolved
proto/internal/core/v1/core.proto Show resolved Hide resolved
@josephschorr josephschorr force-pushed the error-context branch 2 times, most recently from feb47d1 to 3e22b63 Compare March 20, 2022 08:26
}

// NewErrorWithSource creates and returns a new ErrorWithSource.
func NewErrorWithSource(err error, sourceCodeString string, lineNumber uint64, columnPosition uint64) *ErrorWithSource {
Copy link
Member

Choose a reason for hiding this comment

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

Note in method doc or change line number and column position param names to include that they are 1-based to make it clear to the caller? All other line/col fields are named and/or documented so might as well for this as well.

@josephschorr
Copy link
Member Author

Updated

Copy link
Member

@samkim samkim left a comment

Choose a reason for hiding this comment

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

LGTM

@josephschorr josephschorr merged commit 7c3993a into authzed:main Mar 23, 2022
@josephschorr josephschorr deleted the error-context branch March 23, 2022 09:06
@github-actions github-actions bot locked and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/api v0 Affects the v0 API area/api v1 Affects the v1 API area/schema Affects the Schema Language area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Have type system errors raise with proper context
2 participants