4

When an exception is thrown in FormClosing, I can't catch it with a normal try/catch - why not?

Example:

  public partial class Form2 : Form
  {
    public Form2() { InitializeComponent(); }

    protected override void OnClosing(CancelEventArgs e)
    {
      base.OnClosing(e);
      throw new Exception("lets catch this");
    }
  }

and i try catching it like this:

  try
  {
    var f = new Form2();
    f.ShowDialog();
  }
  catch (Exception ex)
  { 
    //this is never hit!
    MessageBox.Show("try/catch: " + ex);
  }

The exception is thrown, just never caught in my try/catch.

I can, however, catch it using Application.ThreadException += .. but at that point it is hard to recover.

What can I do?

In addition:
I'm using Windows8 x64 - and the program has targetplatform x86
I found a problem close to mine here but my exception is not silent.

Update 1
When i attach to process, it fails like it would if I had just manually started the .exe file: enter image description here

Community
  • 1
  • 1
Jens Kloster
  • 11,099
  • 5
  • 40
  • 54

1 Answers1

4

This is the expected behavior.

The ShowDialog method isn't supposed to throw the exceptions that it's events throw. What you're doing is also the wrong approach. Your override on OnClosing should be safe and not throw any errors. When this isn't the case it's passed off as an UnhandledException.

You can catch the exception, but you should do it in your OnClosing handler.

protected override void OnClosing(CancelEventArgs e)
{
  base.OnClosing(e);
  try {
    throw new Exception("lets catch this");
  }
  catch (Exception e) {
    // Do something with e
  }
}

The code responsible for this behavior stripped down looks like:

  try
  {
    FormClosingEventArgs e1 = new FormClosingEventArgs(this.closeReason, false);
    this.OnClosing((CancelEventArgs) e1);
    this.OnFormClosing(e1);
  }
  catch (Exception ex)
  {
    Application.OnThreadException(ex);
  }

A solution to your problem could be something like this:

public partial class Form2 : Form
{
    // I'm using the event here, but if this class in turn is a base for another class, you might as well override the method. (Event will still work, since events allow multiple handlers when using the default event handlers.)
    private void Form2_FormClosing(object sender, FormClosingEventArgs e)
    {
        try
        {
            // Action that might throw an exception
            throw new Exception("Some exception");
        }
        catch (Exception ex)
        {
            OnClosingException = ex;
        }
    }

    public Exception OnClosingException { get; protected set; }
}

// When calling
var f = new Form2();
f.ShowDialog();
if (f.OnClosingException != null) // Re-throw the exception wrapped in another exception that describes the problem best
    throw new InvalidOperationException("Some message", f.OnClosingException); // Might not always be InvalidOperationException
// Instead of re-throwing you can also just handle the exception

If this is something you're planning on using in a lot of scenario's you might want to make an interface for this to wrap it all up in a nice structured way. For a one time scenario I wouldn't bother.

Aidiakapi
  • 6,034
  • 4
  • 33
  • 62
  • Why wrap all event handlers with try-catch? Why not save the exception in `OnThreadException` event handler? – Dialecticus Nov 28 '13 at 10:51
  • @Dialecticus Because the original poster wants to catch it? The other reason would be good design. Unhandled events indicate a problem with the code, because it's something you don't expect. If you expect a method to throw an exception you can catch it, and gracefully recover from it. Unhandled exception generally means there's you can't safely recover and still be in a reliable state. You shouldn't wrap every event handler with a try-catch, only when you expect an exception you can handle. However, when __firing__ events it's generally best to always wrap it in a try-catch. – Aidiakapi Nov 28 '13 at 11:19
  • 1
    I don't know how other programmers feel about it but I would really like to have one catch to catch all **unexpected** exceptions, and in this catch-all I would most likely just inform the user and/or write the exception in log file. The chances that app will be in fault state are very low, but even so the user is warned. I really do not want to wrap all event handlers with try-catch to cover those **unexpected** exceptions. – Dialecticus Nov 28 '13 at 11:46
  • I feel *exacly* like @Dialecticus - in that i don't want to wrap every event handler in a "generel" try/catch. Having said that - @Aidiakapi answer my question, and explains why I can't catch the exception like I want to. But why is it better to go `Application.OnThreadException(ex)` instead of just throwing the exception directly? – Jens Kloster Nov 28 '13 at 12:15
  • I said that that code was the code responsible for the behavior. It's defined by the System.Windows.Forms.Form class. If you don't want the general try catch statement, why is that statement in your example code. [This article](http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx) does a nice job of explaining the types of exceptions, and how you should treat them. I don't know where you read my suggestion of placing a general try-catch around every event handler. That's nowhere in my answer, my 'solution' is a mirror of the code you supplied in the question. – Aidiakapi Nov 28 '13 at 12:51