Skip to content

Commit

Permalink
Add another handler
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi committed Oct 16, 2023
1 parent 1bba049 commit 2489912
Show file tree
Hide file tree
Showing 12 changed files with 65 additions and 45 deletions.
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Syntax/CSharpSyntaxNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ public static SyntaxNode DeserializeFrom(Stream stream, CancellationToken cancel
}

// Report NFW to see if this is being used in the wild.
FatalError.ReportAndPropagate(new SerializationDeprecationException());
FatalError.ReportNonFatalError(new SerializationDeprecationException());
using var reader = ObjectReader.TryGetReader(stream, leaveOpen: true, cancellationToken);

if (reader == null)
Expand Down
82 changes: 50 additions & 32 deletions src/Compilers/Core/Portable/InternalUtilities/FatalError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ public OperationCanceledIgnoringCallerTokenException(Exception innerException)
internal static class FatalError
{
public delegate void ErrorReporterHandler(Exception exception, ErrorSeverity severity, bool forceDump);

private static ErrorReporterHandler? s_handler;
private static ErrorReporterHandler? s_nonFatalHandler;

#pragma warning disable IDE0052 // Remove unread private members - We want to hold onto last exception to make investigation easier
private static Exception? s_reportedException;
Expand All @@ -37,21 +39,15 @@ internal static class FatalError
/// <summary>
/// Set by the host to handle an error report; this may crash the process or report telemetry.
/// </summary>
[DisallowNull]
public static ErrorReporterHandler? Handler
/// <param name="nonFatalHandler">A handler that will not crash the process when called. Used when calling <see
/// cref="ReportNonFatalError(Exception, ErrorSeverity, bool)"/></param>
public static void SetHandlers(ErrorReporterHandler handler, ErrorReporterHandler? nonFatalHandler)
{
get
{
return s_handler;
}

set
if (s_handler != handler)
{
if (s_handler != value)
{
Debug.Assert(s_handler == null, "Handler already set");
s_handler = value;
}
Debug.Assert(s_handler == null, "Handler already set");
s_handler = handler;
s_nonFatalHandler = nonFatalHandler;
}
}

Expand All @@ -72,25 +68,31 @@ public static void OverwriteHandler(ErrorReporterHandler? value)
/// This file is in linked into multiple layers, but we want to ensure that all layers have the same copy.
/// This lets us copy the handler in this instance into the same in another instance.
/// </remarks>
public static void CopyHandlerTo(Assembly assembly)
public static void CopyHandlersTo(Assembly assembly)
{
var targetType = assembly.GetType(typeof(FatalError).FullName!, throwOnError: true)!;
var targetHandlerProperty = targetType.GetProperty(nameof(FatalError.Handler), BindingFlags.Static | BindingFlags.Public)!;
if (Handler is not null)
{
// We need to convert the delegate type to the type in the linked copy since they won't have identity.
var convertedDelegate = Delegate.CreateDelegate(targetHandlerProperty.PropertyType, Handler.Target, method: Handler.Method);
targetHandlerProperty.SetValue(obj: null, value: convertedDelegate);
}
else
copyHandlerTo(assembly, s_handler, nameof(s_handler));
copyHandlerTo(assembly, s_nonFatalHandler, nameof(s_nonFatalHandler));

static void copyHandlerTo(Assembly assembly, ErrorReporterHandler? handler, string handlerName)
{
targetHandlerProperty.SetValue(obj: null, value: null);
var targetType = assembly.GetType(typeof(FatalError).FullName!, throwOnError: true)!;
var targetHandlerProperty = targetType.GetProperty(handlerName, BindingFlags.Static | BindingFlags.NonPublic)!;
if (handler is not null)
{
// We need to convert the delegate type to the type in the linked copy since they won't have identity.
var convertedDelegate = Delegate.CreateDelegate(targetHandlerProperty.PropertyType, handler.Target, handler.Method);
targetHandlerProperty.SetValue(obj: null, value: convertedDelegate);
}
else
{
targetHandlerProperty.SetValue(obj: null, value: null);
}
}
}

/// <summary>
/// Use in an exception filter to report an error without catching the exception.
/// The error is reported by calling <see cref="Handler"/>.
/// The error is reported by calling <see cref="s_handler"/>.
/// </summary>
/// <returns><see langword="false"/> to avoid catching the exception.</returns>
[DebuggerHidden]
Expand All @@ -101,7 +103,7 @@ public static bool ReportAndPropagate(Exception exception, ErrorSeverity severit
}

/// <summary>
/// Use in an exception filter to report an error (by calling <see cref="Handler"/>), unless the
/// Use in an exception filter to report an error (by calling <see cref="s_handler"/>), unless the
/// operation has been cancelled. The exception is never caught.
/// </summary>
/// <returns><see langword="false"/> to avoid catching the exception.</returns>
Expand All @@ -117,7 +119,7 @@ public static bool ReportAndPropagateUnlessCanceled(Exception exception, ErrorSe
}

/// <summary>
/// <para>Use in an exception filter to report an error (by calling <see cref="Handler"/>), unless the
/// <para>Use in an exception filter to report an error (by calling <see cref="s_handler"/>), unless the
/// operation has been cancelled at the request of <paramref name="contextCancellationToken"/>. The exception is
/// never caught.</para>
///
Expand Down Expand Up @@ -152,7 +154,7 @@ public static bool ReportAndPropagateUnlessCanceled(Exception exception, Cancell

/// <summary>
/// Report an error.
/// Calls <see cref="Handler"/> and doesn't pass the exception through (the method returns true).
/// Calls <see cref="s_handler"/> and doesn't pass the exception through (the method returns true).
/// This is generally expected to be used within an exception filter as that allows us to
/// capture data at the point the exception is thrown rather than when it is handled.
/// However, it can also be used outside of an exception filter. If the exception has not
Expand All @@ -175,7 +177,7 @@ public static bool ReportWithDumpAndCatch(Exception exception, ErrorSeverity sev
}

/// <summary>
/// Use in an exception filter to report an error (by calling <see cref="Handler"/>) and catch
/// Use in an exception filter to report an error (by calling <see cref="s_handler"/>) and catch
/// the exception, unless the operation was cancelled.
/// </summary>
/// <returns><see langword="true"/> to catch the exception if the error was reported; otherwise,
Expand All @@ -192,7 +194,7 @@ public static bool ReportAndCatchUnlessCanceled(Exception exception, ErrorSeveri
}

/// <summary>
/// <para>Use in an exception filter to report an error (by calling <see cref="Handler"/>) and
/// <para>Use in an exception filter to report an error (by calling <see cref="s_nonFatalHandler"/>) and
/// catch the exception, unless the operation was cancelled at the request of
/// <paramref name="contextCancellationToken"/>.</para>
///
Expand Down Expand Up @@ -230,12 +232,28 @@ public static bool ReportAndCatchUnlessCanceled(Exception exception, Cancellatio
// into many assemblies and finding the right type can be much harder.
[MethodImpl(MethodImplOptions.NoInlining)]
private static void Report(Exception exception, ErrorSeverity severity = ErrorSeverity.Uncategorized, bool forceDump = false)
{
ReportException(exception, severity, forceDump, s_handler);
}

/// <summary>
/// Used to report a non-fatal-watson (when possible) to report an exception. The exception is not caught. Does
/// nothing if no non-fatal error handler is registered. See the second argument to <see
/// cref="SetHandlers(ErrorReporterHandler, ErrorReporterHandler?)"/>.
/// </summary>
[MethodImpl(MethodImplOptions.NoInlining)]
public static void ReportNonFatalError(Exception exception, ErrorSeverity severity = ErrorSeverity.Uncategorized, bool forceDump = false)
{
ReportException(exception, severity, forceDump, s_nonFatalHandler);
}

private static void ReportException(Exception exception, ErrorSeverity severity, bool forceDump, ErrorReporterHandler? handler)
{
// hold onto last exception to make investigation easier
s_reportedException = exception;
s_reportedExceptionMessage = exception.ToString();

if (s_handler == null)
if (handler == null)
{
return;
}
Expand All @@ -256,7 +274,7 @@ private static void Report(Exception exception, ErrorSeverity severity = ErrorSe
exception.Data[s_reportedMarker] = s_reportedMarker;
}

s_handler(exception, severity, forceDump);
handler(exception, severity, forceDump);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Core/Portable/Syntax/SyntaxNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1406,7 +1406,7 @@ public virtual void SerializeTo(Stream stream, CancellationToken cancellationTok
}

// Report NFW to see if this is being used in the wild.
FatalError.ReportAndPropagate(new SerializationDeprecationException());
FatalError.ReportNonFatalError(new SerializationDeprecationException());
using var writer = new ObjectWriter(stream, leaveOpen: true, cancellationToken);
writer.WriteValue(Green);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Server/VBCSCompiler/BuildServerController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ internal static ICompilerServerHost CreateCompilerServerHost(ICompilerServerLogg
}

compilerServerHost.Logger.Log("Keep alive timeout is: {0} milliseconds.", keepAlive?.TotalMilliseconds ?? 0);
FatalError.Handler = FailFast.Handler;
FatalError.SetHandlers(FailFast.Handler, nonFatalHandler: null);

var dispatcher = new ServerDispatcher(compilerServerHost, clientConnectionHost, listener);
dispatcher.ListenAndDispatchConnections(keepAlive, cancellationToken);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Shared/Csc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal Csc(string responseFile, BuildPaths buildPaths, string[] args, IAnalyze

internal static int Run(string[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader)
{
FatalError.Handler = FailFast.Handler;
FatalError.SetHandlers(FailFast.Handler, nonFatalHandler: null);

var responseFile = Path.Combine(buildPaths.ClientDirectory, CSharpCompiler.ResponseFileName);
var compiler = new Csc(responseFile, buildPaths, args, analyzerLoader);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Shared/Vbc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ internal Vbc(string responseFile, BuildPaths buildPaths, string[] args, IAnalyze

internal static int Run(string[] args, BuildPaths buildPaths, TextWriter textWriter, IAnalyzerAssemblyLoader analyzerLoader)
{
FatalError.Handler = FailFast.Handler;
FatalError.SetHandlers(FailFast.Handler, nonFatalHandler: null);

var responseFile = Path.Combine(buildPaths.ClientDirectory, VisualBasicCompiler.ResponseFileName);
var compiler = new Vbc(responseFile, buildPaths, args, analyzerLoader);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic
End If

' Report NFW to see if this Is being used in the wild.
FatalError.ReportAndPropagate(New SerializationDeprecationException())
FatalError.ReportNonFatalError(New SerializationDeprecationException())

Using reader = ObjectReader.TryGetReader(stream, leaveOpen:=True, cancellationToken:=cancellationToken)
If reader Is Nothing Then
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public abstract class ResultProvider : IDkmClrResultProvider

static ResultProvider()
{
FatalError.Handler = FailFast.Handler;
FatalError.SetHandlers(FailFast.Handler, nonFatalHandler: null);
}

internal ResultProvider(IDkmClrFormatter2 formatter2, IDkmClrFullNameProvider fullNameProvider)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ public static void Initialize(ITelemetryReporter? reporter, string? telemetryLev
{
Contract.ThrowIfTrue(_instance is not null);

FatalError.Handler = ReportFault;
FatalError.CopyHandlerTo(typeof(Compilation).Assembly);
FatalError.ErrorReporterHandler handler = ReportFault;
FatalError.SetHandlers(handler, nonFatalHandler: handler);
FatalError.CopyHandlersTo(typeof(Compilation).Assembly);

if (reporter is not null && telemetryLevel is not null)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Interactive/HostProcess/InteractiveHostEntryPoint.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ internal static class InteractiveHostEntryPoint
{
private static async Task<int> Main(string[] args)
{
FatalError.Handler = FailFast.Handler;
FatalError.SetHandlers(FailFast.Handler, nonFatalHandler: null);

// Disables Windows Error Reporting for the process, so that the process fails fast.
SetErrorMode(GetErrorMode() | ErrorMode.SEM_FAILCRITICALERRORS | ErrorMode.SEM_NOOPENFILEERRORBOX | ErrorMode.SEM_NOGPFAULTERRORBOX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke
// Set both handlers to non-fatal Watson. Never fail-fast the VS process.
// Any exception that is not recovered from shall be propagated.
FaultReporter.InitializeFatalErrorHandlers();
FatalError.CopyHandlerTo(typeof(InteractiveHostFatalError).Assembly);
FatalError.CopyHandlersTo(typeof(InteractiveHostFatalError).Assembly);

// Explicitly set up FatalError handlers for the InteractiveWindowPackage.
Action<Exception> fatalHandler = e => FaultReporter.ReportFault(e, VisualStudio.Telemetry.FaultSeverity.Critical, forceDump: false);
Expand Down
5 changes: 3 additions & 2 deletions src/VisualStudio/Core/Def/Watson/FaultReporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,9 @@ internal static class FaultReporter

public static void InitializeFatalErrorHandlers()
{
FatalError.Handler = static (exception, severity, forceDump) => ReportFault(exception, ConvertSeverity(severity), forceDump);
FatalError.CopyHandlerTo(typeof(Compilation).Assembly);
FatalError.ErrorReporterHandler handler = static (exception, severity, forceDump) => ReportFault(exception, ConvertSeverity(severity), forceDump);
FatalError.SetHandlers(handler, nonFatalHandler: handler);
FatalError.CopyHandlersTo(typeof(Compilation).Assembly);
}

private static FaultSeverity ConvertSeverity(ErrorSeverity severity)
Expand Down

0 comments on commit 2489912

Please sign in to comment.