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 the detailed error model #22

Merged
merged 8 commits into from
Dec 17, 2024
Merged

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Dec 12, 2024

Motivation:

A common extension to gRPC is a the detailed error model. While not a
standard gRPC feature it is widely used. The standard error model only
allows for a status code and message to be propagated to the client. The
detailed error model allows users to use a common set of structured
errors and send them to the client in metadata. The model uses the
'google.rpc.Status' protobuf message to describe the error and any error
details.

Modifications:

  • Update the fetch and generate protobuf scripts to get and build the
    relevant standard protobuf messages.
  • Add ErrorDetails which is a container for common error types. Each
    of the error types maps to one a standard error detail from the
    detailed error model.
  • Add a GoogleRPCStatus error which maps to the 'google.rpc.Status'
    protobuf. This also conforms to RPCErrorConvertible so gRPC will
    automaticlly turn it into an appropriate status and metadata.
  • Add a helper for unpacking the GoogleRPCStatus from an RPCError.

Result:

Support for the detailed error mode.

Motivation:

A common extension to gRPC is a the detailed error model. While not a
standard gRPC feature it is widely used. The standard error model only
allows for a status code and message to be propagated to the client. The
detailed error model allows users to use a common set of structured
errors and send them to the client in metadata. The model uses the
'google.rpc.Status' protobuf message to describe the error and any error
details.

Modifications:

- Update the fetch and generate protobuf scripts to get and build the
  relevant standard protobuf messages.
- Add `ErrorDetails` which is a container for common error types. Each
  of the error types maps to one a standard error detail from the
  detailed error model.
- Add a `GoogleRPCStatus` error which maps to the 'google.rpc.Status'
  protobuf. This also conforms to `RPCErrorConvertible` so gRPC will
  automaticlly turn it into an appropriate status and metadata.
- Add a helper for unpacking the `GoogleRPCStatus` from an `RPCError`.

Result:

Support for the detailed error mode.
@glbrntt glbrntt added the 🆕 semver/minor Adds new public API. label Dec 12, 2024
@glbrntt
Copy link
Collaborator Author

glbrntt commented Dec 12, 2024

This is split into a few commits:

  • e58d337 updates the proto fetch and generation scripts
  • 8dd2ad1 fetches the protos from upstream
  • df1134f generates sources from the fetch protos
  • f8681a is the work built on top of that

@glbrntt glbrntt marked this pull request as ready for review December 12, 2024 13:46
@glbrntt glbrntt requested a review from gjcairo December 12, 2024 13:46
Sources/GRPCProtobuf/Errors/ErrorDetails.swift Outdated Show resolved Hide resolved
Sources/GRPCProtobuf/Errors/ErrorDetails.swift Outdated Show resolved Hide resolved
Sources/GRPCProtobuf/Errors/GoogleRPCStatus.swift Outdated Show resolved Hide resolved
Sources/GRPCProtobuf/Errors/GoogleRPCStatus.swift Outdated Show resolved Hide resolved
/// > the serialized bytes of a "google.protobuf.Any" protocol buffers message. The content of which
/// > is a "google.rpc.Status" protocol buffers message containing the status code, message, and
/// > details.
public struct GoogleRPCStatus: Error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not GoogleRPCError? We refer to it as an error, conforms to Error, we compare it to RPCError, and it wraps an RPCError.Code and [ErrorDetails].

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Naming this type is annoying... I picked GoogleRPCStatus because it's effectively a "google.rpc.Status".

Other implementations which support this use the generated proto directly so also call it similarly (depends a bit on the language and how they generate proto messages). I elected not do that for a number of reasons; naming, retroactive conformance to RPCErrorConvertible, user experience etc.

The other idea I had for naming it was DetailedRPCError.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. Personally, I think DetailedRPCError fits better. But I also understand wanting to match google.rpc.Status. So I'll leave it up to you, I'm not convinced either way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think there's a lot of benefit to giving it a name thats similar to other implementations especially as this isn't a gRPC feature, it's a widely used convention built on top of gRPC.

If this was a custom grpc-swift thing then I'd definitely prefer a name like DetailedRPCError.

Sources/GRPCProtobuf/Errors/ErrorDetails.swift Outdated Show resolved Hide resolved
/// > the serialized bytes of a "google.protobuf.Any" protocol buffers message. The content of which
/// > is a "google.rpc.Status" protocol buffers message containing the status code, message, and
/// > details.
public struct GoogleRPCStatus: Error {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Naming this type is annoying... I picked GoogleRPCStatus because it's effectively a "google.rpc.Status".

Other implementations which support this use the generated proto directly so also call it similarly (depends a bit on the language and how they generate proto messages). I elected not do that for a number of reasons; naming, retroactive conformance to RPCErrorConvertible, user experience etc.

The other idea I had for naming it was DetailedRPCError.

@glbrntt glbrntt requested a review from gjcairo December 17, 2024 12:23
@glbrntt glbrntt merged commit fd197ad into grpc:main Dec 17, 2024
21 checks passed
@glbrntt glbrntt deleted the errors branch December 17, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants