address review feedback

pull/22891/head
Jan Tattermusch 5 years ago
parent 72c92813a8
commit 2ca781f5a1
  1. 14
      src/csharp/Grpc.Core.Api/Status.cs
  2. 6
      src/csharp/Grpc.Core.Tests/ClientServerTest.cs
  3. 2
      src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs
  4. 4
      src/csharp/Grpc.Core/Internal/CoreErrorDetailException.cs

@ -46,15 +46,16 @@ namespace Grpc.Core
/// Creates a new instance of <c>Status</c>. /// Creates a new instance of <c>Status</c>.
/// Users should not use this constructor, except for creating instances for testing. /// Users should not use this constructor, except for creating instances for testing.
/// The debug error string should only be populated by gRPC internals. /// The debug error string should only be populated by gRPC internals.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary> /// </summary>
/// <param name="statusCode">Status code.</param> /// <param name="statusCode">Status code.</param>
/// <param name="detail">Detail.</param> /// <param name="detail">Detail.</param>
/// <param name="debugErrorException">Optional internal error details.</param> /// <param name="debugException">Optional internal error details.</param>
public Status(StatusCode statusCode, string detail, Exception debugErrorException) public Status(StatusCode statusCode, string detail, Exception debugException)
{ {
StatusCode = statusCode; StatusCode = statusCode;
Detail = detail; Detail = detail;
DebugErrorException = debugErrorException; DebugException = debugException;
} }
/// <summary> /// <summary>
@ -75,17 +76,18 @@ namespace Grpc.Core
/// never rely on values of this field (it should use <c>StatusCode</c> and <c>Detail</c> instead). /// never rely on values of this field (it should use <c>StatusCode</c> and <c>Detail</c> instead).
/// Example: when a client fails to connect to a server, this field may provide additional details /// Example: when a client fails to connect to a server, this field may provide additional details
/// why the connection to the server has failed. /// why the connection to the server has failed.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary> /// </summary>
public Exception DebugErrorException { get; } public Exception DebugException { get; }
/// <summary> /// <summary>
/// Returns a <see cref="System.String"/> that represents the current <see cref="Grpc.Core.Status"/>. /// Returns a <see cref="System.String"/> that represents the current <see cref="Grpc.Core.Status"/>.
/// </summary> /// </summary>
public override string ToString() public override string ToString()
{ {
if (DebugErrorException != null) if (DebugException != null)
{ {
return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\", DebugErrorException=\"{DebugErrorException}\")"; return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\", DebugException=\"{DebugException}\")";
} }
return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\")"; return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\")";
} }

@ -144,18 +144,18 @@ namespace Grpc.Core.Tests
{ {
helper.UnaryHandler = new UnaryServerMethod<string, string>((request, context) => helper.UnaryHandler = new UnaryServerMethod<string, string>((request, context) =>
{ {
context.Status = new Status(StatusCode.Unauthenticated, "", new DebugErrorException("this DebugErrorString value should not be transmitted to the client")); context.Status = new Status(StatusCode.Unauthenticated, "", new CoreErrorDetailException("this DebugErrorString value should not be transmitted to the client"));
return Task.FromResult(""); return Task.FromResult("");
}); });
var ex = Assert.Throws<RpcException>(() => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc")); var ex = Assert.Throws<RpcException>(() => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc"));
Assert.AreEqual(StatusCode.Unauthenticated, ex.Status.StatusCode); Assert.AreEqual(StatusCode.Unauthenticated, ex.Status.StatusCode);
StringAssert.Contains("Error received from peer", ex.Status.DebugErrorException.Message, "Is \"Error received from peer\" still a valid substring to search for in the client-generated error message from C-core?"); StringAssert.Contains("Error received from peer", ex.Status.DebugException.Message, "Is \"Error received from peer\" still a valid substring to search for in the client-generated error message from C-core?");
Assert.AreEqual(0, ex.Trailers.Count); Assert.AreEqual(0, ex.Trailers.Count);
var ex2 = Assert.ThrowsAsync<RpcException>(async () => await Calls.AsyncUnaryCall(helper.CreateUnaryCall(), "abc")); var ex2 = Assert.ThrowsAsync<RpcException>(async () => await Calls.AsyncUnaryCall(helper.CreateUnaryCall(), "abc"));
Assert.AreEqual(StatusCode.Unauthenticated, ex2.Status.StatusCode); Assert.AreEqual(StatusCode.Unauthenticated, ex2.Status.StatusCode);
StringAssert.Contains("Error received from peer", ex2.Status.DebugErrorException.Message, "Is \"Error received from peer\" still a valid substring to search for in the client-generated error message from C-core?"); StringAssert.Contains("Error received from peer", ex2.Status.DebugException.Message, "Is \"Error received from peer\" still a valid substring to search for in the client-generated error message from C-core?");
Assert.AreEqual(0, ex2.Trailers.Count); Assert.AreEqual(0, ex2.Trailers.Count);
} }

@ -93,7 +93,7 @@ namespace Grpc.Core.Internal
IntPtr detailsPtr = Native.grpcsharp_batch_context_recv_status_on_client_details(this, out detailsLength); IntPtr detailsPtr = Native.grpcsharp_batch_context_recv_status_on_client_details(this, out detailsLength);
string details = MarshalUtils.PtrToStringUTF8(detailsPtr, (int)detailsLength.ToUInt32()); string details = MarshalUtils.PtrToStringUTF8(detailsPtr, (int)detailsLength.ToUInt32());
string debugErrorString = Marshal.PtrToStringAnsi(Native.grpcsharp_batch_context_recv_status_on_client_error_string(this)); string debugErrorString = Marshal.PtrToStringAnsi(Native.grpcsharp_batch_context_recv_status_on_client_error_string(this));
var status = new Status(Native.grpcsharp_batch_context_recv_status_on_client_status(this), details, debugErrorString != null ? new DebugErrorException(debugErrorString) : null); var status = new Status(Native.grpcsharp_batch_context_recv_status_on_client_status(this), details, debugErrorString != null ? new CoreErrorDetailException(debugErrorString) : null);
IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this); IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this);
var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr); var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr);

@ -26,9 +26,9 @@ namespace Grpc.Core.Internal
/// <summary> /// <summary>
/// Represents error details provides by C-core's debug_error_string /// Represents error details provides by C-core's debug_error_string
/// </summary> /// </summary>
internal class DebugErrorException : Exception internal class CoreErrorDetailException : Exception
{ {
public DebugErrorException(string message) : base(message) public CoreErrorDetailException(string message) : base(message)
{ {
} }
} }
Loading…
Cancel
Save