Skip to content

Commit

Permalink
fix: Parse error details more robustly
Browse files Browse the repository at this point in the history 
This avoids details being lost if new messages are added to error_details.proto and then used in responses.

Fixes #572.
  • Loading branch information
@jskeet
jskeet committed Sep 9, 2022
1 parent 33fe5db commit 67b3438
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 19 deletions.
40 changes: 40 additions & 0 deletions Google.Api.Gax.Grpc.Tests/Rest/ReadHttpResponseMessageTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,46 @@ public void CreateRpcStatus_NoGrpcStatusCode()
Assert.Equal(expectedStatus, actualStatus);
}

[Fact]
public void CreateRpcStatus_UnknownAny()
{
string json = @"
{
'error': {
'message': 'Some message',
'status': 'ALREADY_EXISTS',
'details': [{
'@type': 'type.googleapis.com/google.rpc.ErrorInfo',
'domain': 'googleapis.com',
'reason': 'It failed',
'metadata': {
'x': 'y'
}
}, {
'@type': 'type.googleapis.com/google.rpc.SnazzyNewMessage',
'step3': 'profit'
}, {
'@type': 'type.googleapis.com/google.rpc.LocalizedMessage',
'locale': 'en-US',
'message': 'It failed badly'
}]
}
}".Replace('\'', '"');
var actualStatus = ReadHttpResponseMessage.CreateRpcStatus(HttpStatusCode.PreconditionFailed, json);
var expectedStatus = new Status
{
Code = (int) gc::StatusCode.AlreadyExists,
Message = "Some message",
Details =
{
Any.Pack(new ErrorInfo { Domain = "googleapis.com", Reason = "It failed", Metadata = { { "x", "y" } } }),
// The middle details are omitted because we can't parse them.
Any.Pack(new LocalizedMessage { Locale = "en-US", Message = "It failed badly" })
}
};
Assert.Equal(expectedStatus, actualStatus);
}

[Fact]
public void GetTrailers_WithStatus()
{
Expand Down
25 changes: 23 additions & 2 deletions Google.Api.Gax.Grpc/Rest/ReadHttpResponseMessage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@

using Google.Protobuf;
using Google.Protobuf.Reflection;
using Google.Protobuf.WellKnownTypes;
using Grpc.Core;
using System;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Runtime.ExceptionServices;
Expand All @@ -31,7 +34,12 @@ namespace Google.Api.Gax.Grpc.Rest
internal class ReadHttpResponseMessage
{
private static readonly JsonParser s_responseMetadataParser = new JsonParser(
JsonParser.Settings.Default.WithIgnoreUnknownFields(true).WithTypeRegistry(TypeRegistry.FromFiles(Rpc.ErrorDetailsReflection.Descriptor)));
JsonParser.Settings.Default.WithIgnoreUnknownFields(true));
private static readonly JsonParser s_anyParser = new JsonParser(
JsonParser.Settings.Default
.WithIgnoreUnknownFields(true).
WithTypeRegistry(TypeRegistry.FromFiles(Rpc.ErrorDetailsReflection.Descriptor)));

private HttpResponseMessage OriginalResponseMessage { get; }

private readonly Rpc.Status _rpcStatus;
Expand Down Expand Up @@ -115,7 +123,7 @@ internal static Rpc.Status CreateRpcStatus(HttpStatusCode statusCode, string con
// is populated or not.)
Code = error.Status_ == Rpc.Code.Ok ? (int) grpcStatusCode : (int) error.Status_,
Message = error.Message,
Details = { error.Details }
Details = { error.Details.Select(TryParseAny).Where(a => a is not null) }
};
return status;
}
Expand All @@ -124,6 +132,19 @@ internal static Rpc.Status CreateRpcStatus(HttpStatusCode statusCode, string con
// If we can't parse the result as JSON, just use the content as the error message.
return new Rpc.Status { Code = (int) grpcStatusCode, Message = content };
}

