dotnet / dotnet/winforms

Reconsider propagating only the inner exception in Control.Invoke

Open
#5,352 11 comments 2 reactions 1 assignee Claimed by @KlausLoeffelmann View on GitHub
api-suggestion
Dominant language
C#
Stars
4.9k
Forks
1.1k
Avg merge
20h 23m
Merged PRs (30d)
103

Description

**Is your feature request related to a problem? Please describe.**

See this [Stackoverflow comment which describes the current behavior](https://stackoverflow.com/a/28055196/8731)

Currently, the Control.Invoke method only propagates the innermost exception to the caller [by calling Exception.GetBaseException in Control.InvokeMarshaledCallbacks()](https://github.com/dotnet/winforms/blob/main/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L6576).

There are multiple justifications from Microsoft in the comments (from 2015), and one of the is the "breaking change"-one.
I would like to raise again this problem, and create a discussion about it.

For me, this behavior is misleading and not documented, and can cause headaches about the loss of exception details when InvokeRequired/Invoke code is used.

In my case, I have code which ensure that the current thread is the UI one, and if not, uses Control.Invoke to call the method in the correct context. So, depending if the method was initially called from the UI thread or not, the raised exception is not the same.

Furthermore, if your code is based on [WindowsFormsSynchronizationContext](https://github.com/dotnet/winforms/blob/81774abbcbb885fe906f1c0f813e8175eef8705c/src/System.Windows.Forms/src/System/Windows/Forms/WindowsFormsSynchronizationContext.cs#L87), there is no way to use a workaround and preserve the inner exceptions. You have to wrap every call with a custom delegate which capture the exception in a `ExceptionDispatchInfo`, then throw again this exception as-is when the Send methods throws.

**Example**

Here is a sample program which demonstrates the current behavior:

```csharp
using System;
using System.Drawing;
using System.Runtime.InteropServices;
using System.Threading;
using System.Windows.Forms;

namespace WinFormsApp1
{
public partial class MainForm : Form
{
[STAThread]
static void Main()
{
Application.SetHighDpiMode(HighDpiMode.SystemAware);
Application.EnableVisualStyles();
Application.SetCompatibleTextRenderingDefault(false);
Application.Run(new MainForm());
}

public MainForm()
{
Text = RuntimeInformation.FrameworkDescription;
FormBorderStyle = FormBorderStyle.FixedSingle;
Size = new Size(440, 300);

Button button;

button = new Button()
{
Location = new Point(10, 10),
Size = new Size(200, 25),
Text = "On UI Thread",
};

Controls.Add(button);
button.Click += (o, e) => TryDoWork();

button = new Button()
{
Size = new Size(200, 25),
Location = new Point(10, 45),
Text = "On Non-UI Thread",
};

Controls.Add(button);
button.Click += (o, e) => ThreadPool.QueueUserWorkItem(x => TryDoWork());
}

private void TryDoWork()
{
try
{
DoWorkOnUIThread();
}
catch (Exception ex)
{
MessageBox.Show($"{ex.GetType().FullName}: {ex.Message}", "Exception", MessageBoxButtons.OK, MessageBoxIcon.Warning);
}
}

private void DoWorkOnUIThread()
{
// ensure we are on the UI thread
if (InvokeRequired)
{
Invoke(DoWorkOnUIThread);
return;
}

DoWork();
}

private static void DoWork()
{
try
{
throw new DivideByZeroException();
}
catch (Exception ex)
{
throw new InvalidOperationException("Outer Exception", ex);
}
}
}
}
```

When you click on the first button, the TryDoWork() method is called on UI thread, and the catch receive the `InvalidOperationException` instance.

But if you click the second button, the TryDoWork() method is called with a background thread, and the DoWorkOnUIThread makes uses of Invoke to switch to the UI thread. In this case, the Invoke method does not propagates the `InvalidOperationException` exception but the `DivideByZeroException`.

**Describe the solution you'd like and alternatives you've considered**

The primary reason of the removal of exception details was to ensure that the winform default exception dialog could display the most useful exception to the user.

The only solution I discovered is to change the code of [Control.InvokeMarshaledCallbacks()](https://github.com/dotnet/winforms/blob/main/src/System.Windows.Forms/src/System/Windows/Forms/Control.cs#L6576) to remove the call to `.GetBaseException()`.

This way, all details about the exception are propagated to the caller of the Control.Invoke/BeginInvoke method.

This is a breaking change in cases where code attached to Application.ThreadException event or code that catch Invoke exception expects that the exception raised is the innermost exception.

When the exception does not contains any InnerException, the current behavior will not change.

At least, if this breaking change is not accepted, the current behavior of Control.Invoke/BeginInvoke should be better described in documentation by clearly stating that only the innermost exception is propagated. Actually the [documentation](https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.control.invoke?view=net-5.0#System_Windows_Forms_Control_Invoke_System_Delegate_) says: "Exceptions that are raised during the call will be propagated back to the caller."

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.