Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Update from review comments
  • Loading branch information
tonydnewell committed Aug 31, 2023
commit 1a94ae0eaf159b51612c625f02354a7145f5d9b7
2 changes: 1 addition & 1 deletion build/dependencies.props
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project>
<PropertyGroup>
<BenchmarkDotNetPackageVersion>0.13.1</BenchmarkDotNetPackageVersion>
<GoogleApiCommonProtos>2.10.0</GoogleApiCommonProtos>
<GoogleApiCommonProtosPackageVersion>2.10.0</GoogleApiCommonProtosPackageVersion>
<GoogleApisAuthPackageVersion>1.46.0</GoogleApisAuthPackageVersion>
<GoogleProtobufPackageVersion>3.23.1</GoogleProtobufPackageVersion>
<GrpcDotNetPackageVersion>2.52.0</GrpcDotNetPackageVersion> <!-- Used by example projects -->
Expand Down
102 changes: 102 additions & 0 deletions src/Grpc.StatusProto/ExceptionExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright 2023 gRPC authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

using Google.Rpc;

namespace Grpc.StatusProto;
Copy link
Member

Choose a reason for hiding this comment

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

I think the namespace of the extension types should change. See #2273 (comment)


/// <summary>
/// Extensions methods for <see cref="System.Exception"/>
/// </summary>
public static class ExceptionExtensions
{
/// <summary>
/// Create a <see cref="Google.Rpc.DebugInfo"/> from an <see cref="System.Exception"/>,
/// populating the Message and StackTrace from the exception.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <remarks>
/// <example>
/// For example:
/// <code>
/// try { /* ... */
/// }
/// catch (Exception e) {
/// Google.Rpc.Status status = new() {
/// Code = (int)StatusCode.Internal,
/// Message = "Internal error",
/// Details = {
/// // populate debugInfo from the exception
/// Any.Pack(e.ToRpcDebugInfo())
/// }
/// };
/// // ...
/// }
/// </code>
/// </example>
/// </remarks>
/// <param name="exception"></param>
/// <param name="innerDepth">Maximum number of inner exceptions to include in the StackTrace. Defaults
/// to not including any inner exceptions</param>
/// <returns>
/// A new <see cref="Google.Rpc.DebugInfo"/> populated from the exception.
/// </returns>
public static DebugInfo ToRpcDebugInfo(this Exception exception, int innerDepth = 0)
{
var debugInfo = new DebugInfo();

var message = exception.Message;
var name = exception.GetType().FullName;

// Populate the Detail from the exception type and message
debugInfo.Detail = message is null ? name : name + ": " + message;

// Populate the StackEntries from the exception StackTrace
if (exception.StackTrace is not null)
{
var sr = new StringReader(exception.StackTrace);
var entry = sr.ReadLine();
while (entry is not null)
{
debugInfo.StackEntries.Add(entry);
entry = sr.ReadLine();
}
}

// Add inner exceptions to the StackEntries
var inner = exception.InnerException;
while (innerDepth > 0 && inner is not null)
{
message = inner.Message;
name = inner.GetType().FullName;
debugInfo.StackEntries.Add("InnerException: " + (message is null ? name : name + ": " + message));

if (inner.StackTrace is not null)
{
var sr = new StringReader(inner.StackTrace);
var entry = sr.ReadLine();
while (entry is not null)
{
debugInfo.StackEntries.Add(entry);
entry = sr.ReadLine();
}
}

inner = inner.InnerException;
--innerDepth;
}

return debugInfo;
}
}
4 changes: 2 additions & 2 deletions src/Grpc.StatusProto/Grpc.StatusProto.csproj
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Description>gRPC C# API for error handling using status.proto</Description>
<Description>gRPC C# API for error handling using google/rpc/status.proto</Description>
<PackageTags>gRPC RPC HTTP/2</PackageTags>

<IsGrpcPublishedPackage>true</IsGrpcPublishedPackage>
Expand All @@ -11,7 +11,7 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Google.Api.CommonProtos" Version="$(GoogleApiCommonProtos)" />
<PackageReference Include="Google.Api.CommonProtos" Version="$(GoogleApiCommonProtosPackageVersion)" />
</ItemGroup>

