Crystal: about Exception return

[lgphp]

in follow code, run it, should print:

throw a error
2

but ,i run it ,print many errors

[0x5892dd] main +18893
[0x7f64f89cb830] __libc_start_main +240
[0x5562e1] ???

```ruby/crystal
module TestException
extend self

puts main

def main
begin
test_ex(3)
rescue ex
puts ex.message
return 0
ensure
puts "finally"
return 2
end
return 1
end

def test_ex(a)
if (a > 1)
raise Myex.new("throw a error")
end
end
end

class Myex < Exception
def initialize(@message )
super(@message)
end
end

```

[asterite]

It's a compiler bug, the error you get is not at runtime, try crystal build program.cr.

Reduced:

def foo
  begin
    raise "throw a error"
  rescue ex
    return 0
  ensure
    return 2
  end
end

foo

[asterite]

Also:

def foo
  return 0
ensure
  return 2
end

foo

[bew]

@asterite what should be the correct behavior?

[bcardiff]

I was going to say that I would find more intuitive what c# does in this case: not allowing return in ensure blocks. My first impression is to doubt about the semantics in this case.

I've never used that pattern.

@MakeNowJust is having fun with the compiler though.

Any insight where this pattern is particularly useful?

I'm fine keeping ruby semantics, but not just because of it.

[straight-shoota]

I think it comes just natural: The ensure block follows after the begin and rescue blocks both lexical and in the execution sequence. So it would also be arguable to use the last expression of the ensure block as return value. Though this is probably not intended in most cases.

But it should at least be possible to change the return value explicitly from the ensure block. This can be necessary for example when you're returning error objects instead of exceptions. If an error is only detected in a cleanup task in the ensure block there would be no other direct way to communicate that. An other use case would be enhancement of return values with for examples statistics (like time spent) in case of normal execution as well as when a rescue block was triggered.

While being a different concept, deferred functions in Go have a similar function and also allow do modify named return values.

[bcardiff]

My conflict comes from the premise that return finalize the execution of the current function. For sure surroundings ensures are executed. You mentally track that cleanup blocks for deferred execution. But mixing them introduce the concept of overriding the returning value, whether otherwise once a return in reach the return value is determined.

@straight-shoota you mean something like this?:

def foo
  begin
    return :ok
  ensure
    begin
      clean_up_that_might_raise
    rescue
      return :err
    end
  end
end

I think the following could work also I think and it doesn't need the return in ensure.

def foo
  begin
    begin
      return :ok
    ensure
      clean_up_that_might_raise
    end
  rescue
    return :err
  end
end

But the first comes a bit cleaner maybe.

[straight-shoota]

Yes, or even simpler:

def foo 
  begin
    return :ok
  ensure
    return :err unless clean_up_that_might_detect_fail
  end
end

Of course, this could also be written in a different way, but that would sacrifice expressiveness and with some more complex rescue and ensure operations lead to unnecessary code duplication.

Even if the ensure block could not change the return value, it can always raise (and I don't assume you'd want to change that). Therefore the return value from the main body is not as determined as it might seem anyway, because it could still be surpassed by an exception.
If the ensure block raises, this also overwrites an uncatched exception from the main block. This might also be bad, because you loose the initial failure and catch one that might just be a result from that. And anyway, you should always keep the ensure block as simple as possible. Most of the time you will never need to change a return value or raise (and possibly overwrite) an exception, but I'd argue that it should be possible and is helpful in some cases.

[faultyserver]

I've been thinking about this over the course of the past day, and my instinct is to disallow returns in ensure blocks. I'm not entirely sure why, but @bcardiff's point that return should finalize the execution of the function seems like a big part of it.

To @straight-shoota's point: when a method raises, I see it as essentially panicking up the stack, bypassing return entirely. With that, I wouldn't agree that the return value isn't "determined", rather that the return value is not _guaranteed to be present_. I think that logic also holds with why having a return in a rescue block is valid. Whatever raise brought execution to the rescue did not set a return value, and so rescue has the authority to set one.

In the case of ensures that may raise, I had a quick thought of allowing rescues after an ensure, but I'm pretty skeptical of using it in practice:

def foo 
  return :ok
ensure
  raise :not_ok
rescue
  return :err
end

While this looks very clean, I don't like that ensure is no longer always the final branch of a begin/rescue/else/ensure stack, so I wouldn't suggest it as a good solution. To that extent, I would almost argue that an un-rescued raise in an ensure should also be disallowed.

---

If ensure is allowed to explicitly override the return value of the function, I think it should necessarily take an argument that is the current return value. I think this would make it more obvious that the return value may be changed by the block. As an example:

def foo
  return :ok
ensure
  return :ensured # => invalid, `ensure` does not receive a return-value argument.
end

def foo
  return :ok
ensure result
  return :ensured # valid, `result` is the return value at time-of-entry.
end

To me, this syntax makes it clear that any return value _may_ be modified, but also ensures that it will _not_ be modified unless ensure specifies the argument (Note: ensure without an argument would still be valid, but would guarantee an unchanged return value. See examples below)

Going further, implicit return values would be more trustworthy. If an ensure does _not_ take an argument, then any implicit return value will be ignored (and explicit returns are considered illegal). If the ensure _does_ take an argument, then the return value will be whatever comes from the ensure block, be it implicitly or explicitly set. Another example:

def foo
  :ok
ensure
  :err
end

foo # => :ok

def foo
  return :ok
ensure
  return :err # invalid
end

def foo
  return :ok
ensure result
  :err
end

foo #=> :err

def foo
  return :ok
ensure result
  result
end

foo #=> :ok

[straight-shoota]

I've thought about the argument, too. But I find a return statement is already explicit enough and I don't think that an ensure block should allow an implicit return value, even with such an argument.
I believe it would even encourage the use of ensure for return value modification (which is probably not the best thing to do in most cases), if it would take the return value of the main block as an argument and return implicitly.

To that extent, I would almost argue that an un-rescued raise in an ensure should also be disallowed.

Well, what would you do to ensure to run code that might raise? If you rescue it, can you raise from the rescue block? Or can you change the return value from there?

[faultyserver]

Well, what would you do to ensure to run code that might raise?

It would still allow raising in the ensure, but they would have to be rescued (as bcardiff's first example shows). This would then preserve any raise from the previous clauses, and would force users to handle those raises explicitly. If a cleanup raises and it is not recoverable, is there a case where the appropriate solution wouldn't be aborting execution in some way?

I'm really not sure of a good solution here, so I'm kind of just throwing out ideas for now.

If you rescue it, can you raise from the rescue block?

I believe (I know it is in Ruby, but can't test Crystal atm) that rescue is cascading, i.e., raising in a rescue is also rescuable by any following rescue clauses (hence the thought of having rescues after ensure, but I find that particular use more confusing than anything else).

[faultyserver]

This article ("Ensure With Explicit Return") by Les Hill has some interesting insights into how this works in Ruby. I recommend reading the whole thing, but here are the bits I found most important:

The second thing to note is that the explicit return statement acts as an implicit rescue clause, allowing the code to resume as if no exception had been raised.

Consider the following code:

def some_method
  thing.method_that_might_raise!
rescue Exception
  # we have rescued all possible exceptions
ensure
  return thing
end

Line 3 is a code smell. Rescuing all exceptions is not desirable. From our exploration of ensure we can see that this code is the equivalent of the original code [no rescue clause, just the explicit return].

Can we refactor it? Yes. Yes we can.

def some_method
  begin
    thing.method_that_might_raise!
  rescue SomeAppException => e
    # do something clever here
  end
  thing
end

And when we cannot recover from SomeAppException, we just let the exception propagate up the call stack:

def some_method
  thing.method_that_might_raise!
  thing
end

These refactors seem like the proper way to handle these situations. If cleaning up after thing could raise an error, it should be put in it's own begin...end block, rather than getting tucked into an ensure.

[ysbaddaden]

My gut feeling is with @bcardiff: we shouldn't return, neither implicitely or explicitely from ensure blocks.

[MakeNowJust]

I agree with @bcardiff and @ysbaddaden. We shouldn't use return inside ensure, also not use break and next. (break and next have same problem, do you know?)

In the last 2 days I surveyed ensure (or finally) semantics of some language. Conclusion:

• Most languages (Ruby, Python, Java, JavaScript and more) allow to use return, break and next (or continue) inside ensure. However lint tools like rubocop and eslint have a rule to detect it, and this rule is turned on by default in many case.
• Scala allow it, but the compiler make a warning when finally block has no side effect like: try { 1 } finally { 2 }.
• C# disallow to use it.

I think most languages implement return inside ensure conventionally (without considering, no reason, idolatrously), however it has no use case in fact. If we allow it, but later a lint tool disallow it anyhow. Thus, I think that.

Remaining problem of exception handling is raising an exception from inside ensure, but it is independent from this issue (I may open a new issue for it). Thank you.

[lgphp]

I run Java program, found Java has implemeted return insidefinally, then ,I am writing similar codes in Crystal at onece, Crystal throw some bugs. I think, most languages has implemented that has benefit, golang has defer implement as this function

[MakeNowJust]

Go expresses an error as returning value. In many cases, Go's modifying returning value inside defer is used to fix up returning value. It seems like re-raising a wrapped exception from rescue in Crystal. I think it is wrong manner to modify returning value not to mean an error in Go.

[straight-shoota]

In Go return values are the only way to propagate errors. In Crystal we have exceptions, but there are still valid reasons to use errors as return values. With tuples and unpacking this can be done much like the way Go does. While I usually prefer exceptions, I still find it quite nice that Crystal allows both.
That is, even if your application uses return values, you'd still have to handle exceptions from stdlib.

The more I think about it, I find it more apealing to follow the reasoning that return values from ensure blocks are not really useful. All discussed use cases can be written without it and while that might mean a bit more characters, it's probably helpful to understand the code if ensure blocks are limited to the minimum that needs to be ensured. Return values don't need to be ensured to run, they are only valid if there has been no exception before.

@lgphp Java and Go and even Ruby are different languages from Crystal, you can't assume that similar concepts have exactly the same mechanics in different languages.

[MakeNowJust]

Is there any progress?

[MakeNowJust]

Why is this opened still? I think it is fixed by #4486.

[RX14]

@MakeNowJust thanks for the catch