From adfc7bf2aa5acbdce95c21fa9b67a692651b4fbc Mon Sep 17 00:00:00 2001 From: David Wengier Date: Tue, 4 Jun 2024 15:36:54 -0700 Subject: [PATCH 1/3] Use a serialization compatible marker --- src/Compilers/Core/Portable/InternalUtilities/FatalError.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs index 262e9fabf9326..8234a75e6e0bf 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs @@ -225,7 +225,7 @@ public static bool ReportAndCatchUnlessCanceled(Exception exception, Cancellatio #endif - private static readonly object s_reportedMarker = new(); + private static readonly Guid s_reportedMarker = Guid.NewGuid(); // Do not allow this method to be inlined. That way when we have a dump we can see this frame in the stack and // can examine things like s_reportedExceptionMessage. Without this, it's a lot tricker as FatalError is linked From f0b7b13f1f0147773b448015540e112965846cd8 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 5 Jun 2024 09:21:13 -0700 Subject: [PATCH 2/3] PR Feedback, switch to storing as object --- src/Compilers/Core/Portable/InternalUtilities/FatalError.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs index 8234a75e6e0bf..ccd54380c1a61 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs @@ -225,7 +225,7 @@ public static bool ReportAndCatchUnlessCanceled(Exception exception, Cancellatio #endif - private static readonly Guid s_reportedMarker = Guid.NewGuid(); + private static readonly object s_reportedMarker = Guid.NewGuid(); // Do not allow this method to be inlined. That way when we have a dump we can see this frame in the stack and // can examine things like s_reportedExceptionMessage. Without this, it's a lot tricker as FatalError is linked From 1921ec627d33a919c6cbfa6fb0be50f292617f95 Mon Sep 17 00:00:00 2001 From: David Wengier Date: Wed, 12 Jun 2024 09:32:33 +1000 Subject: [PATCH 3/3] Test and comment --- .../Core/Portable/InternalUtilities/FatalError.cs | 3 +++ .../CoreTest/UtilityTest/ExceptionHelpersTests.cs | 14 ++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs index ccd54380c1a61..492df67ae2597 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/FatalError.cs @@ -225,6 +225,9 @@ public static bool ReportAndCatchUnlessCanceled(Exception exception, Cancellatio #endif + // We use a Guid for the marker because it is used as a key in an exceptions Data dictionary, so we must make sure + // it's serializable if the exception crosses an RPC boundary. In particular System.Text.Json doesn't like plain + // object dictionary keys. private static readonly object s_reportedMarker = Guid.NewGuid(); // Do not allow this method to be inlined. That way when we have a dump we can see this frame in the stack and diff --git a/src/Workspaces/CoreTest/UtilityTest/ExceptionHelpersTests.cs b/src/Workspaces/CoreTest/UtilityTest/ExceptionHelpersTests.cs index d961d6be49cce..a135e545e529a 100644 --- a/src/Workspaces/CoreTest/UtilityTest/ExceptionHelpersTests.cs +++ b/src/Workspaces/CoreTest/UtilityTest/ExceptionHelpersTests.cs @@ -5,6 +5,7 @@ #nullable disable using System; +using System.Text.Json; using Microsoft.CodeAnalysis.ErrorReporting; using Roslyn.Test.Utilities; using Roslyn.Utilities; @@ -55,5 +56,18 @@ void a() Assert.True(false, "Should have returned in the catch block before this point."); } + + [Fact] + public void ErrorReporting_SerializesException() + { + FatalError.SetHandlers(delegate { }, delegate { }); + + var e = new Exception("Hello"); + FatalError.ReportNonFatalError(e); + + Assert.NotEmpty(e.Data); + Assert.NotNull(JsonSerializer.Serialize(e)); + Assert.NotNull(Newtonsoft.Json.JsonConvert.SerializeObject(e)); + } } }