<ItemGroup>
Expand Down
13 changes: 11 additions & 2 deletions src/Grpc.StatusProto/MetadataExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,16 @@ public static class MetadataExtensions
/// Get the <see cref="Google.Rpc.Status"/> from the metadata.
/// If the metadata received contains duplicate status details then the last one found
/// is the one that is returned.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <param name="metadata"></param>
/// <param name="throwOnParseError">if true then <see cref="Google.Protobuf.InvalidProtocolBufferException"/>
/// is thrown if the metadata cannot be parsed. Otherwise null is returned on a parsing error.</param>
/// <returns>
/// The found <see cref="Google.Rpc.Status"/> or null if it was
/// not present or could the data could not be parsed.
/// </returns>
public static Google.Rpc.Status? GetRpcStatus(this Metadata metadata)
public static Google.Rpc.Status? GetRpcStatus(this Metadata metadata, bool throwOnParseError=false)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static Google.Rpc.Status? GetRpcStatus(this Metadata metadata, bool throwOnParseError=false)
public static Google.Rpc.Status? GetRpcStatus(this Metadata metadata, bool throwOnParseError = false)

Copy link
Member

Choose a reason for hiding this comment

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

Why suppress the parse error by default? That seems like it could lead to errors disappearing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@JamesNK JamesNK Sep 7, 2023

Choose a reason for hiding this comment

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

@jskeet Do you know this code? Why did you choose this behavior by default?

Copy link

Choose a reason for hiding this comment

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

Yes, I know this code. It feels like if we're already in an error situation (which is where this code would usually be used), failing in a different way due to parse failures in metadata is unhelpful. It's a bit like ending up with an exception caused by disposing of a socket, when the original exception is trying to write to the socket, if you see what I mean.

Happy to discuss alternatives though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that not losing the original error (because reporting the error itself has problems) is my preference. But no strong views.

{
var entry = metadata.LastOrDefault(t => t.Key == StatusDetailsTrailerName);
if (entry is null)
Expand All @@ -51,14 +54,20 @@ public static class MetadataExtensions
}
catch
{
// If the message is malformed, just report there's no information.
if (throwOnParseError)
{
throw;
}

// By default if the message is malformed, just report there's no information.
return null;
}
}

/// <summary>
/// Add <see cref="Google.Rpc.Status"/> to the metadata.
/// Any existing status in the metadata will be overwritten.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <param name="metadata"></param>
/// <param name="status">Status to add</param>
Expand Down
64 changes: 49 additions & 15 deletions src/Grpc.StatusProto/README.md
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
# Grpc C# API for error handling with Status.proto
# Grpc C# API for error handling with status.proto

