Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Commit

Permalink
Log trace errors to Windows Event Log (#6921)
Browse files Browse the repository at this point in the history 
* Log trace errors to Windows Event Log

Use .NET Runtime event messages from mscorree.dll
Enable AppHost logging regardless of GUI bit
Always write errors to stderr too
Add apphost Advapi32.lib reference
Modify tests

Fixes & PR feedback
  • Loading branch information
@sdmaclea
sdmaclea committed Jun 25, 2019
1 parent aec4d1d commit ea3f1f8
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 178 deletions.
7 changes: 6 additions & 1 deletion src/corehost/cli/apphost/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,9 @@ if(WIN32)
set_property(TARGET ${PROJECT_NAME} PROPERTY
LINK_FLAGS "/MANIFEST:NO"
)
endif()
endif()

# Specify non-default Windows libs to be used for Arm/Arm64 builds
if (WIN32 AND (CLI_CMAKE_PLATFORM_ARCH_ARM OR CLI_CMAKE_PLATFORM_ARCH_ARM64))
target_link_libraries(apphost Advapi32.lib)
endif()
68 changes: 22 additions & 46 deletions src/corehost/corehost.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -253,21 +253,10 @@ pal::string_t g_buffered_errors;

void buffering_trace_writer(const pal::char_t* message)
{
// Add to buffer for later use.
g_buffered_errors.append(message).append(_X("\n"));
}

// Determines if the current module (should be the apphost.exe) is marked as Windows GUI application
// in case it's not a GUI application (so should be CUI) or in case of any error the function returns false.
bool get_windows_graphical_user_interface_bit()
{
HMODULE module = ::GetModuleHandleW(NULL);
BYTE *bytes = (BYTE *)module;

// https://en.wikipedia.org/wiki/Portable_Executable
UINT32 pe_header_offset = ((IMAGE_DOS_HEADER *)bytes)->e_lfanew;
UINT16 subsystem = ((IMAGE_NT_HEADERS *)(bytes + pe_header_offset))->OptionalHeader.Subsystem;

return subsystem == IMAGE_SUBSYSTEM_WINDOWS_GUI;
// Also write to stderr immediately
pal::err_fputs(message);
}

#endif
Expand All @@ -291,28 +280,9 @@ int main(const int argc, const pal::char_t* argv[])
}

#if defined(_WIN32) && defined(FEATURE_APPHOST)
if (get_windows_graphical_user_interface_bit())
{
pal::string_t env_value;
bool gui_errors_disabled = false;

if (pal::getenv(_X("DOTNET_DISABLE_GUI_ERRORS"), &env_value))
{
gui_errors_disabled = pal::xtoi(env_value.c_str()) == 1;
}

if (!gui_errors_disabled)
{
trace::verbose(_X("Redirecting errors to custom writer."));
// If this is a GUI application, buffer errors to display them later. Without this any errors are effectively lost
// unless the caller explicitly redirects stderr. This leads to bad experience of running the GUI app and nothing happening.
trace::set_error_writer(buffering_trace_writer);
}
else
{
trace::verbose(_X("Gui errors disabled, keeping errors in stderr."));
}
}
trace::verbose(_X("Redirecting errors to custom writer."));
// Buffer errors to use them later.
trace::set_error_writer(buffering_trace_writer);
#endif

int exit_code = exe_start(argc, argv);
Expand All @@ -322,21 +292,27 @@ int main(const int argc, const pal::char_t* argv[])

#if defined(_WIN32) && defined(FEATURE_APPHOST)
// No need to unregister the error writer since we're exiting anyway.
if (!g_buffered_errors.empty())
if (exit_code != 0)
{
// If there are errors buffered, display them as a dialog. We only buffer if there's no console attached.
// If there are errors buffered, write them to the Windows Event Log.
pal::string_t executable_path;
pal::string_t executable_name;
if (pal::get_own_executable_path(&executable_name))
if (pal::get_own_executable_path(&executable_path))
{
executable_name = get_filename(executable_name);
executable_name = get_filename(executable_path);
}

trace::verbose(_X("Creating a GUI message box with title: '%s' and message: '%s;."), executable_name.c_str(), g_buffered_errors.c_str());

// Flush here so that when the dialog is up, the traces are up to date (specifically when they are redirected to a file).
trace::flush();

::MessageBoxW(NULL, g_buffered_errors.c_str(), executable_name.c_str(), MB_OK);
auto eventSource = ::RegisterEventSourceW(nullptr, _X(".NET Runtime"));
const DWORD traceErrorID = 1023; // Matches CoreCLR ERT_UnmanagedFailFast
pal::string_t message;
message.append(_X("Description: A .NET Application failed.\n"));
message.append(_X("Application: ")).append(executable_name).append(_X("\n"));
message.append(_X("Path: ")).append(executable_path).append(_X("\n"));
message.append(_X("Message: ")).append(g_buffered_errors).append(_X("\n"));

LPCWSTR messages[] = {message.c_str()};
::ReportEventW(eventSource, EVENTLOG_ERROR_TYPE, 0, traceErrorID, nullptr, 1, 0, messages, nullptr);
::DeregisterEventSource(eventSource);
}
#endif

Expand Down
142 changes: 11 additions & 131 deletions src/test/HostActivationTests/StandaloneAppActivation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ public void Running_Publish_Output_Standalone_EXE_with_Bound_AppHost_Succeeds()
}

