ThreadException event handler doesn't work

Oct 15, 2012 at 5:16 AM

Hi,

I have a solution including WinForms Application (set as startup project), Class Library with my custom shapes and a bunch of NShape projects (NShape, WinFormsUI, GeneralShapes etc). In one of my custom shapes (derived from PolylineBase) I override the Connect() method to throw an exception in certain situations (otherwise base.Connect(...) is called there). In my startup application I assign a handler to the Application.ThreadException event.

In Debug configuration, my event handler doesn't work, but at least I see some dialog "Assertion Failed: Abort=Quit, Retry=Debug, Ignore=Continue". I do not understand where this dialog is shown up: searching through code for "Assertion Failed" string gave me nothing. Stepping through code in debugger didn't help a lot - I lost myself in long event chains (mainly paint stuff) and gave up. Is this a standard dialog raised by System.Diagnostics.Something?

In Release configuration, my event handler also doesn't work - and I see no any dialog, of course. I can only see the following in the Output Window: "A first chance exception of type '...' occurred in <...>.dll". Maybe the exception is suppressed by some catch clause in your code - but I couldn't locate it having drowned in the event chains :)

Why my event handler doesn't work? And how can I make it work?

Coordinator
Oct 15, 2012 at 9:12 AM
Edited Oct 15, 2012 at 9:14 AM

The assertion failure message you see comes from System.Diagnostics.Debug.Assert() or System.Diagnostics.Debug.Fail(). As you already noticed, it's only executed in Debug mode.

And yes, unfortunately, there is a catch handler in the OnMouse... Methods of the display. In Debug mode, an assertion is raised, in both modes, the Tool action (Connection in your case) is cancelled.
You can use this for your purposes as it already does what you want (except showing a message box).

If you do not like this, the cleanest way would be to derive a custom SelectionTool and prevent the connection:

    public class MySelectionTool : SelectionTool {

        // The given IDiagramPresenter interface is in fact a reference on the Display component that owns the tool.
        public override bool ProcessMouseEvent(IDiagramPresenter diagramPresenter, MouseEventArgsDg e) {
            switch (e.EventType) {
                case Dataweb.NShape.Controllers.MouseEventType.MouseUp:
                    ShapeAtCursorInfo info = FindConnectionTargetFromPosition(diagramPresenter, e.Position.X, e.Position.Y, true, true);
                    // Check if the cursor is over a connection point, the shape under the cursor is
                    // of the correct type and if the connection point has Id 4 or 5
                    if (info.IsCursorAtConnectionPoint
                        && info.Shape is MyShapeType
                        && (info.ControlPointId == 4 || info.ControlPointId == 5)) {
                        // Connection is only allowed if ControlPoint has no connection yet
                        if (info.Shape.IsConnected(info.ControlPointId, null) != ControlPointId.None) {
                            // There is a connection: Cancel action and return
                            Cancel();
                            return false;
                        }
                    }
                    return base.ProcessMouseEvent(diagramPresenter, e);

                default: return base.ProcessMouseEvent(diagramPresenter, e);
            }
        }
    }

This code is untested but it should work.

Oct 15, 2012 at 10:29 AM
Edited Oct 15, 2012 at 10:51 AM

What about removing the catch clause from OnMouse... methods of the Display? I think it would be nice to have more freedom...

As I already mentioned in another thread, a rather common task of preventing connection in certain cases turned out to be a back breaker! Because we are not able neither to override the AttachGluePointToConnectionPoint() method (it's sealed) nor to catch our exceptions from out the [overriden] Connect() method, this task becomes hard to implement! Of course, Your suggestion (regarding MySelectionTool) is acceptable but... it seems to be not so nice as the previously mentioned two approaches - because instead of dealing with high-level stuff like attaching glue points or connecting shapes we drill down to low-level mouse stuff.

Seriously, I didn't expect that it will be so hard to solve this task! Your framework seems to be brilliant, but some tiny things are unexpectedly difficult.

[UPDATE] Well, simply removing the catch clause would be not correct, but re-throwing exception after it was handled would be more correct, I guess:

try {
    if (CurrentTool.ProcessMouseEvent(this, WinFormHelpers.GetMouseEventArgs(MouseEventType.MouseUp, e, DrawBounds)))
        mouseEventWasHandled = true;
} catch (Exception exc) {
    CurrentTool.Cancel();
    Debug.Fail(GetFailMessage(exc));
    throw exc; // << CAN YOU ADD THIS?
}

Coordinator
Oct 15, 2012 at 1:23 PM

I agree: The exception should not be catched without any possibility of handling this case. We will discuss what to do in this case and change this issue with the next release.

Oct 18, 2012 at 6:49 AM

Regarding Your sample with MySelectionTool:

1. As far as I understand, your code processes MouseUp always, but it should do this only when MouseUp occurs in the process of a connection, i.e. when we're dragging the end of a line (of a specific type) and then depress mouse button. How can we distinguish this?

2. Target shape can be obtained from info.Shape, and what about the source shape? I mean the one where the connection has started.

3. I tried your code. Doesn't matter where I click with the mouse - info.Shape is always null.

4. As far as I couldn't test your code (see #3 above), I couldn't figure out what is the result of the "Cancel(); return false;". Does any exception occur? Does the line remain on the Display?

Thank you.


Coordinator
Oct 18, 2012 at 3:15 PM

First of all:
I've talked to the NShape architect - we will re-throw the exceptions that are caught in the OnMouseX methods. You can replace the catch blocks in the OnMouseX methods with the following:

} catch (Exception exc) {
	CurrentTool.Cancel();
	Debug.Print(GetFailMessage(exc));
	throw;
}

