Permit yield in catch a̶n̶d̶ ̶f̶i̶n̶a̶l̶l̶y̶ blocks

[lcsondes]

Since C# 6, await is allowed in catch and finally blocks. I would like if yield return and yield break were brought up to parity and permitted in try, catch, and finally blocks.

Edit: After detailed discussion (thank you everyone!) it seems that the finally case would just cause more trouble and confusion than it's worth, however the catch seems straightforward, and it would also match regular return perfectly.

[jnm2]

yield return and yield break already work in try blocks. What's the use case for having them work in catch and finally blocks?

[lcsondes]

Having yield return in a try that has a catch (admittedly not every try) is an error, CS1626. Or at least it is in a default install of VS2017, I assume that's the latest C#.

My use case is performing a test or another operation on each element of an IEnumerable<Thing>, and producing an IEnumerable<Result> from it, similar to Enumerable.Select. However for this scenario an exception is also a kind of Result, so I am putting them into the output with yield return. This is not entirely unlike Task<T>, which can also represent either a normal result or an exception.

I need to do a workaround with a temp variable like this:

Result temp = null;
try
{
    temp = new Result(operation(thing));
}
catch(Exception e)
{
    temp = new Result(e);
}
yield return temp;

Instead of the direct version:

try
{
    yield return new Result(operation(thing));
}
catch(Exception ex)
{
    yield return new Result(ex);
}

[jnm2]

Actually, that makes a lot of sense to me. You should always put as little code as humanly possible inside a try block. Including the yield return in the catching makes it seem like you're catching exceptions from the code you're yielding control to at that point. That makes sense with await but it does not make sense with yield.

