Reconsider propagating only the inner exception in Control.Invoke

[tbolon]

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

See this Stackoverflow comment which describes the current behavior

Currently, the Control.Invoke method only propagates the innermost exception to the caller by calling Exception.GetBaseException in Control.InvokeMarshaledCallbacks().

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, 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:

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() 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 says: "Exceptions that are raised during the call will be propagated back to the caller."

[tbolon]

Sorry, I initially closed this issue because I didn't test if this behavior was present in latest .net core.

I can now confirm that the behavior is still the same in .NET 6.

[tbolon]

I am still trying to understand how exception are propagated when using Control.Invoke/BeginInvoke.

Please ignore this issue for now, I will update it with a solution when I find one.

[tbolon]

Based on my investigation, there is no trivial way to throw the detailed exception AND preserve the current behavior of showing only the innermost exception in Application.ThreadException and the ThreadExceptionDialog.

The only way I found to preserve all the exception details is to remove the call to .GetBaseException().

I would like to keep my suggestion opened for discussion as I see no real interest in having Control.Invoke only keeping the innermost exception, or at least suggest some documentation changes.

[RussKie]

Thank you for raising the issue. It is also late in .NET 6.0 dev cycle, we need to have especially strong reasons to make breaking changes now. Please bear with us, this is a complex topic which requires time to sit down, investigate and think it through. The team is quite pre-occupied making sure .NET 6.0 and the designer are as good as they can be. We likely be revisiting this discussion closer/after .NET 6.0 goes GA.

[tbolon]

No problem, I did not intend that this change should be considered for .NET 6.0.

I just encountered this problem in my app, and found the problem interesting. And since a lot have changed since 2015, I found useful to raise this question again.

For sure, take your time polishing the release of .NET 6.0, and take some leave if you can :)

[JeremyKuhne]

@KlausLoeffelmann this is somewhat related to the async stuff you're looking at. Might be a useful option or overload perhaps?

[KlausLoeffelmann]

Might be. This should be included in some parent epic issue to keep track of all the async-context issues, IMHO. @merriemcgaw FYI.

[merriemcgaw]

@KlausLoeffelmann Yes! I think that's a great idea. As we get deeper into .NET 9 planning this is one place where I know we need to invest.

[KlausLoeffelmann]

In that, we should also include the community API suggestion of automatically marshalling a ViewModel Property Change on a non-UI thread to the UI-thread.

Are you setting that Epic up, or do you want me to do that?