We will address the "Connection point accepts only n connections" problem by extending the base classes in a furture version.

Regarding your questions above:
You are right - my code is not working. Unfortunately, nearly all interesting methods and properties of the SelectionTool are declared as private. In addition to that, the SelectionTool is not really trivial. I'm currently working on a solution...

Oct 19, 2012 at 3:54 AM
KurtHolzinger wrote:

I've talked to the NShape architect - we will re-throw the exceptions that are caught in the OnMouseX methods. [...]

We will address the "Connection point accepts only n connections" problem by extending the base classes in a furture version. [...]

That's great! But can I make a suggestion? Could you please address a more common problem "Preventing connection in certain cases" instead of "Connection point accepts only n connections"? Limitation on the maximum number of connections is just a particular case, other logic may apply, e.g. shape of type "1" can be connected only to shape of type "2", or shapes of certain types can be connected only with a certain (corresponding) type of connector line, etc. The more freedom we have here - the better.

Well, let me share my final solution implemented with regard to all current limitations of the framework. Maybe it would be interesting to somebody. I gave up the idea to use the SelectionTool and used combination of two other approaches: overriding the Connect() method in my Line and processing the MouseUp event in the Diagram component.

Code of my connector:

public class DataFlowLine : PolylineBase
{
    // ...
    public override void Connect(ControlPointId ownPointId, Shape otherShape, ControlPointId otherPointId)
    {
        if (this.connectionInfos != null && this.connectionInfos.Count > 0)
        {
            Shape sourceShape = this.connectionInfos[0].OtherShape;
            Shape targetShape = otherShape;
            //DataFlowConnectionValidator.ValidateConnection(sourceShape, targetShape);
            //(cannot use ValidateConnection overload which throws exception - no chances to catch this
            // exception in my code; instead I will use overload which returns boolean value as well as
            // error text via output parameter)
            string errorText = null;
            if (!DataFlowConnectionValidator.ValidateConnection(sourceShape, targetShape, out errorText))
            {
                /*
                 * MessageBox.Show(errorText, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
                 * 
                 * NOTE: If I raise message here, Display.MouseUp event handler doesn't get invoked
                 *       as expected. So this message will be raised in Display.MouseUp event handler,
                 *       and here I will simply mark this line as "invalid":
                 */
                this.IsConnectionValid = false;
                this.InvalidConnectionReason = errorText;
                /*
                 * NOTE: Cannot delete this shape from diagram here: code "this.Diagram.Shapes.Remove(this)"
                 *       works without exceptions (at least in Release mode) BUT seems to leave our diagram
                 *       in an invalid state - so that diagram crashes later.
                 */
                return;
            }
        }
        this.IsConnectionValid = true;
        base.Connect(ownPointId, otherShape, otherPointId);
    }

    public bool IsConnectionValid { get; set; }
    public string InvalidConnectionReason { get; set; }
}

Code of my form:

private void display1_MouseUp(object sender, MouseEventArgs e)
{
    if (this.display1.CurrentTool is SelectionTool) // not sure this check is really needed
    {
        foreach (Shape s in this.display1.Diagram.Shapes)
        {
            if (s is DataFlowLine && (s as DataFlowLine).IsConnectionValid == false)
            {
                MessageBox.Show((s as DataFlowLine).InvalidConnectionReason, "Error", MessageBoxButtons.OK, MessageBoxIcon.Error);
                this.display1.DeleteShape(s);
                break;
            }
        }
    }
}

Connection prevention logic resides in a separate class:

internal static class DataFlowConnectionValidator
{
    public static bool ValidateConnection(Shape sourceShape, Shape targetShape, out string errorText)
    {
        errorText = null;
        try
        {
            /* Place any checks here. In order to prevent connection - throw ConnectionException, e.g.:
             *
             * if (!(sourceShape is IMySuperShape) || !(targetShape is IMySuperShape))
             *     throw new ConnectionException(
             *        "This connector can be used only for connecting Super Shapes :)", sourceShape, targetShape);
             */
        }
        catch(ConnectionException ex)
        {
            errorText = ex.Message;
        }
        return string.IsNullOrEmpty(errorText);
    }
}

Well, it's certainly not the best possible solution, but it works.

Coordinator
Oct 19, 2012 at 7:54 AM

There will be some (virtual) method or event returning true or false (or providing CancelEventArgs). We will not focus on "n connections per point".

Oct 19, 2012 at 7:59 AM

That's great! Hope this method will provide access to both shapes (source and target) as well as to the line used for connecting these shapes; I guess this will be enough for all possible scenarios.

Thank you!

By the way, when can we expect the next release of your framework? :)