Revert "Revert "C# expose C-core's debug error string in Status struct""

This reverts commit 7c30f0c956.

src/csharp/Grpc.Core.Api/Status.cs

@@ -14,6 +14,8 @@
 // limitations under the License.
 #endregion
 
+using System;
+
 namespace Grpc.Core
 {
     /// <summary>
@@ -31,48 +33,63 @@ namespace Grpc.Core
         /// </summary>
         public static readonly Status DefaultCancelled = new Status(StatusCode.Cancelled, "");
 
-        readonly StatusCode statusCode;
-        readonly string detail;
-
         /// <summary>
         /// Creates a new instance of <c>Status</c>.
         /// </summary>
         /// <param name="statusCode">Status code.</param>
         /// <param name="detail">Detail.</param>
-        public Status(StatusCode statusCode, string detail)
+        public Status(StatusCode statusCode, string detail) : this(statusCode, detail, null)
         {
-            this.statusCode = statusCode;
-            this.detail = detail;
         }
 
         /// <summary>
-        /// Gets the gRPC status code. OK indicates success, all other values indicate an error.
+        /// 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>
-        public StatusCode StatusCode
+        /// <param name="statusCode">Status code.</param>
+        /// <param name="detail">Detail.</param>
+        /// <param name="debugException">Optional internal error details.</param>
+        public Status(StatusCode statusCode, string detail, Exception debugException)
         {
-            get
-            {
-                return statusCode;
-            }
+            StatusCode = statusCode;
+            Detail = detail;
+            DebugException = debugException;
         }
 
+        /// <summary>
+        /// Gets the gRPC status code. OK indicates success, all other values indicate an error.
+        /// </summary>
+        public StatusCode StatusCode { get; }
+
         /// <summary>
         /// Gets the detail.
         /// </summary>
-        public string Detail
-        {
-            get
-            {
-                return detail;
-            }
-        }
+        public string Detail { get; }
+
+        /// <summary>
+        /// 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 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 DebugException { get; }
 
         /// <summary>
         /// Returns a <see cref="System.String"/> that represents the current <see cref="Grpc.Core.Status"/>.
         /// </summary>
         public override string ToString()
         {
-            return string.Format("Status(StatusCode={0}, Detail=\"{1}\")", statusCode, detail);
+            if (DebugException != null)
+            {
+                return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\", DebugException=\"{DebugException}\")";
+            }
+            return $"Status(StatusCode=\"{StatusCode}\", Detail=\"{Detail}\")";
         }
     }
 }

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<string, string>((request, context) =>
+            {
+                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.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.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);
+        }
+
         [Test]
         public void UnaryCall_ServerHandlerSetsStatusAndTrailers()
         {

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 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 CoreErrorDetailException(debugErrorString) : null);
 
             IntPtr metadataArrayPtr = Native.grpcsharp_batch_context_recv_status_on_client_trailing_metadata(this);
             var metadata = MetadataArraySafeHandle.ReadMetadataFromPtrUnsafe(metadataArrayPtr);

src/csharp/Grpc.Core/Internal/CoreErrorDetailException.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
+{
+    /// <summary>
+    /// Represents error details provides by C-core's debug_error_string
+    /// </summary>
+    internal class CoreErrorDetailException : Exception
+    {
+        public CoreErrorDetailException(string message) : base(message)
+        {
+        }
+    }
+}

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<Delegates.grpcsharp_batch_context_recv_message_next_slice_peek_delegate>(library);
             this.grpcsharp_batch_context_recv_status_on_client_status = GetMethodDelegate<Delegates.grpcsharp_batch_context_recv_status_on_client_status_delegate>(library);
             this.grpcsharp_batch_context_recv_status_on_client_details = GetMethodDelegate<Delegates.grpcsharp_batch_context_recv_status_on_client_details_delegate>(library);
+            this.grpcsharp_batch_context_recv_status_on_client_error_string = GetMethodDelegate<Delegates.grpcsharp_batch_context_recv_status_on_client_error_string_delegate>(library);
             this.grpcsharp_batch_context_recv_status_on_client_trailing_metadata = GetMethodDelegate<Delegates.grpcsharp_batch_context_recv_status_on_client_trailing_metadata_delegate>(library);
             this.grpcsharp_batch_context_recv_close_on_server_cancelled = GetMethodDelegate<Delegates.grpcsharp_batch_context_recv_close_on_server_cancelled_delegate>(library);
             this.grpcsharp_batch_context_reset = GetMethodDelegate<Delegates.grpcsharp_batch_context_reset_delegate>(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);
             

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;
 

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();

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)',