From 0ac75d7a199b79e3e31c68c4162d2ba260a7cb42 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 7 May 2020 10:57:29 +0200 Subject: [PATCH 1/8] add grpcsharp_batch_context_recv_status_on_client_error_string native method --- .../Internal/NativeMethods.Generated.cs | 11 +++++++++++ src/csharp/ext/grpc_csharp_ext.c | 17 +++++++++++++++++ .../runtimes/grpc_csharp_ext_dummy_stubs.c | 4 ++++ .../Grpc.Core/Internal/native_methods.include | 1 + 4 files changed, 33 insertions(+) diff --git a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs index c724b30ca8f..8b60268679d 100644 --- a/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs +++ b/src/csharp/Grpc.Core/Internal/NativeMethods.Generated.cs @@ -43,6 +43,7 @@ namespace Grpc.Core.Internal public readonly Delegates.grpcsharp_batch_context_recv_message_next_slice_peek_delegate grpcsharp_batch_context_recv_message_next_slice_peek; public readonly Delegates.grpcsharp_batch_context_recv_status_on_client_status_delegate grpcsharp_batch_context_recv_status_on_client_status; public readonly Delegates.grpcsharp_batch_context_recv_status_on_client_details_delegate grpcsharp_batch_context_recv_status_on_client_details; + public readonly Delegates.grpcsharp_batch_context_recv_status_on_client_error_string_delegate grpcsharp_batch_context_recv_status_on_client_error_string; public readonly Delegates.grpcsharp_batch_context_recv_status_on_client_trailing_metadata_delegate grpcsharp_batch_context_recv_status_on_client_trailing_metadata; public readonly Delegates.grpcsharp_batch_context_recv_close_on_server_cancelled_delegate grpcsharp_batch_context_recv_close_on_server_cancelled; public readonly Delegates.grpcsharp_batch_context_reset_delegate grpcsharp_batch_context_reset; @@ -151,6 +152,7 @@ namespace Grpc.Core.Internal this.grpcsharp_batch_context_recv_message_next_slice_peek = GetMethodDelegate(library); this.grpcsharp_batch_context_recv_status_on_client_status = GetMethodDelegate(library); this.grpcsharp_batch_context_recv_status_on_client_details = GetMethodDelegate(library); + this.grpcsharp_batch_context_recv_status_on_client_error_string = GetMethodDelegate(library); this.grpcsharp_batch_context_recv_status_on_client_trailing_metadata = GetMethodDelegate(library); this.grpcsharp_batch_context_recv_close_on_server_cancelled = GetMethodDelegate(library); this.grpcsharp_batch_context_reset = GetMethodDelegate(library); @@ -258,6 +260,7 @@ namespace Grpc.Core.Internal this.grpcsharp_batch_context_recv_message_next_slice_peek = DllImportsFromStaticLib.grpcsharp_batch_context_recv_message_next_slice_peek; this.grpcsharp_batch_context_recv_status_on_client_status = DllImportsFromStaticLib.grpcsharp_batch_context_recv_status_on_client_status; this.grpcsharp_batch_context_recv_status_on_client_details = DllImportsFromStaticLib.grpcsharp_batch_context_recv_status_on_client_details; + this.grpcsharp_batch_context_recv_status_on_client_error_string = DllImportsFromStaticLib.grpcsharp_batch_context_recv_status_on_client_error_string; this.grpcsharp_batch_context_recv_status_on_client_trailing_metadata = DllImportsFromStaticLib.grpcsharp_batch_context_recv_status_on_client_trailing_metadata; this.grpcsharp_batch_context_recv_close_on_server_cancelled = DllImportsFromStaticLib.grpcsharp_batch_context_recv_close_on_server_cancelled; this.grpcsharp_batch_context_reset = DllImportsFromStaticLib.grpcsharp_batch_context_reset; @@ -365,6 +368,7 @@ namespace Grpc.Core.Internal this.grpcsharp_batch_context_recv_message_next_slice_peek = DllImportsFromSharedLib.grpcsharp_batch_context_recv_message_next_slice_peek; this.grpcsharp_batch_context_recv_status_on_client_status = DllImportsFromSharedLib.grpcsharp_batch_context_recv_status_on_client_status; this.grpcsharp_batch_context_recv_status_on_client_details = DllImportsFromSharedLib.grpcsharp_batch_context_recv_status_on_client_details; + this.grpcsharp_batch_context_recv_status_on_client_error_string = DllImportsFromSharedLib.grpcsharp_batch_context_recv_status_on_client_error_string; this.grpcsharp_batch_context_recv_status_on_client_trailing_metadata = DllImportsFromSharedLib.grpcsharp_batch_context_recv_status_on_client_trailing_metadata; this.grpcsharp_batch_context_recv_close_on_server_cancelled = DllImportsFromSharedLib.grpcsharp_batch_context_recv_close_on_server_cancelled; this.grpcsharp_batch_context_reset = DllImportsFromSharedLib.grpcsharp_batch_context_reset; @@ -475,6 +479,7 @@ namespace Grpc.Core.Internal public delegate int grpcsharp_batch_context_recv_message_next_slice_peek_delegate(BatchContextSafeHandle ctx, out UIntPtr sliceLen, out IntPtr sliceDataPtr); public delegate StatusCode grpcsharp_batch_context_recv_status_on_client_status_delegate(BatchContextSafeHandle ctx); public delegate IntPtr grpcsharp_batch_context_recv_status_on_client_details_delegate(BatchContextSafeHandle ctx, out UIntPtr detailsLength); + public delegate IntPtr grpcsharp_batch_context_recv_status_on_client_error_string_delegate(BatchContextSafeHandle ctx); public delegate IntPtr grpcsharp_batch_context_recv_status_on_client_trailing_metadata_delegate(BatchContextSafeHandle ctx); public delegate int grpcsharp_batch_context_recv_close_on_server_cancelled_delegate(BatchContextSafeHandle ctx); public delegate void grpcsharp_batch_context_reset_delegate(BatchContextSafeHandle ctx); @@ -605,6 +610,9 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern IntPtr grpcsharp_batch_context_recv_status_on_client_details(BatchContextSafeHandle ctx, out UIntPtr detailsLength); + [DllImport(ImportName)] + public static extern IntPtr grpcsharp_batch_context_recv_status_on_client_error_string(BatchContextSafeHandle ctx); + [DllImport(ImportName)] public static extern IntPtr grpcsharp_batch_context_recv_status_on_client_trailing_metadata(BatchContextSafeHandle ctx); @@ -922,6 +930,9 @@ namespace Grpc.Core.Internal [DllImport(ImportName)] public static extern IntPtr grpcsharp_batch_context_recv_status_on_client_details(BatchContextSafeHandle ctx, out UIntPtr detailsLength); + [DllImport(ImportName)] + public static extern IntPtr grpcsharp_batch_context_recv_status_on_client_error_string(BatchContextSafeHandle ctx); + [DllImport(ImportName)] public static extern IntPtr grpcsharp_batch_context_recv_status_on_client_trailing_metadata(BatchContextSafeHandle ctx); diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index e09ad694328..a48d29af294 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -69,6 +69,7 @@ typedef struct grpcsharp_batch_context { grpc_metadata_array trailing_metadata; grpc_status_code status; grpc_slice status_details; + const char* error_string; } recv_status_on_client; int recv_close_on_server_cancelled; @@ -223,6 +224,7 @@ grpcsharp_batch_context_reset(grpcsharp_batch_context* ctx) { grpcsharp_metadata_array_destroy_metadata_only( &(ctx->recv_status_on_client.trailing_metadata)); grpc_slice_unref(ctx->recv_status_on_client.status_details); + gpr_free(ctx->recv_status_on_client.error_string); memset(ctx, 0, sizeof(grpcsharp_batch_context)); } @@ -328,6 +330,12 @@ grpcsharp_batch_context_recv_status_on_client_details( return (char*)GRPC_SLICE_START_PTR(ctx->recv_status_on_client.status_details); } +GPR_EXPORT const char* GPR_CALLTYPE +grpcsharp_batch_context_recv_status_on_client_error_string( + const grpcsharp_batch_context* ctx) { + return ctx->recv_status_on_client.error_string; +} + GPR_EXPORT const grpc_metadata_array* GPR_CALLTYPE grpcsharp_batch_context_recv_status_on_client_trailing_metadata( const grpcsharp_batch_context* ctx) { @@ -631,6 +639,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_unary( &(ctx->recv_status_on_client.status); ops[5].data.recv_status_on_client.status_details = &(ctx->recv_status_on_client.status_details); + ops[5].data.recv_status_on_client.error_string = + &(ctx->recv_status_on_client.error_string); ops[5].flags = 0; ops[5].reserved = NULL; @@ -652,6 +662,7 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_test_call_start_unary_echo( // received from server. ctx->recv_status_on_client.status = GRPC_STATUS_OK; ctx->recv_status_on_client.status_details = grpc_empty_slice(); + ctx->recv_status_on_client.error_string = NULL; // echo initial metadata as if received from server (as trailing metadata) grpcsharp_metadata_array_move(&(ctx->recv_status_on_client.trailing_metadata), initial_metadata); @@ -691,6 +702,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_client_streaming( &(ctx->recv_status_on_client.status); ops[3].data.recv_status_on_client.status_details = &(ctx->recv_status_on_client.status_details); + ops[3].data.recv_status_on_client.error_string = + &(ctx->recv_status_on_client.error_string); ops[3].flags = 0; ops[3].reserved = NULL; @@ -732,6 +745,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_server_streaming( &(ctx->recv_status_on_client.status); ops[3].data.recv_status_on_client.status_details = &(ctx->recv_status_on_client.status_details); + ops[3].data.recv_status_on_client.error_string = + &(ctx->recv_status_on_client.error_string); ops[3].flags = 0; ops[3].reserved = NULL; @@ -761,6 +776,8 @@ GPR_EXPORT grpc_call_error GPR_CALLTYPE grpcsharp_call_start_duplex_streaming( &(ctx->recv_status_on_client.status); ops[1].data.recv_status_on_client.status_details = &(ctx->recv_status_on_client.status_details); + ops[1].data.recv_status_on_client.error_string = + &(ctx->recv_status_on_client.error_string); ops[1].flags = 0; ops[1].reserved = NULL; diff --git a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c index 58a7c58e91a..11097e613d8 100644 --- a/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c +++ b/src/csharp/unitypackage/unitypackage_skeleton/Plugins/Grpc.Core/runtimes/grpc_csharp_ext_dummy_stubs.c @@ -58,6 +58,10 @@ void grpcsharp_batch_context_recv_status_on_client_details() { fprintf(stderr, "Should never reach here"); abort(); } +void grpcsharp_batch_context_recv_status_on_client_error_string() { + fprintf(stderr, "Should never reach here"); + abort(); +} void grpcsharp_batch_context_recv_status_on_client_trailing_metadata() { fprintf(stderr, "Should never reach here"); abort(); diff --git a/templates/src/csharp/Grpc.Core/Internal/native_methods.include b/templates/src/csharp/Grpc.Core/Internal/native_methods.include index f2b3a165a3b..e6fe0a2dd5e 100644 --- a/templates/src/csharp/Grpc.Core/Internal/native_methods.include +++ b/templates/src/csharp/Grpc.Core/Internal/native_methods.include @@ -9,6 +9,7 @@ native_method_signatures = [ 'int grpcsharp_batch_context_recv_message_next_slice_peek(BatchContextSafeHandle ctx, out UIntPtr sliceLen, out IntPtr sliceDataPtr)', 'StatusCode grpcsharp_batch_context_recv_status_on_client_status(BatchContextSafeHandle ctx)', 'IntPtr grpcsharp_batch_context_recv_status_on_client_details(BatchContextSafeHandle ctx, out UIntPtr detailsLength)', + 'IntPtr grpcsharp_batch_context_recv_status_on_client_error_string(BatchContextSafeHandle ctx)', 'IntPtr grpcsharp_batch_context_recv_status_on_client_trailing_metadata(BatchContextSafeHandle ctx)', 'int grpcsharp_batch_context_recv_close_on_server_cancelled(BatchContextSafeHandle ctx)', 'void grpcsharp_batch_context_reset(BatchContextSafeHandle ctx)', From f2178a136d2488dc4600faacecd128f69bc3fbbc Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 7 May 2020 11:20:57 +0200 Subject: [PATCH 2/8] populate error string in ClientSideStatus --- .../Grpc.Core.Tests/Internal/AsyncCallTest.cs | 30 ++++++++-------- src/csharp/Grpc.Core/Internal/AsyncCall.cs | 2 +- .../Internal/BatchContextSafeHandle.cs | 4 ++- .../Grpc.Core/Internal/ClientSideStatus.cs | 35 ++++++------------- 4 files changed, 29 insertions(+), 42 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs index 78c7f3ad5bb..49ec2636db6 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs @@ -78,7 +78,7 @@ namespace Grpc.Core.Internal.Tests { var resultTask = asyncCall.UnaryCallAsync("request1"); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -102,7 +102,7 @@ namespace Grpc.Core.Internal.Tests { var resultTask = asyncCall.UnaryCallAsync("request1"); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), null, new Metadata()); @@ -159,7 +159,7 @@ namespace Grpc.Core.Internal.Tests { var resultTask = asyncCall.ClientStreamingCallAsync(); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -197,7 +197,7 @@ namespace Grpc.Core.Internal.Tests completeTask.Wait(); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -283,7 +283,7 @@ namespace Grpc.Core.Internal.Tests var requestStream = new ClientRequestStream(asyncCall); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -301,7 +301,7 @@ namespace Grpc.Core.Internal.Tests var requestStream = new ClientRequestStream(asyncCall); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(new Status(StatusCode.OutOfRange, ""), new Metadata()), + new ClientSideStatus(new Status(StatusCode.OutOfRange, ""), new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -325,7 +325,7 @@ namespace Grpc.Core.Internal.Tests fakeCall.SendCompletionCallback.OnSendCompletion(true); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -339,7 +339,7 @@ namespace Grpc.Core.Internal.Tests var requestStream = new ClientRequestStream(asyncCall); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata()), + new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), CreateResponsePayload(), new Metadata()); @@ -395,7 +395,7 @@ namespace Grpc.Core.Internal.Tests Assert.AreEqual(0, asyncCall.ResponseHeadersAsync.Result.Count); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); } @@ -408,7 +408,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); // try alternative order of completions - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); @@ -444,7 +444,7 @@ namespace Grpc.Core.Internal.Tests Assert.AreEqual("response1", responseStream.Current); var readTask3 = responseStream.MoveNext(); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask3); @@ -484,7 +484,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); } @@ -498,7 +498,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); @@ -516,7 +516,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); @@ -639,7 +639,7 @@ namespace Grpc.Core.Internal.Tests ClientSideStatus CreateClientSideStatus(StatusCode statusCode) { - return new ClientSideStatus(new Status(statusCode, ""), new Metadata()); + return new ClientSideStatus(new Status(statusCode, ""), new Metadata(), null); } IBufferReader CreateResponsePayload() diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index 6a7466ac278..37cf86016e1 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -553,7 +553,7 @@ namespace Grpc.Core.Internal if (deserializeException != null && receivedStatus.Status.StatusCode == StatusCode.OK) { - receivedStatus = new ClientSideStatus(DeserializeResponseFailureStatus, receivedStatus.Trailers); + receivedStatus = new ClientSideStatus(DeserializeResponseFailureStatus, receivedStatus.Trailers, receivedStatus.Error); } finishedStatus = receivedStatus; diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs index 50a626842dd..9d0e9ce41eb 100644 --- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs @@ -97,7 +97,9 @@ namespace Grpc.Core.Internal IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this); var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr); - return new ClientSideStatus(status, metadata); + string error = Marshal.PtrToStringAnsi(Native.grpcsharp_batch_context_recv_status_on_client_error_string(this)); + + return new ClientSideStatus(status, metadata, error); } public IBufferReader GetReceivedMessageReader() diff --git a/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs b/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs index d19c6538ba4..893849279ed 100644 --- a/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs +++ b/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs @@ -16,40 +16,25 @@ #endregion -using System; -using Grpc.Core; - namespace Grpc.Core.Internal { /// - /// Status + metadata received on client side when call finishes. + /// Status + metadata + error received on client side when call finishes. /// (when receive_status_on_client operation finishes). /// - internal struct ClientSideStatus + internal readonly struct ClientSideStatus { - readonly Status status; - readonly Metadata trailers; - - public ClientSideStatus(Status status, Metadata trailers) + public ClientSideStatus(Status status, Metadata trailers, string error) { - this.status = status; - this.trailers = trailers; + Status = status; + Trailers = trailers; + Error = error; } - public Status Status - { - get - { - return this.status; - } - } + public Status Status { get; } - public Metadata Trailers - { - get - { - return this.trailers; - } - } + public Metadata Trailers { get; } + + public string Error { get; } } } From cfe0b3a3275651391860a670723d9ef6a867a18d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 7 May 2020 12:00:46 +0200 Subject: [PATCH 3/8] Revert "populate error string in ClientSideStatus" This reverts commit f2178a136d2488dc4600faacecd128f69bc3fbbc. --- .../Grpc.Core.Tests/Internal/AsyncCallTest.cs | 30 ++++++++-------- src/csharp/Grpc.Core/Internal/AsyncCall.cs | 2 +- .../Internal/BatchContextSafeHandle.cs | 4 +-- .../Grpc.Core/Internal/ClientSideStatus.cs | 35 +++++++++++++------ 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs index 49ec2636db6..78c7f3ad5bb 100644 --- a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs +++ b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs @@ -78,7 +78,7 @@ namespace Grpc.Core.Internal.Tests { var resultTask = asyncCall.UnaryCallAsync("request1"); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), CreateResponsePayload(), new Metadata()); @@ -102,7 +102,7 @@ namespace Grpc.Core.Internal.Tests { var resultTask = asyncCall.UnaryCallAsync("request1"); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), null, new Metadata()); @@ -159,7 +159,7 @@ namespace Grpc.Core.Internal.Tests { var resultTask = asyncCall.ClientStreamingCallAsync(); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), CreateResponsePayload(), new Metadata()); @@ -197,7 +197,7 @@ namespace Grpc.Core.Internal.Tests completeTask.Wait(); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), CreateResponsePayload(), new Metadata()); @@ -283,7 +283,7 @@ namespace Grpc.Core.Internal.Tests var requestStream = new ClientRequestStream(asyncCall); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), CreateResponsePayload(), new Metadata()); @@ -301,7 +301,7 @@ namespace Grpc.Core.Internal.Tests var requestStream = new ClientRequestStream(asyncCall); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(new Status(StatusCode.OutOfRange, ""), new Metadata(), null), + new ClientSideStatus(new Status(StatusCode.OutOfRange, ""), new Metadata()), CreateResponsePayload(), new Metadata()); @@ -325,7 +325,7 @@ namespace Grpc.Core.Internal.Tests fakeCall.SendCompletionCallback.OnSendCompletion(true); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), CreateResponsePayload(), new Metadata()); @@ -339,7 +339,7 @@ namespace Grpc.Core.Internal.Tests var requestStream = new ClientRequestStream(asyncCall); fakeCall.UnaryResponseClientCallback.OnUnaryResponseClient(true, - new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null), + new ClientSideStatus(Status.DefaultSuccess, new Metadata()), CreateResponsePayload(), new Metadata()); @@ -395,7 +395,7 @@ namespace Grpc.Core.Internal.Tests Assert.AreEqual(0, asyncCall.ResponseHeadersAsync.Result.Count); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); } @@ -408,7 +408,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); // try alternative order of completions - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); @@ -444,7 +444,7 @@ namespace Grpc.Core.Internal.Tests Assert.AreEqual("response1", responseStream.Current); var readTask3 = responseStream.MoveNext(); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask3); @@ -484,7 +484,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); } @@ -498,7 +498,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); @@ -516,7 +516,7 @@ namespace Grpc.Core.Internal.Tests var readTask = responseStream.MoveNext(); fakeCall.ReceivedMessageCallback.OnReceivedMessage(true, CreateNullResponse()); - fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata(), null)); + fakeCall.ReceivedStatusOnClientCallback.OnReceivedStatusOnClient(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata())); AssertStreamingResponseSuccess(asyncCall, fakeCall, readTask); @@ -639,7 +639,7 @@ namespace Grpc.Core.Internal.Tests ClientSideStatus CreateClientSideStatus(StatusCode statusCode) { - return new ClientSideStatus(new Status(statusCode, ""), new Metadata(), null); + return new ClientSideStatus(new Status(statusCode, ""), new Metadata()); } IBufferReader CreateResponsePayload() diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs index 37cf86016e1..6a7466ac278 100644 --- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs +++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs @@ -553,7 +553,7 @@ namespace Grpc.Core.Internal if (deserializeException != null && receivedStatus.Status.StatusCode == StatusCode.OK) { - receivedStatus = new ClientSideStatus(DeserializeResponseFailureStatus, receivedStatus.Trailers, receivedStatus.Error); + receivedStatus = new ClientSideStatus(DeserializeResponseFailureStatus, receivedStatus.Trailers); } finishedStatus = receivedStatus; diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs index 9d0e9ce41eb..50a626842dd 100644 --- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs @@ -97,9 +97,7 @@ namespace Grpc.Core.Internal IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this); var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr); - string error = Marshal.PtrToStringAnsi(Native.grpcsharp_batch_context_recv_status_on_client_error_string(this)); - - return new ClientSideStatus(status, metadata, error); + return new ClientSideStatus(status, metadata); } public IBufferReader GetReceivedMessageReader() diff --git a/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs b/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs index 893849279ed..d19c6538ba4 100644 --- a/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs +++ b/src/csharp/Grpc.Core/Internal/ClientSideStatus.cs @@ -16,25 +16,40 @@ #endregion +using System; +using Grpc.Core; + namespace Grpc.Core.Internal { /// - /// Status + metadata + error received on client side when call finishes. + /// Status + metadata received on client side when call finishes. /// (when receive_status_on_client operation finishes). /// - internal readonly struct ClientSideStatus + internal struct ClientSideStatus { - public ClientSideStatus(Status status, Metadata trailers, string error) + readonly Status status; + readonly Metadata trailers; + + public ClientSideStatus(Status status, Metadata trailers) { - Status = status; - Trailers = trailers; - Error = error; + this.status = status; + this.trailers = trailers; } - public Status Status { get; } + public Status Status + { + get + { + return this.status; + } + } - public Metadata Trailers { get; } - - public string Error { get; } + public Metadata Trailers + { + get + { + return this.trailers; + } + } } } From 1cbd0d4c2a17af695329c335785918e64d5ead0d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 7 May 2020 12:25:12 +0200 Subject: [PATCH 4/8] populate Status.DebugErrorString in C# --- src/csharp/Grpc.Core.Api/Status.cs | 51 +++++++++++-------- .../Internal/BatchContextSafeHandle.cs | 3 +- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/Status.cs b/src/csharp/Grpc.Core.Api/Status.cs index b1a030b2d1f..5c3ff18e22e 100644 --- a/src/csharp/Grpc.Core.Api/Status.cs +++ b/src/csharp/Grpc.Core.Api/Status.cs @@ -31,48 +31,59 @@ namespace Grpc.Core /// public static readonly Status DefaultCancelled = new Status(StatusCode.Cancelled, ""); - readonly StatusCode statusCode; - readonly string detail; + /// + /// Creates a new instance of Status. + /// + /// Status code. + /// Detail. + public Status(StatusCode statusCode, string detail) : this(statusCode, detail, null) + { + } /// /// Creates a new instance of Status. /// /// Status code. /// Detail. - public Status(StatusCode statusCode, string detail) + /// Optional internal error string. + public Status(StatusCode statusCode, string detail, string debugErrorString) { - this.statusCode = statusCode; - this.detail = detail; + StatusCode = statusCode; + Detail = detail; + DebugErrorString = debugErrorString; } /// /// Gets the gRPC status code. OK indicates success, all other values indicate an error. /// - public StatusCode StatusCode - { - get - { - return statusCode; - } - } + public StatusCode StatusCode { get; } /// /// Gets the detail. /// - public string Detail - { - get - { - return detail; - } - } + public string Detail { get; } + + /// + /// In case of an error, this field may contain additional error details to help with debugging. + /// This field will be only populated on a client and its value is generated locally, + /// based on the internal state of the gRPC client stack (i.e. the value is never sent over the wire). + /// Note that this field is available only for debugging purposes, the application logic should + /// never rely on values of this field (it should should StatusCode and Detail 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. + /// + public string DebugErrorString { get; } /// /// Returns a that represents the current . /// public override string ToString() { - return string.Format("Status(StatusCode={0}, Detail=\"{1}\")", statusCode, detail); + if (DebugErrorString != null) + { + return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\", DebugErrorString=\"{DebugErrorString}\")"; + } + return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\")"; } } } diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs index 50a626842dd..d0f5c100be0 100644 --- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs @@ -92,7 +92,8 @@ namespace Grpc.Core.Internal UIntPtr detailsLength; IntPtr detailsPtr = Native.grpcsharp_batch_context_recv_status_on_client_details(this, out detailsLength); string details = MarshalUtils.PtrToStringUTF8(detailsPtr, (int)detailsLength.ToUInt32()); - var status = new Status(Native.grpcsharp_batch_context_recv_status_on_client_status(this), details); + string error = 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, error); IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this); var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr); From fa99395610f7c22db441eddd4d8e16d812d9735d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 13 May 2020 12:29:33 +0200 Subject: [PATCH 5/8] csharp debug error string improvements --- src/csharp/Grpc.Core.Api/Status.cs | 2 ++ .../Grpc.Core.Tests/ClientServerTest.cs | 20 +++++++++++++++++++ .../Internal/BatchContextSafeHandle.cs | 4 ++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core.Api/Status.cs b/src/csharp/Grpc.Core.Api/Status.cs index 5c3ff18e22e..72d9e5371b8 100644 --- a/src/csharp/Grpc.Core.Api/Status.cs +++ b/src/csharp/Grpc.Core.Api/Status.cs @@ -42,6 +42,8 @@ namespace Grpc.Core /// /// Creates a new instance of Status. + /// Users should not use this constructor, except for creating instances for testing. + /// The debug error string should only be populated by gRPC internals. /// /// Status code. /// Detail. diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs index 331c3321e14..d06c55040be 100644 --- a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs @@ -139,6 +139,26 @@ namespace Grpc.Core.Tests Assert.AreEqual(0, ex2.Trailers.Count); } + [Test] + public void UnaryCall_StatusDebugErrorStringNotTransmittedFromServer() + { + helper.UnaryHandler = new UnaryServerMethod((request, context) => + { + context.Status = new Status(StatusCode.Unauthenticated, "", "this DebugErrorString value should not be transmitted to the client"); + return Task.FromResult(""); + }); + + var ex = Assert.Throws(() => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc")); + Assert.AreEqual(StatusCode.Unauthenticated, ex.Status.StatusCode); + Assert.IsTrue(ex.Status.DebugErrorString.Contains("Error received from peer")); // a different debug error string set by grpc client + Assert.AreEqual(0, ex.Trailers.Count); + + var ex2 = Assert.ThrowsAsync(async () => await Calls.AsyncUnaryCall(helper.CreateUnaryCall(), "abc")); + Assert.AreEqual(StatusCode.Unauthenticated, ex2.Status.StatusCode); + Assert.IsTrue(ex2.Status.DebugErrorString.Contains("Error received from peer")); // a different debug error string set by grpc client + Assert.AreEqual(0, ex2.Trailers.Count); + } + [Test] public void UnaryCall_ServerHandlerSetsStatusAndTrailers() { diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs index d0f5c100be0..a8470af549e 100644 --- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs @@ -92,8 +92,8 @@ namespace Grpc.Core.Internal UIntPtr detailsLength; IntPtr detailsPtr = Native.grpcsharp_batch_context_recv_status_on_client_details(this, out detailsLength); string details = MarshalUtils.PtrToStringUTF8(detailsPtr, (int)detailsLength.ToUInt32()); - string error = 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, error); + 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); IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this); var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr); From 860d6e5e26fa8fa573c2378571a297f691c37b3f Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 14 May 2020 09:27:43 +0200 Subject: [PATCH 6/8] improve asserts --- src/csharp/Grpc.Core.Tests/ClientServerTest.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs index d06c55040be..a12ed31e95d 100644 --- a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs @@ -150,12 +150,12 @@ namespace Grpc.Core.Tests var ex = Assert.Throws(() => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc")); Assert.AreEqual(StatusCode.Unauthenticated, ex.Status.StatusCode); - Assert.IsTrue(ex.Status.DebugErrorString.Contains("Error received from peer")); // a different debug error string set by grpc client + StringAssert.Contains("Error received from peer", ex.Status.DebugErrorString, "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(async () => await Calls.AsyncUnaryCall(helper.CreateUnaryCall(), "abc")); Assert.AreEqual(StatusCode.Unauthenticated, ex2.Status.StatusCode); - Assert.IsTrue(ex2.Status.DebugErrorString.Contains("Error received from peer")); // a different debug error string set by grpc client + StringAssert.Contains("Error received from peer", ex2.Status.DebugErrorString, "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); } From 72c92813a8dbfc56055744164675037a48ace65e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 2 Jun 2020 21:29:32 +0200 Subject: [PATCH 7/8] alternative API: expose debug error in form of an exception --- src/csharp/Grpc.Core.Api/Status.cs | 16 +++++---- .../Grpc.Core.Tests/ClientServerTest.cs | 6 ++-- .../Internal/BatchContextSafeHandle.cs | 2 +- .../Grpc.Core/Internal/DebugErrorException.cs | 35 +++++++++++++++++++ 4 files changed, 48 insertions(+), 11 deletions(-) create mode 100644 src/csharp/Grpc.Core/Internal/DebugErrorException.cs diff --git a/src/csharp/Grpc.Core.Api/Status.cs b/src/csharp/Grpc.Core.Api/Status.cs index 72d9e5371b8..32f79f74cc1 100644 --- a/src/csharp/Grpc.Core.Api/Status.cs +++ b/src/csharp/Grpc.Core.Api/Status.cs @@ -14,6 +14,8 @@ // limitations under the License. #endregion +using System; + namespace Grpc.Core { /// @@ -47,12 +49,12 @@ namespace Grpc.Core /// /// Status code. /// Detail. - /// Optional internal error string. - public Status(StatusCode statusCode, string detail, string debugErrorString) + /// Optional internal error details. + public Status(StatusCode statusCode, string detail, Exception debugErrorException) { StatusCode = statusCode; Detail = detail; - DebugErrorString = debugErrorString; + DebugErrorException = debugErrorException; } /// @@ -70,20 +72,20 @@ namespace Grpc.Core /// This field will be only populated on a client and its value is generated locally, /// based on the internal state of the gRPC client stack (i.e. the value is never sent over the wire). /// Note that this field is available only for debugging purposes, the application logic should - /// never rely on values of this field (it should should StatusCode and Detail instead). + /// never rely on values of this field (it should use StatusCode and Detail 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. /// - public string DebugErrorString { get; } + public Exception DebugErrorException { get; } /// /// Returns a that represents the current . /// public override string ToString() { - if (DebugErrorString != null) + if (DebugErrorException != null) { - return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\", DebugErrorString=\"{DebugErrorString}\")"; + return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\", DebugErrorException=\"{DebugErrorException}\")"; } return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\")"; } diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs index a12ed31e95d..7072851732a 100644 --- a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs @@ -144,18 +144,18 @@ namespace Grpc.Core.Tests { helper.UnaryHandler = new UnaryServerMethod((request, context) => { - context.Status = new Status(StatusCode.Unauthenticated, "", "this DebugErrorString value should not be transmitted to the client"); + context.Status = new Status(StatusCode.Unauthenticated, "", new DebugErrorException("this DebugErrorString value should not be transmitted to the client")); return Task.FromResult(""); }); var ex = Assert.Throws(() => Calls.BlockingUnaryCall(helper.CreateUnaryCall(), "abc")); Assert.AreEqual(StatusCode.Unauthenticated, ex.Status.StatusCode); - StringAssert.Contains("Error received from peer", ex.Status.DebugErrorString, "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.DebugErrorException.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(async () => await Calls.AsyncUnaryCall(helper.CreateUnaryCall(), "abc")); Assert.AreEqual(StatusCode.Unauthenticated, ex2.Status.StatusCode); - StringAssert.Contains("Error received from peer", ex2.Status.DebugErrorString, "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.DebugErrorException.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); } diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs index a8470af549e..e00f153c21f 100644 --- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs @@ -93,7 +93,7 @@ namespace Grpc.Core.Internal 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); + var status = new Status(Native.grpcsharp_batch_context_recv_status_on_client_status(this), details, debugErrorString != null ? new DebugErrorException(debugErrorString) : null); IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this); var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr); diff --git a/src/csharp/Grpc.Core/Internal/DebugErrorException.cs b/src/csharp/Grpc.Core/Internal/DebugErrorException.cs new file mode 100644 index 00000000000..a8c4561c4bf --- /dev/null +++ b/src/csharp/Grpc.Core/Internal/DebugErrorException.cs @@ -0,0 +1,35 @@ +#region Copyright notice and license + +// Copyright 2019 The 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. + +#endregion + +using System; +using System.Runtime.InteropServices; +using System.Threading; +using Grpc.Core.Utils; + +namespace Grpc.Core.Internal +{ + /// + /// Represents error details provides by C-core's debug_error_string + /// + internal class DebugErrorException : Exception + { + public DebugErrorException(string message) : base(message) + { + } + } +} From 2ca781f5a1259449cd3cbc602072ed82d07e54c3 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 3 Jun 2020 18:39:44 +0200 Subject: [PATCH 8/8] address review feedback --- src/csharp/Grpc.Core.Api/Status.cs | 14 ++++++++------ src/csharp/Grpc.Core.Tests/ClientServerTest.cs | 6 +++--- .../Grpc.Core/Internal/BatchContextSafeHandle.cs | 2 +- ...rorException.cs => CoreErrorDetailException.cs} | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) rename src/csharp/Grpc.Core/Internal/{DebugErrorException.cs => CoreErrorDetailException.cs} (87%) diff --git a/src/csharp/Grpc.Core.Api/Status.cs b/src/csharp/Grpc.Core.Api/Status.cs index 32f79f74cc1..c13f9d88133 100644 --- a/src/csharp/Grpc.Core.Api/Status.cs +++ b/src/csharp/Grpc.Core.Api/Status.cs @@ -46,15 +46,16 @@ namespace Grpc.Core /// Creates a new instance of Status. /// 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. /// /// Status code. /// Detail. - /// Optional internal error details. - public Status(StatusCode statusCode, string detail, Exception debugErrorException) + /// Optional internal error details. + public Status(StatusCode statusCode, string detail, Exception debugException) { StatusCode = statusCode; Detail = detail; - DebugErrorException = debugErrorException; + DebugException = debugException; } /// @@ -75,17 +76,18 @@ namespace Grpc.Core /// never rely on values of this field (it should use StatusCode and Detail 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. /// - public Exception DebugErrorException { get; } + public Exception DebugException { get; } /// /// Returns a that represents the current . /// 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}\")"; } diff --git a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs index 7072851732a..7ff639c7ca9 100644 --- a/src/csharp/Grpc.Core.Tests/ClientServerTest.cs +++ b/src/csharp/Grpc.Core.Tests/ClientServerTest.cs @@ -144,18 +144,18 @@ namespace Grpc.Core.Tests { helper.UnaryHandler = new UnaryServerMethod((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(() => 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(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); } diff --git a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs index e00f153c21f..025f93e86c8 100644 --- a/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs +++ b/src/csharp/Grpc.Core/Internal/BatchContextSafeHandle.cs @@ -93,7 +93,7 @@ namespace Grpc.Core.Internal 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); diff --git a/src/csharp/Grpc.Core/Internal/DebugErrorException.cs b/src/csharp/Grpc.Core/Internal/CoreErrorDetailException.cs similarity index 87% rename from src/csharp/Grpc.Core/Internal/DebugErrorException.cs rename to src/csharp/Grpc.Core/Internal/CoreErrorDetailException.cs index a8c4561c4bf..ca688648138 100644 --- a/src/csharp/Grpc.Core/Internal/DebugErrorException.cs +++ b/src/csharp/Grpc.Core/Internal/CoreErrorDetailException.cs @@ -26,9 +26,9 @@ namespace Grpc.Core.Internal /// /// Represents error details provides by C-core's debug_error_string /// - internal class DebugErrorException : Exception + internal class CoreErrorDetailException : Exception { - public DebugErrorException(string message) : base(message) + public CoreErrorDetailException(string message) : base(message) { } }