[C#] Click van Button veilig maken

[Daos]

We nemen een Button waar je events aan kan hangen (aan Click) die gevuurd worden als je er op klikt (door DoClick aan te roepen).

C#:

class Button
{
    public event EventHandler Click;

    public void DoClick() {
        if (Click != null) {
            Click (this, null);
        }
    }
}

Nu wil ik dat de Exceptions die eventHandlers gooien de boel niet laten crashen. Ik heb daarom het volgende bedacht:

C#:

class SafeHandler
{
    public static EventHandler Create(Action<object, EventArgs> handler) {
        return (sender, e) => {
            try {
                handler (sender, e);
            } catch (Exception ex) {
                Console.WriteLine ("Sync Exception: " + ex.Message);
            }
        };
    }

    public static EventHandler Create(Func<object, EventArgs, Task> asyncHandler) {
        return async (sender, e) => {
            try {
                await asyncHandler (sender, e);
            } catch (Exception ex) {
                Console.WriteLine ("Async Exception: " + ex.Message);
            }
        };
    }
}

Je stopt je functie/lambda door een statische Create-methode die er wat veiligers van maakt. Door overloading wordt automatisch de sync of async versie gepakt. Een Action is een functie die void is en een Func heeft een return-type. Dit werkt vrij aardig (in mijn console-test-project).

Probleem 1: Als een gewone/sync lamdba een Exception gooit pakt de verkeerde Create-methode hem. Hoe kan dat ineens? De rest van de lambda's gaat wel goed.

C#:

// this lambda is seen as async
Button btnExceptionV2 = new Button ();
btnExceptionV2.Click += SafeHandler.Create ((sender, e) => {
    throw new Exception ("btnExceptionV2 did something wrong");
});
btnExceptionV2.DoClick ();

// this one is correct
Button btnExceptionV3 = new Button ();
btnExceptionV3.Click += SafeHandler.Create ((sender, e) => {
    if (e == null)
        throw new Exception ("btnExceptionV3 did something wrong");
});
btnExceptionV3.DoClick ();

// async works also correctly
Button btnAsyncExceptionV2 = new Button ();
btnAsyncExceptionV2.Click += SafeHandler.Create (async (sender, e) => {
    await Task.Delay (3000);
    throw new Exception ("btnAsyncExceptionV2 too");
});
btnAsyncExceptionV2.DoClick ();

Probleem 2: Voor echte functies heb ik een cast nodig omdat de compiler anders niet weet wat hij moet doen. Is er een manier om die cast kwijt te raken? Of zit er niets anders op dan geen overloading meer te gebruiken? Ik vind het raar dat lambda's (meestal) geen problemen geven.

C#:

static void NotALambda(object sender, EventArgs e) {
    Console.WriteLine ("Hello from NotALambda");
}

static async Task AsyncNotALambda(object sender, EventArgs e) {
    await Task.Delay (1500);
    Console.WriteLine ("Hello from AsyncNotALambda");
}

// not a lambda does work
Button btnNotLambdaV2 = new Button ();
btnNotLambdaV2.Click += SafeHandler.Create ((Action<object, EventArgs>)NotALambda);
btnNotLambdaV2.DoClick ();

// async not a lambda also works
Button btnAsyncNotLambdaV2 = new Button ();
btnAsyncNotLambdaV2.Click += SafeHandler.Create ((Func<object, EventArgs, Task>)AsyncNotALambda);
btnAsyncNotLambdaV2.DoClick ();

Als alternatief heb ik ook een veilige Button bedacht:

C#:

class SafeButton : Button
{
    public event EventHandler SafeClick;

    public SafeButton () {
        Click += saveClickHandler;
    }

    private void saveClickHandler(object sender, EventArgs e) {

        try {
            if (SafeClick != null)
                SafeClick (sender, e);
        }
        catch (Exception ex) {
            Console.WriteLine ("Exception: " + ex.Message);
        }
    }
}

Probleem 3: Als ik sync dingen aan de SafeClick event hang werkt alles goed, maar bij async dingen niet. Ik kreeg geen gele lijn in mijn ide en mijn callstack zegt:

System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw () in 
System.Runtime.CompilerServices.AsyncMethodBuilderCore.<ThrowAsync>b__1 (state={System.Runtime.ExceptionServices.ExceptionDispatchInfo}) in 
System.Threading.QueueUserWorkItemCallback.WaitCallback_Context (state={System.Threading.QueueUserWorkItemCallback}) in 
System.Threading.ExecutionContext.RunInternal (executionContext={System.Threading.ExecutionContext}, callback={System.Threading.ContextCallback}, state={System.Threading.QueueUserWorkItemCallback}, preserveSyncCtx=true) in 
System.Threading.ExecutionContext.Run (executionContext={System.Threading.ExecutionContext}, callback={System.Threading.ContextCallback}, state={System.Threading.QueueUserWorkItemCallback}, preserveSyncCtx=true) in 
System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem () in 
System.Threading.ThreadPoolWorkQueue.Dispatch () in 
System.Threading._ThreadPoolWaitCallback.PerformWaitCallback () in 
[Native to Managed Transition] in 

Dit werkt ook al niet (met zelfde crash):

C#:

EventHandler AsyncLambda = async (sender, e) => {
    await Task.Delay (3000);
    Console.WriteLine ("AsyncLambda");
    throw new Exception ("AsyncLambda did something wrong");
};

// doesn't work
try {
    AsyncLambda (null, null);
}
catch (Exception e) {
    Console.WriteLine ("Exception: " + e.Message);
}

Het probleem zijn async-functies die void terug geven. Als je een async-functie hebt die Task of Task<T> terug geeft krijg je geen crashes (de exception zie je pas bij await). Die laatsten kan ik niet in mijn event stoppen, maar dat is het probleem niet.

Hoe vang ik Exceptions op of schakel die uit bij async void? Is daar echt niets voor? Ik kwam dit tegen: http://lunarfrog.com/blog...81-async-void-exceptions/

edit:
Update bij Probleem 2. Als ik overloading niet meer gebruik geeft dit weer een niet te vangen crash (CreateSync is hernoemde Create(Action<... van boven):

C#:

Button btnAsyncHandlerV3 = new Button ();
btnAsyncHandlerV3.Click += SafeHandler.CreateSync (async (sender, e) => {
    await Task.Delay (3000);
    throw new Exception ("btnAsyncHandlerV3 did something wrong");
});
btnAsyncHandlerV3.DoClick ();

Aaaaargggg... Het lukt gewoon niet mooi (= handig in gebruik en met weinig kans op fouten door gebruikers).

Een fail-safe button of handler-factory zou toch mogelijk moeten zijn? Iemand nog ideeen/tips?

[edit2]
Ik heb nu mijn eigen delegates. Dat maakt het casten wat dragelijker.

C#:

class SafeHandler
{
    public delegate void HandlerType(object sender, EventArgs e);
    public delegate Task AsyncHandlerType(object sender, EventArgs e);

    public static EventHandler Create(HandlerType handler) {
        return (sender, e) => {
                    // ...


// not a lambda does work
Button btnNotLambdaV2 = new Button ();
btnNotLambdaV2.Click += SafeHandler.Create ((SafeHandler.HandlerType)NotALambda);
btnNotLambdaV2.DoClick ();

Probleem 2 is opgelost. (Al vind ik het nog steeds oneerlijk dat lambda's naar return types kijken en gewone functies niet)

[ Voor 10% gewijzigd door Daos op 23-03-2014 01:17 ]

[Lethalis]

Ik heb het vermoeden dat je de exceptions op de verkeerde plek in jouw programma probeert op te lossen.

Als ik plugins zou moeten ondersteunen die mijn programma niet mogen crashen, dan zou ik de plugin architectuur zo opzetten dat het niet fout kan gaan, door bijvoorbeeld methods te definieren in een base class die derden kunnen overriden, die dus door mijn code al in een try catch worden uitgevoerd.

Misschien kun je toelichten wat je probeert te bereiken, want ik vind de user interface niet echt de plek voor een catch all exceptions scenario.

[Daos]

Het gaat niet om plugins, maar om code die ik zelf schrijf. Het komt bij mij vrij vaak voor dat het programma crasht op iets wat net zo goed genegeerd had kunnen worden.

Neem bijvoorbeeld een programma dat het gemiddelde berekent van getallen:

C#:

btnMyButton.Click += { (sender, e) =>
    text = txtMyTextField.Text;
    List<int> values = MyInputSanitizers.MySafeIntListParser(text);
    Console.WriteLine (values.Sum (v => v) / values.Count);
};

Ook al zet je de berekening in een aparte BusinessLogic klasse, dit crasht je programma als je geen waarden opgeeft en values een lege List is (delen door 0).

[Merethil]

Daos schreef op zondag 23 maart 2014 @ 11:23:
Het gaat niet om plugins, maar om code die ik zelf schrijf. Het komt bij mij vrij vaak voor dat het programma crasht op iets wat net zo goed genegeerd had kunnen worden.

Neem bijvoorbeeld een programma dat het gemiddelde berekent van getallen:

C#:

btnMyButton.Click += { (sender, e) =>
    text = txtMyTextField.Text;
    List<int> values = MyInputSanitizers.MySafeIntListParser(text);
    Console.WriteLine (values.Sum (v => v) / values.Count);
};

Ook al zet je de berekening in een aparte BusinessLogic klasse, dit crasht je programma als je geen waarden opgeeft en values een lege List is (delen door 0).

Vrij slecht voorbeeld lijkt me dan, want als ik zoiets zou maken zou ik een failfast-strategie volgen en eerst alles double-checken voor je de berekening uitvoert zodat je sowieso "nooit" bij die crash komt (al zijn er altijd mooie manieren om dat toch te doen).
Op die manier kan je gewoon (ik zie je een text-veld getten, dus je kan vast ook wel een label/textvel setten) je programma de berekening niet laten uitvoeren en een textveld/label setten als "Value A was not filled or was not a correct number" oid.

...Of denk ik nou te simpel?

[D-Raven]

Volgens mij werkt je async code niet omdat je geen controle hebt over waar de code word uitgevoerd. Er is geen TaskScheduler gedefineerd. Dus hij pakt de scheduler van de default/current context.

Zonder meer context in je voorbeeld heb ik geen idee welke dat is (en ik zou weer even moeten uitzoeken hoe die ook alweer aan zn default scheduler komt).

Punt is. Dat je dan dus niet weet waar je async code word uitgevoerd. Dit kan op de UI thread zijn, maar ook op een background thread. Het kan dus zijn dat je async code word uitgevoerd op een andere thread als waar je try catch leeft.

Zodra je met Tasks en exceptions gaat werken moet je deze sowieso via de TPL manier afvangen, ongeacht op welke thread je Task wordt uitgevoerd.

C#:

EventHandler AsyncLambda = async (sender, e) => {
    await Task.Delay (3000);
    Console.WriteLine ("AsyncLambda");
    throw new Exception ("AsyncLambda did something wrong");
};

// doesn't work
try {
    AsyncLambda (null, null);
}
catch (Exception e) {
    Console.WriteLine ("Exception: " + e.Message);
}

Alles wat in je handler staat moet je zien als iets wat door de compiler in een Task gewrapped wordt. Deze task wordt afaik door de default Task.Factory gemaakt. Alleen wordt er geen exception handler aan gehangen, want tjah. wat moet die dan doen? Dus als er dan een exception optreed gebeurd dat op een task waar geen exception handler is gedefinieerd. Waardoor t fout gaat, en je volgens mij een UnobserverTaskException krijgt of iets in die trant.

De manier om dit normaliter te doen, is om je async code een Task te laten returnen, zodat je op deze task instance kan observen of er een exception is opgetreden.

C#:

EventHandler AsyncLambda = async (sender, e) => {
return Task.Factory.StartNew(() => {
  await Task.Delay (3000);
    Console.WriteLine ("AsyncLambda");
    throw new Exception ("AsyncLambda did something wrong");
});
};

public static EventHandler Create(Func<object, EventArgs, Task> asyncHandler) {
        return async (sender, e) => {
            var resultTask = asyncHandler (sender, e);
            if(resultTask.Exception != null)
                     Console.Writeline(resultTask.Exception.Message);
        };
    } 

Ik zou hier nog eens kijjken: MSDN: Exception Handling (Task Parallel Library)

[ Voor 3% gewijzigd door D-Raven op 23-03-2014 12:45 ]

[Lethalis]

Daos schreef op zondag 23 maart 2014 @ 11:23:
Het gaat niet om plugins, maar om code die ik zelf schrijf. Het komt bij mij vrij vaak voor dat het programma crasht op iets wat net zo goed genegeerd had kunnen worden.

Neem bijvoorbeeld een programma dat het gemiddelde berekent van getallen.. Ook al zet je de berekening in een aparte BusinessLogic klasse, dit crasht je programma als je geen waarden opgeeft en values een lege List is (delen door 0).

Heb je ook een beter voorbeeld?

Als je dergelijke code bij ons op kantoor zou schrijven en niet eens een simpele check doet of de berekening ueberhaupt wel mogelijk is, dan kun je maar beter niet in je proeftijd zitten op dat moment

Sowieso wil je aan de gebruiker ook een zinvolle melding geven, bijvoorbeeld dat er nog geen getallen zijn ingevuld.

Je bent momenteel stront door een trechter aan het persen en probeert ervoor te zorgen dat hij nooit verstopt raakt

Input -> validatie -> verwerking -> uitvoer

Jij slaat een stap over.

PS:
Een generic list heeft ook een Average extension method. Dit voorkomt de exception niet, maar lijkt me netter dan eerst zelf een sum doen en daarna delen.

[ Voor 7% gewijzigd door Lethalis op 23-03-2014 17:47 ]

[Daos]

Merethil schreef op zondag 23 maart 2014 @ 11:39:
...Of denk ik nou te simpel?

Niet alle code is vrij van bugs. Heb jij er dan nooit last van dat je programma onderuit gaat? Zelfs windows crasht soms.

Het was trouwens een verzonnen voorbeeld. Mijn laatste crash was op een tablet en kwam door verkeerd gebruik door mij als programmeur van een foto-module. Ik had er geen rekening mee gehouden dat je als gebruiker ook nog het nemen van foto's kon afbreken. In andere gevallen zijn het meestal exceptions die door de verwerking gegooid worden.

D-Raven schreef op zondag 23 maart 2014 @ 12:44:
De manier om dit normaliter te doen, is om je async code een Task te laten returnen, zodat je op deze task instance kan observen of er een exception is opgetreden.

Dat async code die een Task returnt een goed idee is, dat snap ik inmiddels. Maar in een event kan je alleen dingen stoppen die void returnen.

Ik probeer dus (als alternatief voor mijn factory) een soort veilige button om een bestaande heen te bouwen. Met reflection kan ik inmiddels wel alle delegates in een event vinden. Kan ik op enige manier deze nog veranderen naar een functie die een Task terug geeft (zodat ik ze op een veilige manier uit kan voeren)?

Bij await werkt het trouwens tegenwoordig net ietsjes anders. Tasks gooien hun eigen exceptions waar de oorspronkelijke in zit. await haalt deze er weer uit en gooit deze op de plek waar await staat. Met try en catch om de await kan je de exception van de task afvangen.

C#:

            var resultTask = asyncHandler (sender, e);
            if(resultTask.Exception != null)

Niet getest, maar je bent misschien al voorbij de if voordat de andere taak een.Exception gooit.

Lethalis schreef op zondag 23 maart 2014 @ 17:32:
Je bent momenteel stront door een trechter aan het persen en probeert ervoor te zorgen dat hij nooit verstopt raakt

Input -> validatie -> verwerking -> uitvoer

Jij slaat een stap over.

Valt toch wel mee? Mijn voorbeeld is een twijfelgeval (geen getallen geeft lege lijst terug; kan misschien in andere gevallen nodig zijn), maar in het algemeen kan ook bij valide input er bij de verwerking en zelfs uitvoer nog van alles misgaan.

De meeste code zit zo in elkaar dat het alleen de problemen afvangt die dat stukje zelf kan verhelpen. De rest wordt gewoon doorgegooid. De afhandeling hiervan moet dus volgens mij tussen de UI en rest. Bij een console programma is het nog wel te doen. Invoer komt daar op 1 plek binnen (en gaat op 1 plek weer weg), maar bij een GUI word ik er langzamerhand gek van.

Bij de meeste knoppen zitten maar een paar dingen in de event-handler. try catch er omheen te zetten kost mij tijd en verzuim ik nog wel eens. Ik zoek iets dat sneller programmeert.

Een snippet in mijn ide instellen is misschien ook nog een optie...

PS:
Een generic list heeft ook een Average extension method. Dit voorkomt de exception niet, maar lijkt me netter dan eerst zelf een sum doen en daarna delen.

Het viel me nu pas op dat op msdn netjes aangegeven wordt welke exceptions gegooid worden. Misschien daar ook maar eens beter op letten