search

grpc / grpc-swift

The Swift language implementation of gRPC.
Apache License 2.0
2.01k stars 413 forks source link

Support gRPC Richer Error Model or ease trailing metadata access #1892

Open Skoti opened 3 months ago

Skoti commented 3 months ago

Currently, the GRPCStatus is not related to Status defined in google/rpc/status.proto.

According to Richer Error Model specification, the repeated google.protobuf.Any details field

enables servers to return and clients to consume additional error details expressed as one or more protobuf messages.

This is beneficial if you want to go beyond the standard gRPC codes error model and start providing more detailed information.

Preferred solution

The GRPCStatus gains the details field.

Alternative

Since the details field is transported as trailing metadata:

The protobuf binary encoding of this extra error information is provided as trailing metadata in the response.

We can workaround the missing details field. The trailingMetadata is available in all calls, for example: https://github.com/grpc/grpc-swift/blob/716d6d48b3c5f8a44399bbcebb864e8146a36086/Sources/GRPC/AsyncAwaitSupport/GRPCAsyncUnaryCall.swift#L76-L82

The only inconvenient part is that async/await wrappers do not expose it in any way. https://github.com/grpc/grpc-swift/blob/716d6d48b3c5f8a44399bbcebb864e8146a36086/Sources/GRPC/AsyncAwaitSupport/GRPCClient%2BAsyncAwaitSupport.swift#L158-L178

However, the library has GRPCStatusAndTrailers struct that could be used here or perhaps even deeper in the codebase. https://github.com/grpc/grpc-swift/blob/716d6d48b3c5f8a44399bbcebb864e8146a36086/Sources/GRPC/GRPCStatusAndMetadata.swift#L21-L32

Then the async/await wrappers (or maybe at some lower level) instead of simply propagating the error, could transform it to status and then add metadata and throw it, something like:

let status = (error as! GRPCStatusTransformable).makeGRPCStatus()
throw GRPCStatusAndTrailers(status: status, trailers: call.trailingMetadata)

The GRPCStatusAndTrailers could conform to GRPCStatusTransformable for backward compatibility. This would be an easy win, and allow library users to easily obtain detailed error information.

Questions

  1. Do you plan to support the Rich Error Model in the long term?
  2. What do you think about the alternative solution (for example the one above) in the short term for async/await APIs?
glbrntt commented 3 months ago

Thanks for filing a detailed issue!

Questions

  1. Do you plan to support the Rich Error Model in the long term?

For v1, I don't think so. We're focused on v2 at the moment where we do plan to have support.

  1. What do you think about the alternative solution (for example the one above) in the short term for async/await APIs?

I don't think we can make the above change because the thrown error type changes: any users catching a GRPCStatus will have the semantics of their error handling changed by a package update.

Skoti commented 3 months ago

Thanks for the insights 💪

We're focused on v2 at the moment where we do plan to have support.

Great to hear! I guess v2 is still in early development, do you need some manpower? Do you plan to use the upcoming features like typed throws?

I don't think we can make the above change because the thrown error type changes: any users catching a GRPCStatus will have the semantics of their error handling changed by a package update.

True, though the fix would be as easy as casting to GRPCStatusTransformable. But I get it, it's better to keep the focus on v2.