In other words, I believe that including new Result in your try block is poor coding, let alone yield return. operation.Invoke, given only constants, literals, locals, parameters or fields (things that can't throw), should be the only thing inside that try.

(Also you should almost never use a catch-all except as the top-level handler. If you don't know precisely why an exception is being thrown, any kind of corruption may have occurred and continuing operation is dangerous. Failing fast and loud is safe and helpful.)

[jnm2]

I agree that it isn't pretty, but hopefully that discourages common abuses of try-catch.

[lcsondes]

This was a greatly simplified example, but it clearly shows the feature disparity between yield and await. Best practices are a different discussion altogether, I'm sure many people would love if C# deprecated goto.

This milder example also doesn't work:

try
{
    yield return Foo(myThing1, myThing2);
}
catch(VerySpecificException e)
{
    // For this method's callers, VerySpecificExceptions coming from Foo are not important,
    // but log them for diagnostics
    myLogger.LogExpectedException(e);
}

[Logerfo]

@jskeet once explained it. (times flies, doesn't it?).
My thoughts, regarding time passing:

• The compiler grew up a lot. This can both mean it is easier or harder to allow this today (hopefully someone can answer that).
• New language features allowed us to write more complex code with fewer lines/characters, which makes a contrast with the ugly code in cases where at least one goto statement is required to have a try/catch block to return something via lazy enumeration.
• Personal computers processing speed quite increased. This might mean that the complexity added to the compiler regarding the feature you asked may not be that much nowadays.

I agree that best practices should not be something on the table. I don't believe that being a bad practice is a valid argument against the development of a language feature.

[HaloFour]

The reason for not being able to yield within a try block is the potential expectation that the iterator would be able to handle exceptions thrown by the enumerator, which is not the case.

CodePlex: Anonymous Iterators, Yielding in a Try...Catch Block

[lcsondes]

I'm not sure I follow.

How is this:

try
{
    yield return Foo();
}
catch(SomeException e)
{
    // handle the exception
}

expected to behave differently than this?

SomeType temp;
bool ranSuccessfully = false;
try
{
    temp = Foo();
    ranSuccessfully = true;
}
catch(SomeException e)
{
    // handle the exception
}
if(ranSuccessfully)
{
    yield return temp;
}

[lcsondes]

The argument that the callee can somehow handle exceptions made from its caller is weird, but at least the feature is available in VB, that's good to know.

[HaloFour]

@lcsondes

The "mental model", as it were, is that the yield keyword is calling back into the enumerator (e.g. the body of the foreach loop). So yes, it is different. Doesn't seem like @gafter is that convinced of that argument and it is otherwise technically feasible so you might be able to get someone on the team interested in changing it.

[lcsondes]

That's a weird model. The iterator "calling up" to the enumerator body goes against the syntax as well as the actual implementation.

[jnm2]

@lcsondes No weirder than await which is designed to yield control but create the mental model of synchronous programming. Same deal.

[orthoxerox]

I have actually written production code in PL/SQL that does its local equivalent of yielding in a catch block, so I can see the benefit of allowing that instead of emulating the behavior with flags.

[lcsondes]

@jnm2 Sure. Iterators also pretend that you're writing synchronous code, but you're yielding control to your caller. I expected them to behave similarly when it came to exceptions.

[sharwell]

@HaloFour It would never cross my mind to think about it like that. And I can't even imagine trying to explain the semantics of the result to someone.

Supporting yield in catch/finally seems like a straightforward improvement to the language, so I expect the only real issue would be the time it takes to implement and test properly.

[lcsondes]

I don't think it's fundamentally different from try{yield return 1;yield break;}finally{Write(2);}. I'd have similar expectations as above: if it's MoveNext() then return false and stop for now, if it's Dispose() then keep running until control leaves the iterator body. It's like yield return but skipping the expression evaluation and Current updating part. After checking the details of yield break it should execute finally (or the rest of the finally block if it's in one as per this proposal) even in MoveNext, then return false from it.

This code has similar theoretical problems but it already compiles, calling MoveNext and Dispose on it results in 0, 2, 3:

try
{
  try
  {
    Write(0);
    yield break;
    Write(1);
  }
  finally
  {
    Write(2);
  }
  yield break;
}
finally
{
  Write(3);
}
yield break;

[jcouv]

if it's Dispose() then keep running until control leaves the iterator body.

Thanks for clarifying your proposal.
I see this kind of conditional semantic (two meaning for evaluating a given piece of syntax, sometimes exiting and sometimes passing through) as a major downside. It would be very surprising that the Write(); after a yield break; gets executed only if in disposal mode.
Moreover, I expect most users do not realize that code in finally can sometimes be executed as part of Dispose() in the first place.

I don't think the implementation would be that hard (mostly removing diagnostics).

Thinking more about my earlier comment, I had the design of async-iterators in mind. Iterators may actually be trickier. Would have to check.

[lcsondes]

It would be very surprising that the Write(); after a yield break; gets executed only if in disposal mode.

I've double-checked the behavior of yield break and I was wrong, it should execute finally blocks before returning from MoveNext() like this would: try{yield break;}finally{Write(1);}. I'm not after special treatment but parity with existing yields and awaits in catch and finally.

Moreover, I expect most users do not realize that code in finally can sometimes be executed as part of Dispose() in the first place.

I can get behind this, but most users would use some variant of using or foreach to handle this for them anyway which would call Dispose(). Not calling Dispose() today means that your finally blocks aren't guaranteed to run either so that wouldn't be a new problem introduced by yield-await parity.

[jeroen-mostert]

This is made so much worse with async iteration and using declarations, since the compiler adds a lot more magic that becomes unavailable to you. I have a method of the form

IAsyncEnumerator<T> myEnumeration(CancellationToken cancellationToken) {
    await using var someAsyncDisposable = ...
    await using var someOtherAsyncDisposable = someAsyncDisposable(...);
    // etc.
    await foreach (var t in myRealEnumeratingThing(cancellationToken)) yield return t;
}

But here's the problem: inside the block, any code (both setup and enumeration) may throw a WeirdThirdPartyException on cancellation that I don't want to expose to the caller (it should become an OperationCanceledException instead).

The simple solution of wrapping the whole thing in a try .. catch of course doesn't work due to the above. As a result, I can pick between rewriting all of this code into long form (which is unwieldy, if still less unwieldy than biting the bullet and implementing IAsyncEnumerator explicitly complete with proper dispose logic), or adding yet another wrapper method that just takes care of the exception logic by unspooling the foreach and calling MoveNextAsync explicitly (which is what I ended up doing). Either of these approaches seem vastly inferior to just having the compiler do what it's there to do, that is, take care of the heavy lifting of implementing iterator methods. I'm not at all convinced that there's somehow confusion about the semantics and that this would warrant not doing it.

And just to head off the discussion: yes, of course the third-party code throwing WeirdThirdPartyException is the real problem and should be rewritten in an ideal world, but that's third-party code for you, and the compiler isn't required to add insult to injury.

[gafter]

The simple solution of wrapping the whole thing in a try .. catch of course doesn't work due to the above.

You can yield in a try block, so the simple solution should work just fine.

[jnm2]

@gafter They might be asking for yield break inside the catch block in that case. They could work around it by setting a flag and doing yield break outside the catch, or using break; if available.

[jeroen-mostert]

The simple solution of wrapping the whole thing in a try .. catch of course doesn't work due to the above.

You can yield in a try block, so the simple solution should work just fine.

try .. finally, yes. try .. catch, no.

error CS1626: Cannot yield a value in the body of a try block with a catch clause.

[lcsondes]

That seems even weirder of a restriction, what issue does a try-yield-catch have that a try-yield-finally does not? (and generally speaking, you can do most of your standard exception handling in iterators anyway as long as they're separated from yield but await is fine)

[jeroen-mostert]

try .. catch works fine if you don't combine it with yield, but of course the problem comes from blocks like

foo() {
    using var x = ..
    using var y = ..
    using var z = .. // use x and y
    yield ... // use z
}

If an error can happen creating x , y or z, and you'd like to do something else altogether if that's the case, you're looking at some really ugly rewrites (or a wrapper). I don't actually need to catch exceptions of the yield statement, of course, which is where this whole "this is not such a big deal" attitude might have come from inside the compiler -- the actual issue is getting the value I want to the yield statement without having to turn all the blocks upside down.

[lcsondes]

Yeah, that's the whole reason I've opened this issue - since C# 6 we know that breaking up methods in catch/finally blocks is in fact not impossible to implement, and most of the work involving the state machine transformation has been done at that time for awaits anyway.

[AartBluestoke]

I think that yield inside catch and yield inside finally are 2 different scenarios.

In a catch, control is currently inside your own code, and wishing to replace some of your own exceptions when you try'd something with a yield'd object makes sense.

In a finally, control is currently in a semi-uncontrolled state, potentially already returning via exception-throw, so for the same reason that "return" in finally doesn't make sense, a yield here doesn't make sense.

Implementation wise, i don't beleive it could be harder than having a using around an async, which we already have.

knowing we start at -100:

in non yielding code we have, and expect behaviour like the following:

    public string  M() {
        try{
            return f();
        } catch(Exception e){
            return "errorThing";
        } finally{
            // we don't expect this to work
            //return "thing-or-error-thing, but wait i'm alreadyReturning"
        }
    }
    string f(){return "thing";}

adding async stuff doesn't change this mental model

public class C {
   public async Task<string> M() {
        try{
            return await f();
        } catch(Exception e){
            return "errorThing";
        } finally{
            // i would expect the following to work, because i'm happy for another thread to clean this object up whenever someone has some spare cpu.
           await prepareForCleanup().ConfigureAwait(false);

            // we still don't expect return to work here
            //return "thing-or-error-thing, but wait i'm already returning"
        }
    }
    Task<string> f(){return Task.FromResult("thing");}
    Task prepareForCleanup(){return Task.CompletedTask;}
}

I believe this request would be helpful to avoid switching mental models for error handling on IEnumerables - to me it makes sense that this could/should result in either "thing" or "errorThing" just like the prior two examples.

   public IEnumerable<string> M() {
        try{
            yield return f(); // error CS1626: Cannot yield a value in the body of a try block with a catch clause
        } catch(Exception e){
            yield return "errorThing"; //error CS1631: Cannot yield a value in the body of a catch clause
        } finally{
            // we still don't expect return to work here
            //yield return "thing-or-error-thing, but wait i'm already returning"
        }
    }
    string f(){return "thing";}

it would make sense that yield return can be in the same place as return.
that we had to revert to "conceptual flow about who is calling who" above, with confusion, amongst a (what i assume to be a ) more-technical-than-average audience indicates that we need a "pit of success" where things "just work"

To me the third example isn't ambiguous, even when understanding how IEnumerable works with the coceptual flow - somebody calls "MoveNext", we call f(), and if I exception then the errorThing is what we get back, the state machine isn't particularly complex IMO.

This could be the same as the following ,but with no leakage of the boolean, no issues to do with the early assignment of result to null to avoid the "use of not assigned" error, and minimising the extra code complexity

 public IEnumerable<string> M() {
       string result=null;
       Boolean isError = false;
        try{
            result =  f();
        } catch(Exception e){
            isError=true;
        } finally{
            // we still don't expect return to work here
            //yield return "thing-or-error-thing, but wait i'm already-returning"
        }
       if(isError){
           yield return "errorThing";
       } else {
           yield return result;
       }
    }    

[lcsondes]

I can get behind not allowing it in finally - due to the parity of the return you mentioned, but also that it just complicates the MoveNext()/Dispose() issue as it was discussed waay above (TL;DR Dispose() is void so even currently if you Dispose() of your iterator halfway through any yield returns just get skipped like if an invisible exception was thrown through your code). Fair enough, more trouble than it's worth, and likely confusing. I'll go and edit the top comment.

I'd like to highlight one thing from your examples though. Unless I'm missing some odd peculiarity of the language spec, if a catch happens in an iterator block, it always happens "synchronously" just like it would in a regular method: the only possible time when an exception can happen inside an iterator is if it's actively running. There's no expectation, or even the possibility for that matter for a method to somehow deal with exceptions being thrown from its caller, with return or yield return in C# today. We however do have a hint of "caller exception handling" in the form of a well-behaved caller using its own try-finally to call Dispose() on the iterator if something bad happened, but this is voluntary, not guaranteed, and already there in the language today.

It looks like we somehow got the more difficult part of this feature with finally that relies on outside support to even execute in some cases, and the straightforward part was missed.

[AartBluestoke]

many applications (eg, web api's that fetch by block, database queries, csv parsers) .... work by IEnumerable.
I think that over the last few years the need for this language features has not decreased.

@lcsondes - would this request include yield in 'try' as well as 'catch' ?

[lcsondes]

Yes.

[markusschaber]

Now, what's the current state here? Discussion seems to have reached some kind of consent 4 years ago, and then everything fell asleep. :-)

We have some iterator which internally loops, and has to handle exceptions (as it should handle the exception and continue looping and yielding, instead of just letting the exception bubble outside). And currently, we have to code around this limitation.
(We need to yield in the try and sometimes in the catch part of try/catch, but never in the finally part (when existing).)

[CyrusNajmabadi]

Now, what's the current state here?

Nothing has changed here. No one is championing the issue.

[jaredpar]

Got a bit nerd sniped into thinking about this feature earlier this week. Paged enough of the problem space in that I wrote a proposal for it. Appreciate any feedback.

#8413