This is a protoype NuGet package providing client and server side support for the
This is a protoype NuGet package providing C# and .NET client and server side support for the
[gRPC richer error model](https://grpc.io/docs/guides/error/#richer-error-model).

It had dependencies NuGet packages on:
This feature is already available in many other implementations including C++,
Go, Java and Python.

This package has dependencies on these NuGet packages:
* `Google.Api.CommonProtos` - to provide the proto implementations used by the richer error model
Copy link
Contributor

Choose a reason for hiding this comment

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

as discussed offline, if some of the C# gets added to Google.Api.CommonProtos as extension methods, let's pay close attention to in which namespace these extension methods live, since we don't want to require adding too many "using x.y.z" statements to be able to use the Grpc.StatusProto functionality (in order for the extension methods to become visible).

* `Grpc.Core.Api` - for API classes such as `RpcException`

## Error handling in gRPC

The standard way for gRPC to report the success or failure of a gRPC call is for a
status code to be returned. If a call completes successfully the server returns an `OK`
status to the client, otherwise an error status code is returned. This is known as the
status to the client, otherwise an error status code is returned with an optional string
error message that provides further details about what happened. This is known as the
_standard error model_ and is the official gRPC error model supported by all gRPC
implementations.

Expand All @@ -22,38 +26,36 @@ messages, and a
is defined to cover most needs. The protobuf binary encoding of this extra error
information is provided as trailing metadata in the response.

Not all languages currently have support for this richer error model. It is already
supported in the C++, Go, Java, Python, and Ruby libraries. This NuGet package adds
support for C# and .NET.

For more information on the richer error model see the
[gRPC documentation on error handling](https://grpc.io/docs/guides/error/),
and the [Google APIs overview of the error model](https://cloud.google.com/apis/design/errors#error_model).

## .NET implementation of the richer error model

The error model is define by the protocol buffers files [status.proto](https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto)
and [error_details.proto](https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto)
and their implementations in classes provided in the `Google.Api.CommonProtos` NuGet package.
and [error_details.proto](https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto),
and the `Google.Api.CommonProtos` NuGet package that provides the generated .NET classes
from these proto files.

The error is encapsulated by an instance of `Google.Rpc.Status` and
returned in the trailing response metadata. Setting and reading this metadata is handled
for you when using the extension methods provided in this package.
returned in the trailing response metadata with well-known key `grpc-status-details-bin`.
Setting and reading this metadata is handled
for you when using the methods provided in this package.

## Server Side

The server side uses C#'s Object and Collection initializer syntax.

There are two ways that the server can return additional error information:
- by throwing an `RpcException` that contains the details of the error.
- by setting the `Status` and `ResponseTrailers` in the `ServerCallContext`
- by setting the `Status` and adding to the `ResponseTrailers` in the `ServerCallContext`
before returning from the call. See the examples below.

There are examples of both methods below. In a .NET client the result will always be
an exception received by the client no matter which of the above methods the server
implements.

The `Google.Rpc.Status` can be created and initialized using C#'s Object and Collection initializer syntax. To add messages to the `Details` repeated field, wrap each one in `Any.Pack()` - see example below.
To add messages to the `Details` repeated field in `Google.Rpc.Status`, wrap each one in `Any.Pack()` - see example below.

The `Google.Rpc.Status` extension method `ToRpcException` creates the appropriate `RpcException` from the status.

Expand Down Expand Up @@ -113,10 +115,42 @@ But users can use a different domain of values if they want and and as long as t
services are mutually compatible, things will work fine.

In the richer error model the `RpcException` will contain both a `Grpc.Core.Status` (for the
standard error mode) and a `Google.Rpc.Status` (for the richer error model), each with their
standard error model) and a `Google.Rpc.Status` (for the richer error model), each with their
own status code. While an application is free to set these to different values we recommend
that they are set to the same value to avoid ambiguity.

### Passing stack traces from the server to the client

The richer error model defines a standard way of passing stack traces from the server to the
client. The `DebugInfo` message can be populated with stack traces and then it can
be included in the `Details` of the `Google.Rpc.Status`.

This package includes the extension method `ToRpcDebugInfo` for `System.Exception` to help
create the `DebugInfo` message with the details from the exception.

Example:

```C#
try
{
// ...
}
catch (Exception e)
{
throw new Google.Rpc.Status
{
Code = Google.Rpc.Code.Internal,
Message = "Internal error",
Details =
{
// populate debugInfo from the exception
Any.Pack(e.ToRpcDebugInfo()),
// Add any other messages to the details ...
}
}.ToRpcException();
}
```

## Client Side

There is an extension method to retrieve a `Google.Rpc.Status` from the metadata in
Expand Down
1 change: 1 addition & 0 deletions src/Grpc.StatusProto/RpcExceptionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public static class RpcExceptionExtensions
/// <summary>
/// Retrieves the <see cref="Google.Rpc.Status"/> message containing extended error information
/// from the trailers in an <see cref="RpcException"/>, if present.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <param name="ex">The RPC exception to retrieve details from. Must not be null.</param>
/// <returns>The <see cref="Google.Rpc.Status"/> message specified in the exception, or null
Expand Down
5 changes: 5 additions & 0 deletions src/Grpc.StatusProto/RpcStatusExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ public static class RpcStatusExtensions
/// <summary>
/// Retrieves the error details of type <typeparamref name="T"/> from the <see cref="Google.Rpc.Status"/>
/// message.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <remarks>
/// <example>
Expand Down Expand Up @@ -57,6 +58,7 @@ public static class RpcStatusExtensions

/// <summary>
/// Create a <see cref="RpcException"/> from the <see cref="Google.Rpc.Status"/>
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <remarks>
/// <para>
Expand Down Expand Up @@ -88,6 +90,7 @@ public static RpcException ToRpcException(this Google.Rpc.Status status)

/// <summary>
/// Create a <see cref="RpcException"/> from the <see cref="Google.Rpc.Status"/>
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <remarks>
/// <para>
Expand Down Expand Up @@ -125,6 +128,7 @@ public static RpcException ToRpcException(this Google.Rpc.Status status, StatusC

/// <summary>
/// Iterate over all the messages in the <see cref="Google.Rpc.Status.Details"/>
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <remarks>
/// <para>
Expand Down Expand Up @@ -159,6 +163,7 @@ public static IEnumerable<IMessage> UnpackDetailMessage(this Google.Rpc.Status s
/// <summary>
/// Iterate over all the messages in the <see cref="Google.Rpc.Status.Details"/> that match types
/// in the given <see cref="TypeRegistry"/>
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
/// <remarks>
/// <para>
Expand Down
2 changes: 2 additions & 0 deletions src/Grpc.StatusProto/StandardErrorTypeRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ namespace Grpc.StatusProto;
/// Registry of the <see href="https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto">
/// standard set of error types</see> defined in the richer error model developed and used by Google.
/// These can be sepcified in the <see cref="Google.Rpc.Status.Details"/>.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
public static class StandardErrorTypeRegistry
{
Expand All @@ -42,6 +43,7 @@ public static class StandardErrorTypeRegistry

/// <summary>
/// Get the registry
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
public static TypeRegistry Registry
{
Expand Down
Loading