Skip to content

Commit

Permalink
address review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jtattermusch committed Jun 3, 2020
1 parent 72c9281 commit 2ca781f
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 12 deletions.
14 changes: 8 additions & 6 deletions src/csharp/Grpc.Core.Api/Status.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,16 @@ public Status(StatusCode statusCode, string detail) : this(statusCode, detail, n
/// Creates a new instance of <c>Status</c>.
/// Users should not use this constructor, except for creating instances for testing.
/// 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>
/// <param name="statusCode">Status code.</param>
/// <param name="detail">Detail.</param>
/// <param name="debugErrorException">Optional internal error details.</param>
public Status(StatusCode statusCode, string detail, Exception debugErrorException)
/// <param name="debugException">Optional internal error details.</param>
public Status(StatusCode statusCode, string detail, Exception debugException)
{
StatusCode = statusCode;
Detail = detail;
DebugErrorException = debugErrorException;
DebugException = debugException;
}

/// <summary>
Expand All @@ -75,17 +76,18 @@ public Status(StatusCode statusCode, string detail, Exception debugErrorExceptio
/// 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
/// why the connection to the server has failed.
/// Note: experimental API that can change or be removed without any prior notice.
/// </summary>
public Exception DebugErrorException { get; }
public Exception DebugException { get; }

/// <summary>
/// Returns a <see cref="System.String"/> that represents the current <see cref="Grpc.Core.Status"/>.
/// </summary>
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}\")";
}
Expand Down
6 changes: 3 additions & 3 deletions src/csharp/Grpc.Core.Tests/ClientServerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,18 @@ public void UnaryCall_StatusDebugErrorStringNotTransmittedFromServer()
{
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("");
});

var ex = Assert.Throws<RpcException>(() => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc"));
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);

var ex2 = Assert.ThrowsAsync<RpcException>(async () => await Calls.AsyncUnaryCall(helper.CreateUnaryCall(), "abc"));
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);
}

Expand Down
2 changes: 1 addition & 1 deletion src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public ClientSideStatus GetReceivedStatusOnClient()
IntPtr detailsPtr = Native.grpcsharp_batch_context_recv_status_on_client_details(this, out detailsLength);
string details = MarshalUtils.PtrToStringUTF8(detailsPtr, (int)detailsLength.ToUInt32());
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);
var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ namespace Grpc.Core.Internal
/// <summary>
/// Represents error details provides by C-core's debug_error_string
/// </summary>
internal class DebugErrorException : Exception
internal class CoreErrorDetailException : Exception
{
public DebugErrorException(string message) : base(message)
public CoreErrorDetailException(string message) : base(message)
{
}
}
Expand Down

0 comments on commit 2ca781f

Please sign in to comment.