[Fact]
public void Running_AppHost_with_GUI_Reports_Errors_In_Window()
public void Running_AppHost_with_GUI_No_Console()
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -255,67 +255,16 @@ public void Running_AppHost_with_GUI_Reports_Errors_In_Window()
UseBuiltAppHost(appExe);
MarkAppHostAsGUI(appExe);

Command command = Command.Create(appExe)
Command.Create(appExe)
.CaptureStdErr()
.CaptureStdOut()
.Start();

WaitForPopupFromProcess(command.Process);

// In theory we should close the window - but it's just easier to kill the process.
// The popup should be the last thing the process does anyway.
command.Process.Kill();

CommandResult result = command.WaitForExit(true);

// There should be no output written by the process.
Assert.Equal(string.Empty, result.StdOut);
Assert.Equal(string.Empty, result.StdErr);

result.Should().Fail();
}

[Fact]
public void Running_AppHost_with_GUI_Reports_Errors_In_Window_and_Traces()
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
// GUI app host is only supported on Windows.
return;
}

var fixture = sharedTestState.StandaloneAppFixture_Published
.Copy();

string appExe = fixture.TestProject.AppExe;

// Mark the apphost as GUI, but don't bind it to anything - this will cause it to fail
UseBuiltAppHost(appExe);
MarkAppHostAsGUI(appExe);

string traceFilePath;
Command command = Command.Create(appExe)
.EnableHostTracingToFile(out traceFilePath)
.Start();

WaitForPopupFromProcess(command.Process);

// In theory we should close the window - but it's just easier to kill the process.
// The popup should be the last thing the process does anyway.
command.Process.Kill();

CommandResult result = command.WaitForExit(true);

result.Should().Fail()
.And.FileExists(traceFilePath)
.And.FileContains(traceFilePath, "Creating a GUI message box with")
.And.FileContains(traceFilePath, "This executable is not bound to a managed DLL to execute.");

FileUtils.DeleteFileIfPossible(traceFilePath);
.Execute()
.Should().Fail()
.And.HaveStdErrContaining("This executable is not bound to a managed DLL to execute.");
}

[Fact]
public void Running_AppHost_with_GUI_Doesnt_Report_Errors_In_Window_When_Disabled()
public void Running_AppHost_with_GUI_Traces()
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -333,88 +282,19 @@ public void Running_AppHost_with_GUI_Doesnt_Report_Errors_In_Window_When_Disable
MarkAppHostAsGUI(appExe);

string traceFilePath;
Command command = Command.Create(appExe)
Command.Create(appExe)
.EnableHostTracingToFile(out traceFilePath)
.CaptureStdOut()
.CaptureStdErr()
.EnvironmentVariable(Constants.DisableGuiErrors.EnvironmentVariable, "1")
.Start();

CommandResult commandResult = command.WaitForExit(fExpectedToFail: false, timeoutMilliseconds: 30000);
if (commandResult.ExitCode == -1)
{
try
{
// Try to kill the process - it may be up with a dialog, or have some other issue.
command.Process.Kill();
}
catch
{
// Ignore exceptions, we don't know what's going on with the process.
}

Assert.True(false, "The process failed to exit in the alloted time, it's possible it has a dialog up which should not be there.");
}

commandResult
.CaptureStdOut()
.Execute()
.Should().Fail()
.And.FileExists(traceFilePath)
.And.FileContains(traceFilePath, "Gui errors disabled, keeping errors in stderr.")
.And.NotFileContains(traceFilePath, "Creating a GUI message box with")
.And.FileContains(traceFilePath, "This executable is not bound to a managed DLL to execute.");
.And.FileContains(traceFilePath, "This executable is not bound to a managed DLL to execute.")
.And.HaveStdErrContaining("This executable is not bound to a managed DLL to execute.");

FileUtils.DeleteFileIfPossible(traceFilePath);
}

#if WINDOWS
private delegate bool EnumThreadWindowsDelegate(IntPtr hWnd, IntPtr lParam);

[DllImport("user32.dll")]
private static extern bool EnumThreadWindows(int dwThreadId, EnumThreadWindowsDelegate plfn, IntPtr lParam);

private IntPtr WaitForPopupFromProcess(Process process, int timeout = 30000)
{
IntPtr windowHandle = IntPtr.Zero;
StringBuilder diagMessages = new StringBuilder();

int longTimeout = timeout * 3;
int timeRemaining = longTimeout;
while (timeRemaining > 0)
{
foreach (ProcessThread thread in process.Threads)
{
// Note we take the last window we find - there really should only be one at most anyway.
EnumThreadWindows(thread.Id,
(hWnd, lParam) => {
diagMessages.AppendLine($"Callback for a window {hWnd} on thread {thread.Id}.");
windowHandle = hWnd;
return true;
},
IntPtr.Zero);
}

if (windowHandle != IntPtr.Zero)
{
break;
}

Thread.Sleep(100);
timeRemaining -= 100;
}

// Do not fail if the window could be detected, sometimes the check is fragile and doesn't work.
// Not worth the trouble trying to figure out why (only happens rarely in the CI system).
// We will rely on product tracing in the failure case.

return windowHandle;
}
#else
private IntPtr WaitForPopupFromProcess(Process process, int timeout = 5000)
{
throw new PlatformNotSupportedException();
}
#endif

private void UseBuiltAppHost(string appExe)
{
File.Copy(Path.Combine(sharedTestState.RepoDirectories.HostArtifacts, AppHostExeName), appExe, true);
Expand Down

0 comments on commit ea3f1f8

Please sign in to comment.