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

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

Open
Skoti opened this issue May 27, 2024 · 2 comments
Open

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

Skoti opened this issue May 27, 2024 · 2 comments

Comments

@Skoti
Copy link

Skoti commented May 27, 2024

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:

public var trailingMetadata: HPACKHeaders {
get async throws {
try await self.withRPCCancellation {
try await self.responseParts.trailingMetadata.get()
}
}
}

The only inconvenient part is that async/await wrappers do not expose it in any way.

extension GRPCClient {
public func performAsyncUnaryCall<Request: Message & Sendable, Response: Message & Sendable>(
path: String,
request: Request,
callOptions: CallOptions? = nil,
interceptors: [ClientInterceptor<Request, Response>] = [],
responseType: Response.Type = Response.self
) async throws -> Response {
let call = self.channel.makeAsyncUnaryCall(
path: path,
request: request,
callOptions: callOptions ?? self.defaultCallOptions,
interceptors: interceptors
)
return try await withTaskCancellationHandler {
try await call.response
} onCancel: {
call.cancel()
}
}

However, the library has GRPCStatusAndTrailers struct that could be used here or perhaps even deeper in the codebase.

public struct GRPCStatusAndTrailers: Equatable {
/// The status.
public var status: GRPCStatus
/// The trailers.
public var trailers: HPACKHeaders?
public init(status: GRPCStatus, trailers: HPACKHeaders? = nil) {
self.status = status
self.trailers = trailers
}
}

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
Copy link
Collaborator

glbrntt commented May 28, 2024

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
Copy link
Author

Skoti commented May 28, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants