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

Log trace errors to Windows Event Log #6921

Merged
merged 3 commits into from
Jun 25, 2019
Merged

Log trace errors to Windows Event Log #6921

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70a7a89
Log trace errors to Windows Event Log
sdmaclea Jun 25, 2019
c0fd40a
Fixes & PR feedback
sdmaclea Jun 25, 2019
d65ec3e
Add apphost Advapi32.lib reference
sdmaclea Jun 25, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
35 changes: 9 additions & 26 deletions src/corehost/corehost.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,25 +293,9 @@ int main(const int argc, const pal::char_t* argv[])
#if defined(_WIN32) && defined(FEATURE_APPHOST)
if (get_windows_graphical_user_interface_bit())
sdmaclea marked this conversation as resolved.
Show resolved Hide resolved
{
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."));
// If this is a GUI application, buffer errors to use them later.
trace::set_error_writer(buffering_trace_writer);
sdmaclea marked this conversation as resolved.
Show resolved Hide resolved
}
#endif

Expand All @@ -324,19 +308,18 @@ int main(const int argc, const pal::char_t* argv[])
// No need to unregister the error writer since we're exiting anyway.
if (!g_buffered_errors.empty())
{
// 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_name;
if (pal::get_own_executable_path(&executable_name))
{
executable_name = get_filename(executable_name);
}

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, executable_name.c_str());
const DWORD traceErrorID = 1024; // Somewhat arbitrary
LPCWSTR messages[] = {g_buffered_errors.c_str()};
::ReportEventW(eventSource, EVENTLOG_ERROR_TYPE, 0, traceErrorID, nullptr, 1, 0, messages, nullptr);
::DeregisterEventSource(eventSource);
}
#endif

Expand Down
118 changes: 2 additions & 116 deletions src/test/HostActivationTests/StandaloneAppActivation.cs
View file
Open in desktop
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 @@ -260,12 +260,6 @@ public void Running_AppHost_with_GUI_Reports_Errors_In_Window()
.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.
Expand All @@ -276,7 +270,7 @@ public void Running_AppHost_with_GUI_Reports_Errors_In_Window()
}

[Fact]
public void Running_AppHost_with_GUI_Reports_Errors_In_Window_and_Traces()
public void Running_AppHost_with_GUI_Traces()
{
if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
Expand All @@ -298,123 +292,15 @@ public void Running_AppHost_with_GUI_Reports_Errors_In_Window_and_Traces()
.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);
}

[Fact]
public void Running_AppHost_with_GUI_Doesnt_Report_Errors_In_Window_When_Disabled()
{
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)
.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
.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.");

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