IEnumerator<IResult> is not disposed when result fails


I have a button x:Name="Start" and method
public IEnumerable<IResult> Start()
    using (Disposable.Create(() => AppendLog("Finished")))
        yield return new DelegateResult(callback => callback(new InvalidOperationException("Horrible error")));
I expect to see "Finished" in the log but I don't. I suspect this can be fixed by disposing IEnumerator.

Implementation of DelegateResult is naive:
public class DelegateResult : IResult
    readonly Action<Action<Exception>> execute;

    public DelegateResult(Action<Action<Exception>> execute)
        this.execute = execute;

    public void Execute(ResultExecutionContext context)
        execute(ex => Completed(this, new ResultCompletionEventArgs { Error = ex }));

    public event EventHandler<ResultCompletionEventArgs> Completed;

file attachments

Closed Apr 2, 2011 at 7:31 PM by EisenbergEffect
Fixed in 77c065346871


EisenbergEffect wrote Apr 2, 2011 at 6:06 PM

Can you attach a sample that reproduces this? That would help me to get it fixed fast.

MatFiz wrote Apr 2, 2011 at 10:41 PM

Sample project attached.
SayHi method in ShellViewModel has [Rescue("OnError")]. After you click "Say Hi" you'll see "Horrible Error" displayed three times (i.e. OnError is invoked three times).

MatFiz wrote Apr 2, 2011 at 10:44 PM

Please remove my last comment, wrong issue

MatFiz wrote Apr 3, 2011 at 12:11 AM

Thanks for fixing that!
It was a really good surprise when I downloaded Caliburn sources, opened sln file, clicked Compile and it just worked!

There's a minor issue with Dispose being called before Rescue (I'd expect it to be called after).
I've attached a sample project (CaliburnDisposeResultEnumeratorBug.7z)

MatFiz wrote Apr 3, 2011 at 12:14 AM

Fix to correct order of Rescue and enumerator disposal (in SequentialResult.cs)
    void OnComplete(Exception error, bool wasCancelled) {
            Log.Info("Result enumeration complete.");
            Completed(this, new ResultCompletionEventArgs { Error = error, WasCancelled = wasCancelled });