// Attempts to parse the given Struct as an Any, returning null on failure.
Any TryParseAny(Struct json)
{
try
{
return s_anyParser.Parse<Any>(json.ToString());
}
catch (InvalidOperationException)
{
return null;
}
}
}
}
}
32 changes: 18 additions & 14 deletions Google.Api.Gax.Grpc/Rest/ResponseMetadata.g.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,16 @@ static ResponseMetadataReflection() {
byte[] descriptorData = global::System.Convert.FromBase64String(
string.Concat(
"ChdyZXNwb25zZV9tZXRhZGF0YS5wcm90bxIYZ29vZ2xlLmFwaS5nYXguZ3Jw",
"Yy5yZXN0Ghlnb29nbGUvcHJvdG9idWYvYW55LnByb3RvGhVnb29nbGUvcnBj",
"L2NvZGUucHJvdG8i+wEKBUVycm9yEjUKBWVycm9yGAEgASgLMiYuZ29vZ2xl",
"LmFwaS5nYXguZ3JwYy5yZXN0LkVycm9yLlN0YXR1cxoMCgpFcnJvclByb3Rv",
"GqwBCgZTdGF0dXMSDAoEY29kZRgBIAEoBRIPCgdtZXNzYWdlGAIgASgJEjoK",
"BmVycm9ycxgDIAMoCzIqLmdvb2dsZS5hcGkuZ2F4LmdycGMucmVzdC5FcnJv",
"ci5FcnJvclByb3RvEiAKBnN0YXR1cxgEIAEoDjIQLmdvb2dsZS5ycGMuQ29k",
"ZRIlCgdkZXRhaWxzGAUgAygLMhQuZ29vZ2xlLnByb3RvYnVmLkFueWIGcHJv",
"dG8z"));
"Yy5yZXN0Ghxnb29nbGUvcHJvdG9idWYvc3RydWN0LnByb3RvGhVnb29nbGUv",
"cnBjL2NvZGUucHJvdG8i/gEKBUVycm9yEjUKBWVycm9yGAEgASgLMiYuZ29v",
"Z2xlLmFwaS5nYXguZ3JwYy5yZXN0LkVycm9yLlN0YXR1cxoMCgpFcnJvclBy",
"b3RvGq8BCgZTdGF0dXMSDAoEY29kZRgBIAEoBRIPCgdtZXNzYWdlGAIgASgJ",
"EjoKBmVycm9ycxgDIAMoCzIqLmdvb2dsZS5hcGkuZ2F4LmdycGMucmVzdC5F",
"cnJvci5FcnJvclByb3RvEiAKBnN0YXR1cxgEIAEoDjIQLmdvb2dsZS5ycGMu",
"Q29kZRIoCgdkZXRhaWxzGAUgAygLMhcuZ29vZ2xlLnByb3RvYnVmLlN0cnVj",
"dGIGcHJvdG8z"));
descriptor = pbr::FileDescriptor.FromGeneratedCode(descriptorData,
new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.AnyReflection.Descriptor, global::Google.Rpc.CodeReflection.Descriptor, },
new pbr::FileDescriptor[] { global::Google.Protobuf.WellKnownTypes.StructReflection.Descriptor, global::Google.Rpc.CodeReflection.Descriptor, },
new pbr::GeneratedClrTypeInfo(null, null, new pbr::GeneratedClrTypeInfo[] {
new pbr::GeneratedClrTypeInfo(typeof(global::Google.Api.Gax.Grpc.Rest.Error), global::Google.Api.Gax.Grpc.Rest.Error.Parser, new[]{ "Error_" }, null, null, null, new pbr::GeneratedClrTypeInfo[] { new pbr::GeneratedClrTypeInfo(typeof(global::Google.Api.Gax.Grpc.Rest.Error.Types.ErrorProto), global::Google.Api.Gax.Grpc.Rest.Error.Types.ErrorProto.Parser, null, null, null, null, null),
new pbr::GeneratedClrTypeInfo(typeof(global::Google.Api.Gax.Grpc.Rest.Error.Types.Status), global::Google.Api.Gax.Grpc.Rest.Error.Types.Status.Parser, new[]{ "Code", "Message", "Errors", "Status_", "Details" }, null, null, null, null)})
Expand Down Expand Up @@ -524,15 +524,19 @@ public string Message {

/// <summary>Field number for the "details" field.</summary>
public const int DetailsFieldNumber = 5;
private static readonly pb::FieldCodec<global::Google.Protobuf.WellKnownTypes.Any> _repeated_details_codec
= pb::FieldCodec.ForMessage(42, global::Google.Protobuf.WellKnownTypes.Any.Parser);
private readonly pbc::RepeatedField<global::Google.Protobuf.WellKnownTypes.Any> details_ = new pbc::RepeatedField<global::Google.Protobuf.WellKnownTypes.Any>();
private static readonly pb::FieldCodec<global::Google.Protobuf.WellKnownTypes.Struct> _repeated_details_codec
= pb::FieldCodec.ForMessage(42, global::Google.Protobuf.WellKnownTypes.Struct.Parser);
private readonly pbc::RepeatedField<global::Google.Protobuf.WellKnownTypes.Struct> details_ = new pbc::RepeatedField<global::Google.Protobuf.WellKnownTypes.Struct>();
/// <summary>
/// This corresponds to `google.rpc.Status.details`.
/// This corresponds to `google.rpc.Status.details`, but
/// we represent each element as a Struct instead of an Any so that
/// we can definitely parse the outer JSON. We then convert each struct back into
/// JSON and attempt to parse it as an Any, ignoring values that have type URLs
/// corresponding to messages we're not aware of.
/// </summary>
[global::System.Diagnostics.DebuggerNonUserCodeAttribute]
[global::System.CodeDom.Compiler.GeneratedCode("protoc", null)]
public pbc::RepeatedField<global::Google.Protobuf.WellKnownTypes.Any> Details {
public pbc::RepeatedField<global::Google.Protobuf.WellKnownTypes.Struct> Details {
get { return details_; }
}

Expand Down
13 changes: 10 additions & 3 deletions Google.Api.Gax.Grpc/Rest/response_metadata.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,17 @@ syntax = "proto3";

package google.api.gax.grpc.rest;

import "google/protobuf/any.proto";
import "google/protobuf/struct.proto";
import "google/rpc/code.proto";

// Taken from https://cloud.google.com/apis/design/errors#error_mapping.
// This allows detailed error information to be parsed from HTTP/1.1 responses
// and converted into an appropriate google.rpc.Status. The message names
// aren't ideal, but it's better to just take the proto definition directly
// from the page than to modify it. This is only generated as internal classes.
// Note: there is one modification, which is to change the details field to use
// Struct rather than Any. This avoids parse failures if the packed Any is
// a type we don't know about.

// This message defines the error schema for Google's JSON HTTP APIs.
message Error {
Expand All @@ -34,8 +37,12 @@ message Error {
repeated ErrorProto errors = 3;
// This is the enum version for `google.rpc.Status.code`.
google.rpc.Code status = 4;
// This corresponds to `google.rpc.Status.details`.
repeated google.protobuf.Any details = 5;
// This corresponds to `google.rpc.Status.details`, but
// we represent each element as a Struct instead of an Any so that
// we can definitely parse the outer JSON. We then convert each struct back into
// JSON and attempt to parse it as an Any, ignoring values that have type URLs
// corresponding to messages we're not aware of.
repeated google.protobuf.Struct details = 5;
}
// The actual error payload. The nested message structure is for backward
// compatibility with [Google API Client
Expand Down

0 comments on commit 67b3438

Please sign in to comment.