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 support for encoding error structure for grpc errors #15

Closed
wants to merge 2 commits into from

Conversation

jsternberg
Copy link

This adds support for encoding the error structure for grpc errors. It
does this by using the details to encode any wrapped errors.

Each error is composed of a Status protobuf. The message included in
that status is the default error message if no additional details are
provided. The first detail is an encoding of the error if one was given.
If there was no encoding possible or it wasn't possible to deserialize
it, a generic error is created with the original message.

Any additional details are any wrapped errors. Each wrapped error is a
Status that follows the same encoding scheme. This allows an error to
wrap multiple errors and the implementation isn't dependent on the
contents.

@jsternberg
Copy link
Author

This is an alternative to #7. I think this implementation might be better because it doesn't rely on the contents of the messages and the structure is encoded directly into the serialization scheme.

Still needs some tests and I'd like to add some functionality to enable the currently existing serialization that buildkit uses for some backwards compatibility, but I wanted to get this idea out there first before spending time on that.

This consolidates the various error types to a single error type with a
code that designates the type of error. This reduces the amount and
complexity of the overall code.

The individual methods like `IsInvalidArgument` that support both errors
from this package and moby errors are now moved to the `compat.go` file
to organize them together.

Signed-off-by: Jonathan A. Sternberg <[email protected]>
@jsternberg jsternberg force-pushed the grpc-errors branch 3 times, most recently from 2f04932 to 93d6dca Compare July 22, 2024 18:11
This adds support for encoding the error structure for grpc errors. It
does this by using the details to encode any wrapped errors.

Each error is composed of a `Status` protobuf. The message included in
that status is the default error message if no additional details are
provided. The first detail is an encoding of the error if one was given.
If there was no encoding possible or it wasn't possible to deserialize
it, a generic error is created with the original message.

Any additional details are any wrapped errors. Each wrapped error is a
`Status` that follows the same encoding scheme. This allows an error to
wrap multiple errors and the implementation isn't dependent on the
contents.

Signed-off-by: Jonathan A. Sternberg <[email protected]>
@jsternberg
Copy link
Author

A more complete and bug free implementation of this is created here: moby/buildkit#5203

I figured it was probably easier to update the buildkit serialization format there and then move that code here when it was ready than it was to write the code here and transition buildkit. That way, we'll at least know it works before it is merged and starts to be used here.

@dmcgowan
Copy link
Member

I'm going to close this one since the other is merged and heavily conflicts. Feel free to open one based on what you learned with the serialization in the buildkit PR. We should be able to rely on the new unit tests to either check for the same behavior or at least detect how it might be different.

@dmcgowan dmcgowan closed this Aug 16, 2024
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