ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block.
@ 2021-08-18  3:03 ioquatix (Samuel Williams)
  2021-08-18  3:09 ` [ruby-core:104962] " ioquatix (Samuel Williams)
                   ` (30 more replies)
  0 siblings, 31 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-18  3:03 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been reported by ioquatix (Samuel Williams).

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104962] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
@ 2021-08-18  3:09 ` ioquatix (Samuel Williams)
  2021-08-18 17:04 ` [ruby-core:104971] " Eregon (Benoit Daloze)
                   ` (29 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-18  3:09 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


By the way, perhaps we should consider deprecating `$!` since it's usage can lead to incorrect behaviour.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93324

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104971] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
  2021-08-18  3:09 ` [ruby-core:104962] " ioquatix (Samuel Williams)
@ 2021-08-18 17:04 ` Eregon (Benoit Daloze)
  2021-08-18 17:09 ` [ruby-core:104972] " Eregon (Benoit Daloze)
                   ` (28 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-08-18 17:04 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


The second snippet seems clear, the first one much less.
In what cases `error` is set? If it's a StandardError? If it's an Exception? If it's any kind of "Ruby exception"?

IMHO this pattern is rarely enough needed that the second snippet is good enough.
If you need other `rescue`, you can add them in a nested begin/end, or in a different method, no?

BTW I think the second snippet only differentiates `Exception` and put together regular execution and control flow exit, is that intended?

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93336

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104972] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
  2021-08-18  3:09 ` [ruby-core:104962] " ioquatix (Samuel Williams)
  2021-08-18 17:04 ` [ruby-core:104971] " Eregon (Benoit Daloze)
@ 2021-08-18 17:09 ` Eregon (Benoit Daloze)
  2021-08-18 23:04 ` [ruby-core:104980] " ioquatix (Samuel Williams)
                   ` (27 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-08-18 17:09 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


If you only need to know if there is an `Exception` this seems easier:
Using the example from https://bugs.ruby-lang.org/issues/15567#note-27
```ruby
begin
  ...
rescue Exception => exception
  transaction.abort
ensure
  transaction.commmit unless transaction.aborted?
end

# or

begin
  ...
rescue Exception => exception
  aborted = true
  transaction.abort
ensure
  transaction.commmit unless aborted
end

# or 

begin
  ...
rescue Exception => exception
  transaction.abort
ensure
  transaction.commmit unless exception
end
```

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93337

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104980] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (2 preceding siblings ...)
  2021-08-18 17:09 ` [ruby-core:104972] " Eregon (Benoit Daloze)
@ 2021-08-18 23:04 ` ioquatix (Samuel Williams)
  2021-08-19  0:45 ` [ruby-core:104981] " duerst
                   ` (26 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-18 23:04 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


There are lots of ways to achieve this but the reality is there is a lot of code which uses `$!` in the ensure block which is incorrect for the reasons already explained.

Your proposed ideas are fine, except that RuboCop will warn you against `rescue Exception`. I agree with the general sentiment around this.

That being said, there are definitely valid use cases where your ensure block wants to know whether it's exiting via a normal code flow or an exceptional one. To answer your question, `ensure => error`, error would be the equivalent of a local `$!`, so nothing really changes, we are just providing a mechanism to do correctly and safely what people are already doing with `$!`.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93380

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104981] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (3 preceding siblings ...)
  2021-08-18 23:04 ` [ruby-core:104980] " ioquatix (Samuel Williams)
@ 2021-08-19  0:45 ` duerst
  2021-08-19  0:51 ` [ruby-core:104982] " ioquatix (Samuel Williams)
                   ` (25 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: duerst @ 2021-08-19  0:45 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by duerst (Martin Dürst).


Eregon (Benoit Daloze) wrote in #note-3:
> If you only need to know if there is an `Exception` this seems easier:
> Using the example from https://bugs.ruby-lang.org/issues/15567#note-27
> ```ruby
> begin
>   ...
> rescue Exception => exception
>   transaction.abort
>   raise exception
> ensure
>   transaction.commmit unless transaction.aborted?
> end
> ```

What about
```ruby
begin
  ...
rescue Exception
  transaction.abort
else
  transaction.commit
end
```
It seems shorter and easier to understand.

Having read through most of #15567, my impression is that:
- Ruby already has too many features for exceptional flow control. Adding more stuff that makes this even more complicated doesn't look like an improvement of the language. (Go To Statement Considered Harmful, anybody?)
- All the documentation/books that I can remember explain Exceptions and catch/throw completely separately (although usually in close proximity, because both features don't really fit anywhere). Some improvement seems in order, even if this is only "don't use them together".
- `ensure` contains always executed code. Adding conditions to its syntax seems strange. It would be similar to extending the syntax of `else` (both in `if` expressions as well as in `begin` blocks). Specific conditions should be handled where we already have them, on `rescue`, e.g. like `rescue when throw` or some such (if at all).
- The argument that Rubocop complains about something isn't really an argument. If Rubocop is wrong, it should be fixed.


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93381

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104982] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (4 preceding siblings ...)
  2021-08-19  0:45 ` [ruby-core:104981] " duerst
@ 2021-08-19  0:51 ` ioquatix (Samuel Williams)
  2021-08-19  1:00 ` [ruby-core:104984] " ioquatix (Samuel Williams)
                   ` (24 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-19  0:51 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


> Ruby already has too many features for exceptional flow control. Adding more stuff that makes this even more complicated doesn't look like an improvement of the language. (Go To Statement Considered Harmful, anybody?)

We are not implementing any additional flow control.

> ensure contains always executed code. Adding conditions to its syntax seems strange. It would be similar to extending the syntax of else (both in if expressions as well as in begin blocks). Specific conditions should be handled where we already have them, on rescue, e.g. like rescue when throw or some such (if at all).

Whether or not you agree with it, people already do it: https://bugs.ruby-lang.org/issues/15567#note-26

I want to make this use case safer, as I was bitten by this bug. The original discussion is a bit different but the surrounding context is similar.

> The argument that Rubocop complains about something isn't really an argument. If Rubocop is wrong, it should be fixed.

`rescue Exception` can produce unusual results that users might not expect, so I think RuboCop is correct here and we shouldn't encourage users to write `rescue Exception`.


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93382

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:104984] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (5 preceding siblings ...)
  2021-08-19  0:51 ` [ruby-core:104982] " ioquatix (Samuel Williams)
@ 2021-08-19  1:00 ` ioquatix (Samuel Williams)
  2021-08-19  8:04 ` [ruby-core:105001] " ioquatix (Samuel Williams)
                   ` (23 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-19  1:00 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


Your proposed "shorter and easier to understand" implementation is actually incorrect:

```
def transaction1
  begin
    yield
  rescue Exception => error
  ensure
    if error
      puts "abort"
    else
      puts "commit"
    end
  end
end

def transaction2
  begin
    yield
  rescue Exception
    puts "abort"
  else
    puts "commit"
  end
end

catch(:foo) do
  transaction1 do
    throw :foo
  end
end
```

Try for `transaction1` and `transaction2`. The flow control is wrong for `transaction2`.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93384

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105001] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (6 preceding siblings ...)
  2021-08-19  1:00 ` [ruby-core:104984] " ioquatix (Samuel Williams)
@ 2021-08-19  8:04 ` ioquatix (Samuel Williams)
  2021-08-19  8:19 ` [ruby-core:105002] " matz (Yukihiro Matsumoto)
                   ` (22 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-19  8:04 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


We want to make it easy for people to write robust Ruby programs, even if there is failure. As discussed in <https://bugs.ruby-lang.org/issues/15567> there are some tricky edge cases when using `$!` because it's non-local state. As an example:

``` ruby
def do_work
  begin # $! may be set on entry to `begin`.
    # ... do work ...
  ensure
    if $!
      puts "work failed"
    else
      puts "work success"
    end
  end
end
 
begin
  raise "Boom"
ensure
  # $! is set.
  do_work
end
```

There are real examples of this kind of code: https://bugs.ruby-lang.org/issues/15567#note-26

There is another kind of error that can occur:

``` ruby
begin
  raise "Boom"
rescue => error
  # Ignore?
ensure
  pp error # <RuntimeError: Boom>
  pp $! # nil
end
```

As a general model, something like the following would be incredibly useful, and help guide people in the right direction:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can almost achieve this:

``` ruby
begin
  ...
rescue Exception => error # RuboCop will complain.
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other rescue clause. Otherwise, it may not work as expected or require extra care (e.g. care with `rescue ... => error` naming. Also, Rubocop will complain about it, which I think is generally correct since we shouldn't encourage users to `rescue Exception`.

Because `$!` can be buggy and encourage incorrect code, I also believe we should consider deprecating it. But in order to do so, we need to provide users with a functional alternative, e.g. `ensure => error`.

Another option is to clear `$!` when entering `begin` block (and maybe restore it on exit?), but this may break existing code.

#### More Examples

```
Gem: bundler has 1480241823 downloads.
/lib/bundler/vendor/net-http-persistent/lib/net/http/persistent.rb

  ensure
    return sleep_time unless $!

Gem: rubygems-update has 817517436 downloads.
/lib/rubygems/core_ext/kernel_require.rb

  ensure
    if RUBYGEMS_ACTIVATION_MONITOR.respond_to?(:mon_owned?)
      if monitor_owned != (ow = RUBYGEMS_ACTIVATION_MONITOR.mon_owned?)
        STDERR.puts [$$, Thread.current, $!, $!.backtrace].inspect if $!

Gem: launchy has 196850833 downloads.
/lib/launchy.rb

    ensure
      if $! and block_given? then
        yield $!

Gem: spring has 177806450 downloads.
/lib/spring/application.rb

          ensure
            if $!
              lib = File.expand_path("..", __FILE__)
              $!.backtrace.reject! { |line| line.start_with?(lib) }

Gem: net-http-persistent has 108254000 downloads.
/lib/net/http/persistent.rb

  ensure
    return sleep_time unless $!

Gem: open4 has 86007490 downloads.
/lib/open4.rb

            ensure
              killall rescue nil if $!

Gem: net-ssh-gateway has 74452292 downloads.
/lib/net/ssh/gateway.rb

    ensure
      close(local_port) if block || $!

Gem: vault has 48165849 downloads.
/lib/vault/persistent.rb

  ensure
    return sleep_time unless $!

Gem: webrick has 46884726 downloads.
/lib/webrick/httpauth/htgroup.rb

        ensure
          tmp.close
          if $!

Gem: stripe-ruby-mock has 11533268 downloads.
/lib/stripe_mock/api/server.rb

        ensure
          raise e if $! != e

Gem: logstash-input-udp has 8223308 downloads.
/lib/logstash/inputs/udp.rb

  ensure
    if @udp
      @udp.close_read rescue ignore_close_and_log($!)
      @udp.close_write rescue ignore_close_and_log($!)

Gem: shrine has 6332950 downloads.
/lib/shrine/uploaded_file.rb

      ensure
        tempfile.close! if ($! || block_given?) && tempfile

```

Discussion:
* nobu: `$!` is thread-local.
* ko1: Your proposal is to make shorthand for the following code?
  * mame: No

```ruby=
begin
 ...
ensure
  error = $!
  ...
end
```

* ko1: FYI

```ruby=
def foo
  p $! #=> #<RuntimeError: FOO>
  raise SyntaxError
rescue SyntaxError
end

begin
  raise 'FOO'
ensure
  foo; p $! #=> #<RuntimeError: FOO>
  exit
end
```

* mame: It is surprising to me that `$!` is not a method-local but a thread-local variable

```ruby=
at_exit do # |exception| ???
  if $! # exception
    puts "Exiting because of error"
  end
end
```

```ruby=
begin
  raise "Boom"
ensure
  begin
  ensure => error # should be nil - yes correct. This is the difference between ko1's workaround and samuel's proposal
    p $! #=> #<RuntimeError: Boom>
    error = $!
    puts "failed" if $!
  end
end
```

* knu:
```ruby=
begin
  Integer("foo")
rescue
  begin
    Integer("1")
  ensure => error
    # You can't tell `$!` came from this begin block without being rescued.
    p $!  #=> #<ArgumentError: invalid value for Integer(): "foo">
    p error #=> nil
  end
end
```

You can almost achieve the ideal situation with this:

``` ruby
begin
  ...
rescue Exception => error # RuboCop will complain.
  raise
ensure
  pp "error occurred" if error
end
```

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93400

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105002] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (7 preceding siblings ...)
  2021-08-19  8:04 ` [ruby-core:105001] " ioquatix (Samuel Williams)
@ 2021-08-19  8:19 ` matz (Yukihiro Matsumoto)
  2021-08-19  9:57 ` [ruby-core:105005] " ioquatix (Samuel Williams)
                   ` (21 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: matz (Yukihiro Matsumoto) @ 2021-08-19  8:19 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by matz (Yukihiro Matsumoto).


I hesitate to enhance `ensure`. It's ugly and against the basic concept of `ensure`.
The `ensure` behavior was taken from `unwind-protect` macro of Lisp, and it does not distinguish between error execution and normal execution. If you check the error condition in the `ensure` clause, especially using `$!`, I doubt you misuse the `ensure` concept.

If you really wanted to check the error condition, you may want to use local variables.

```ruby
begin
  ....
  no_error = true
ensure
  error_handling unless no_error
end
```

At the same time, I may underestimate the pain of OP. Let me think about it for a while.

Matz.


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93402

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105005] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (8 preceding siblings ...)
  2021-08-19  8:19 ` [ruby-core:105002] " matz (Yukihiro Matsumoto)
@ 2021-08-19  9:57 ` ioquatix (Samuel Williams)
  2021-08-19 16:42 ` [ruby-core:105013] " Eregon (Benoit Daloze)
                   ` (20 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-19  9:57 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


@matz I agree with your general feeling completely. But it's too late: We already have "ugly and against the basic concept of ensure" in the form of `$!`. As demonstrated, people are checking `$!` and unfortunately, in general code, it is not safe to check `$!` in an `ensure` block because it might come from the parent scope (or some other scope). This puts us in a difficult situation.

I think the decision depends on whether you want to support or deprecate the usage of `$!`. Fundamentally, if you think that `ensure => error` is ugly, I believe you must agree `ensure ... if $!` is equally ugly.

## Support `$!`

If we want to support $!, I would propose that we make it safe, by limiting it to the current block. That means it's effectively reset on entering a `begin...ensure...end` block, so we prevent it carrying state from the parent scope (if any).

## Deprecate `$!`

If we want to deprecate $!, I would propose that we consider how we want engineers to approach the problem they are currently depending on `$!` for.

The most common use case is something like this:

```ruby
begin
  resource = acquire # get a resource from the pool or allocate one if pool is empty.
  yield resource
ensure
  if $!
    free(resource) # resource is broken, get rid of it.
  else
    release(resource) # resource is okay, return it to the pool.
  end
end
```

The proposal to use `else` is a good one, except it doesn't capture all flow controls:

```ruby
begin
  resource = acquire # get a resource from the pool or allocate one if pool is empty.
  yield resource
rescue Exception
  free(resource) # resource is broken, get rid of it.
else
  release(resource) # resource is okay, return it to the pool.
end
```

The problem with the above code, is there are some cases where you can exit the block and neither the rescue nor the else block are executed. It will result in a resource leak. Also, some people would argue that `rescue Exception` is wrong. So, can we make the above better? If we make `else` the same as "ensure without exception" then it would be sufficient, but right now it's not.

The next best alternative is something like this:

```ruby
begin
  resource = acquire # get a resource from the pool or allocate one if pool is empty.
  yield resource
rescue Exception => error
  free(resource) # resource is broken, get rid of it.
  raise
ensure
  unless error
    release(resource) # resource is okay, return it to the pool.
  end
end
```

So the fundamental problem we have is that we don't have any way to guarantee, AFAIK, on error execute this code, otherwise execute this other code, and it turns out that a lot of code needs this, essentially "ensure if no exception was raised" or "ensure if no rescue block was executed", or something similar.

By the way, your proposed solution also falls short:

~~~
catch(:finished) do
  begin
    throw :finished
    no_error = true
  ensure
    p :error_handling unless no_error
  end
end
~~~

Will execute `error_handling`. This is why it's a non-trivial problem. Everyone comes up with the wrong answer (including me). If you look at real code, you see these incorrect patterns repeated over and over.


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93405

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105013] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (9 preceding siblings ...)
  2021-08-19  9:57 ` [ruby-core:105005] " ioquatix (Samuel Williams)
@ 2021-08-19 16:42 ` Eregon (Benoit Daloze)
  2021-08-20  1:09 ` [ruby-core:105019] " ioquatix (Samuel Williams)
                   ` (19 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-08-19 16:42 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


I think use-cases needing separate cleanups whether there is a Ruby exception or "normal execution or non-local control flow" will always be ugly, fundamentally because they do something hacky and approximative:
the exception that happens inside is not a proof the resource is at fault, it could be anything, including a NoMemoryError or a SystemStackError for instance (which e.g. could be due to a bug in the block, not related to the resource at all).

So in general, there should be one call to e.g. `close` in `ensure` and code should not attempt to differentiate (e.g., what happens for `File.open {}`).

For the rare cases which do need to differentiate, I think `rescue Exception => error` + `if error` is fine.
You literally want to test "any Ruby exception" or "any other exit from this block", and then `rescue Exception` just makes sense.
It's true one should avoid `rescue Exception` in general, or at the very least make sure it's always re-raised to not swallow important and possibly unrelated errors.
So RuboCop is right to warn for such usages as they should be avoided in general, and it's worth double-checking if one really needs to `rescue Exception`.
But as every rule, there are exceptions, and this is a good exception to that rule, if you literally want to differentiate "any Ruby exception" or "any other exit from this block", then `rescue Exception` is *exactly* what one wants, just go ahead and suppress the RuboCop warning.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93416

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105019] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (10 preceding siblings ...)
  2021-08-19 16:42 ` [ruby-core:105013] " Eregon (Benoit Daloze)
@ 2021-08-20  1:09 ` ioquatix (Samuel Williams)
  2021-08-20 22:00 ` [ruby-core:105033] " Eregon (Benoit Daloze)
                   ` (18 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-20  1:09 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


> the exception that happens inside is not a proof the resource is at fault, it could be anything, including a NoMemoryError or a SystemStackError for instance (which e.g. could be due to a bug in the block, not related to the resource at all).

Yes, it's a fair assessment. But the key point is there is a different from normal exit (which can include `throw`) and exceptional exit (exception was raised during user code). The reality is, as shown by the examples, lots of code does need to make this differentiation.

> You literally want to test "any Ruby exception" or "any other exit from this block", and then rescue Exception just makes sense.

Actually, I'm less concerned about the "any Ruby exception" and more concerned with splitting execution between "normal code flow" and "exceptional code flow", and even that is just as an example. Clearly, lots of people **are** using `$!` in an unsafe way, and that's the key point I want to fix. My initial proposal is one way to fix it, by introducing explicit `ensure => error`. However, I agree with Matz, that it can be considered kind of ugly. But if that's true, why don't we get rid of `$!`, surely every case for `$!` can be better handled by explicit `rescue`? If not, why is `$!` necessary? If we can answer that question, then I think we will know what path to take.

Personally, I'd vote for the path to get rid of `$!` because I do agree with Matz that its fundamentally pretty ugly both sematically, as well as practically. I looked at a lot of code that uses `$!` and it's often used as a poor replacement for the implicit exception `cause` argument, or for implicit passing error information between code where it should really be explicit.


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93421

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105033] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (11 preceding siblings ...)
  2021-08-20  1:09 ` [ruby-core:105019] " ioquatix (Samuel Williams)
@ 2021-08-20 22:00 ` Eregon (Benoit Daloze)
  2021-08-20 22:03 ` [ruby-core:105034] " ioquatix (Samuel Williams)
                   ` (17 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-08-20 22:00 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


> lots of code does need to make this differentiation.

A few places, but in relative usages probably 1/1000 of begin/rescue or ensure/end patterns, right?

Agreed `$!` should be deprecated and then removed.
The only usage of `$!` I know of which can't be trivially replaced by `=> exc` is `p((abc rescue $!))` and that's 1) unpretty (double parens, modifier rescue) 2) only good for debugging / printing an exception in a quick script.
It's easy enough to `begin; expr; rescue => exc; p exc; end` (or better, on multiple lines).

Changing `$!` to be thread+frame local would likely be too incompatible so I think deprecating + removing it is better.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93442

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105034] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (12 preceding siblings ...)
  2021-08-20 22:00 ` [ruby-core:105033] " Eregon (Benoit Daloze)
@ 2021-08-20 22:03 ` ioquatix (Samuel Williams)
  2021-08-20 22:09 ` [ruby-core:105035] " Eregon (Benoit Daloze)
                   ` (16 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-20 22:03 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


@matz based on the above discussion, do you agree to deprecate and remove `$!` if possible?

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93443

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105035] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (13 preceding siblings ...)
  2021-08-20 22:03 ` [ruby-core:105034] " ioquatix (Samuel Williams)
@ 2021-08-20 22:09 ` Eregon (Benoit Daloze)
  2021-08-21  0:52 ` [ruby-core:105036] " ioquatix (Samuel Williams)
                   ` (15 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-08-20 22:09 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


BTW, deprecating and removing `$!` has another advantage:
a raised Ruby exception wouldn't immediately leak to the current thread/fiber and more optimizations might be possible, including skipping to compute the exception backtrace (the most expensive part of Ruby exceptions) if e.g. a JIT can prove the exception or its backtrace is never used.
That analysis would only require tracking usages of `exc` in `rescue => exc`, and not any call in the `rescue` that might use `$!`.

Of course, the related `$@` (which is `$?.backtrace`) should be deprecated and removed too.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93444

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105036] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (14 preceding siblings ...)
  2021-08-20 22:09 ` [ruby-core:105035] " Eregon (Benoit Daloze)
@ 2021-08-21  0:52 ` ioquatix (Samuel Williams)
  2021-10-19  4:46 ` [ruby-core:105667] " ko1 (Koichi Sasada)
                   ` (14 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-08-21  0:52 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


> A few places, but in relative usages probably 1/1000 of begin/rescue or ensure/end patterns, right?

I scanned top 10,000 gems by download count:

Checking for "$!":
Found 459 gems with this pattern.
Found 2624 instances of the pattern.

Checking for "ensure":
Found 3221 gems with this pattern.
Found 28306 instances of the pattern.

Checking for /(.*ensure(.*\n){0,4}.*\$!.*)/:
Found 35 gems with potential issues.
Found 46 instances of the pattern.

Any usage of $! is potentially suspect, but at least 35 gems out of 10,000 are definitely incorrect.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-93445

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105667] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (15 preceding siblings ...)
  2021-08-21  0:52 ` [ruby-core:105036] " ioquatix (Samuel Williams)
@ 2021-10-19  4:46 ` ko1 (Koichi Sasada)
  2021-10-19 17:51 ` [ruby-core:105682] " Eregon (Benoit Daloze)
                   ` (13 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ko1 (Koichi Sasada) @ 2021-10-19  4:46 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ko1 (Koichi Sasada).


Another proposal is change `$!` semantics to `begin/rescue/ensure' clause local.
Does it solve the issues?

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94166

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105682] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (16 preceding siblings ...)
  2021-10-19  4:46 ` [ruby-core:105667] " ko1 (Koichi Sasada)
@ 2021-10-19 17:51 ` Eregon (Benoit Daloze)
  2021-10-21  6:13 ` [ruby-core:105715] " ko1 (Koichi Sasada)
                   ` (12 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-10-19 17:51 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


ko1 (Koichi Sasada) wrote in #note-18:
> Another proposal is change `$!` semantics to `begin/rescue/ensure' clause local.
> Does it solve the issues?

That seems elegant actually, because 
```ruby
begin
  ...
rescue
  foo($!)
end
```
would behave like:
```ruby
begin
  ...
rescue => exc
  foo(exc)
end
```

And it would avoid escaping the Exception instance to the thread, which mean it can be escaped analyzed.
Also, if the `rescue` doesn't use `=>` or `$!`, there is no need to generate a backtrace which is a huge performance gain!
With current semantics that's difficult to know as any call inside the rescue clause might use `$!`.

I"m not sure regarding compatibility if e.g. `$!` is accessed inside a method called from the rescue clause. But that's probably rare and seems bad code, so I think fine to try/experiment.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94182

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105715] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (17 preceding siblings ...)
  2021-10-19 17:51 ` [ruby-core:105682] " Eregon (Benoit Daloze)
@ 2021-10-21  6:13 ` ko1 (Koichi Sasada)
  2021-10-21  6:28 ` [ruby-core:105716] " matz (Yukihiro Matsumoto)
                   ` (11 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ko1 (Koichi Sasada) @ 2021-10-21  6:13 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ko1 (Koichi Sasada).


Sorry, my proposal is introducing dynamic scope, not syntactical scope like:

```
def foo
  p $! #=> A
  begin
    # no raise
  ensure
    p $! #=> nil
  end
end

begin
  raise A
ensure
  foo
end
```

We can implement `raise` like method:

```ruby
def my_raise
  # DO SOMETHING
  raise $!
end
```


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94213

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105716] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (18 preceding siblings ...)
  2021-10-21  6:13 ` [ruby-core:105715] " ko1 (Koichi Sasada)
@ 2021-10-21  6:28 ` matz (Yukihiro Matsumoto)
  2021-10-21 10:24 ` [ruby-core:105727] " Eregon (Benoit Daloze)
                   ` (10 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: matz (Yukihiro Matsumoto) @ 2021-10-21  6:28 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by matz (Yukihiro Matsumoto).


First, I am not going to change the syntax for the `ensure` clause. The `ensure` clause is to unwind-protect so it is not recommended to branch depending on exceptions.

I admit the current `$!` behavior is error-prone. But considering compatibility, I am not sure we can fix the problem. For the workaround at the moment, avoid using `$!`. If it is possible to address the issue (e.g. @ko1's proposal of dynamic scoping or giving a warning for the bad usage), I agree with experimenting with them.

Matz.


----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94214

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105727] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (19 preceding siblings ...)
  2021-10-21  6:28 ` [ruby-core:105716] " matz (Yukihiro Matsumoto)
@ 2021-10-21 10:24 ` Eregon (Benoit Daloze)
  2021-10-21 11:15 ` [ruby-core:105728] " ioquatix (Samuel Williams)
                   ` (9 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Eregon (Benoit Daloze) @ 2021-10-21 10:24 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by Eregon (Benoit Daloze).


ko1 (Koichi Sasada) wrote in #note-20:
> Sorry, my proposal is introducing dynamic scope, not syntactical scope like:

Ah, I misunderstood.

Then I think deprecating `$!` is the right way, since that seems the only way to be able to analyze decently if an exception's backtrace is needed and do the important optimizations I mentioned in earlier comments.
This would help a lot for making Ruby exceptions faster.
I'm pretty sure @headius would agree since he did a similar optimization, but currently quite limited because of `$!` being accessible inside calls.

And of course it would avoid the problem of `$!` having confusing semantics, by telling users to use `rescue => exc` instead which is clear, simple and clean.

If needed maybe we can avoid warning for `rescue $!` since that's fairly harmless, but probably that's very rarely used in library/app code (i.e., only used in REPL and for trying things).

---

We could also warn whenever `$!` is not used syntactically in the `rescue` clause, but IMHO better deprecate it entirely.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94226

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105728] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (20 preceding siblings ...)
  2021-10-21 10:24 ` [ruby-core:105727] " Eregon (Benoit Daloze)
@ 2021-10-21 11:15 ` ioquatix (Samuel Williams)
  2021-10-21 11:17 ` [ruby-core:105729] " ioquatix (Samuel Williams)
                   ` (8 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-10-21 11:15 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


https://github.com/rubocop/rubocop/issues/10204

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94227

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105729] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (21 preceding siblings ...)
  2021-10-21 11:15 ` [ruby-core:105728] " ioquatix (Samuel Williams)
@ 2021-10-21 11:17 ` ioquatix (Samuel Williams)
  2021-10-21 13:00 ` [ruby-core:105733] " nobu (Nobuyoshi Nakada)
                   ` (7 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) @ 2021-10-21 11:17 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by ioquatix (Samuel Williams).


Regarding

``` ruby
foo rescue $!
```

Maybe we can improve this:

``` ruby
foo rescue => error
```

I'm not sure it's worth the effort...

Surprisingly this does work:

``` ruby
foo rescue $! => error
```

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94228

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:105733] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (22 preceding siblings ...)
  2021-10-21 11:17 ` [ruby-core:105729] " ioquatix (Samuel Williams)
@ 2021-10-21 13:00 ` nobu (Nobuyoshi Nakada)
  2023-12-14 19:40 ` [ruby-core:115747] " ioquatix (Samuel Williams) via ruby-core
                   ` (6 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: nobu (Nobuyoshi Nakada) @ 2021-10-21 13:00 UTC (permalink / raw)
  To: ruby-core

Issue #18083 has been updated by nobu (Nobuyoshi Nakada).


ioquatix (Samuel Williams) wrote in #note-24:
> Surprisingly this does work:
> 
> ``` ruby
> foo rescue $! => error
> ```

It is a pattern-matching.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-94232

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115747] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (23 preceding siblings ...)
  2021-10-21 13:00 ` [ruby-core:105733] " nobu (Nobuyoshi Nakada)
@ 2023-12-14 19:40 ` ioquatix (Samuel Williams) via ruby-core
  2023-12-14 20:08 ` [ruby-core:115748] " kddnewton (Kevin Newton) via ruby-core
                   ` (5 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) via ruby-core @ 2023-12-14 19:40 UTC (permalink / raw)
  To: ruby-core; +Cc: ioquatix (Samuel Williams)

Issue #18083 has been updated by ioquatix (Samuel Williams).


Here is a good example of why `$!` is dangerous:

https://github.com/ruby/net-ftp/pull/24

I believe the merged fix is also wrong, since flow control can exit without closing the connection.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105679

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115748] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (24 preceding siblings ...)
  2023-12-14 19:40 ` [ruby-core:115747] " ioquatix (Samuel Williams) via ruby-core
@ 2023-12-14 20:08 ` kddnewton (Kevin Newton) via ruby-core
  2023-12-15 14:46 ` [ruby-core:115752] " Dan0042 (Daniel DeLorme) via ruby-core
                   ` (4 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: kddnewton (Kevin Newton) via ruby-core @ 2023-12-14 20:08 UTC (permalink / raw)
  To: ruby-core; +Cc: kddnewton (Kevin Newton)

Issue #18083 has been updated by kddnewton (Kevin Newton).


If you're already using global variables, we could add another global variable that gets set in `ensure` blocks that determines if an error was raised. That way you could safely check `$!`. That wouldn't require a syntax change.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105680

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115752] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (25 preceding siblings ...)
  2023-12-14 20:08 ` [ruby-core:115748] " kddnewton (Kevin Newton) via ruby-core
@ 2023-12-15 14:46 ` Dan0042 (Daniel DeLorme) via ruby-core
  2023-12-15 20:15 ` [ruby-core:115755] " ioquatix (Samuel Williams) via ruby-core
                   ` (3 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: Dan0042 (Daniel DeLorme) via ruby-core @ 2023-12-15 14:46 UTC (permalink / raw)
  To: ruby-core; +Cc: Dan0042 (Daniel DeLorme)

Issue #18083 has been updated by Dan0042 (Daniel DeLorme).


ko1 (Koichi Sasada) wrote in #note-20:
> my proposal is introducing dynamic scope, not syntactical scope like:

I think this is a good idea, definitely an improvement over the current situation. `$!` is already always correct inside a `rescue` clause, and that would make it always correct inside the `ensure` clause. But I want to confirm if this would be the expected behavior:

```ruby
#1
p $! #=> A
begin
  # no raise
ensure
  p $! #=> nil (unlike A currently)
end
p $! #=> A (restored after previous begin-ensure block?)

#2
p $! #=> A
begin
  raise "B"
rescue 
  p $! #=> B
  #not re-raised
ensure
  p $! #=> B (unlike A currently)
end
p $! #=> A (restored after previous begin-ensure block?)
```

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105683

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115755] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (26 preceding siblings ...)
  2023-12-15 14:46 ` [ruby-core:115752] " Dan0042 (Daniel DeLorme) via ruby-core
@ 2023-12-15 20:15 ` ioquatix (Samuel Williams) via ruby-core
  2023-12-15 20:20 ` [ruby-core:115756] " jeremyevans0 (Jeremy Evans) via ruby-core
                   ` (2 subsequent siblings)
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) via ruby-core @ 2023-12-15 20:15 UTC (permalink / raw)
  To: ruby-core; +Cc: ioquatix (Samuel Williams)

Issue #18083 has been updated by ioquatix (Samuel Williams).


`$!` is non-trivial to implement - can have performance implications, and dynamic scope might make it more expensive. cc @Eregon @headius can we get your input?

Giving `$!` dynamic scope mitigates the problem but still seems to encourage an approach that @matz has said he was against. Specifically, he was against `ensure => exception` and this is just a variant of that approach.

In the first instance, the question we need to answer is: "Is there a valid use case for $! (assuming dynamic scope as proposed) that isn't better addressed using existing techniques?".

If the answer is YES, we should implement it.

If the answer is NO, we should deprecate and remove `$!`.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105693

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115756] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (27 preceding siblings ...)
  2023-12-15 20:15 ` [ruby-core:115755] " ioquatix (Samuel Williams) via ruby-core
@ 2023-12-15 20:20 ` jeremyevans0 (Jeremy Evans) via ruby-core
  2023-12-15 20:30 ` [ruby-core:115757] " ioquatix (Samuel Williams) via ruby-core
  2023-12-15 21:41 ` [ruby-core:115759] " Dan0042 (Daniel DeLorme) via ruby-core
  30 siblings, 0 replies; 32+ messages in thread
From: jeremyevans0 (Jeremy Evans) via ruby-core @ 2023-12-15 20:20 UTC (permalink / raw)
  To: ruby-core; +Cc: jeremyevans0 (Jeremy Evans)

Issue #18083 has been updated by jeremyevans0 (Jeremy Evans).


ioquatix (Samuel Williams) wrote in #note-29:
> In the first instance, the question we need to answer is: "Is there a valid use case for $! (assuming dynamic scope as proposed) that isn't better addressed using existing techniques?".
> 
> If the answer is YES, we should implement it.
> 
> If the answer is NO, we should deprecate and remove `$!`.

This perspective ignores backwards compatibility.  If the answer is NO, we would still need to determine if the benefit of removing `$!` exceeds the cost of the loss of backwards compatibility.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105694

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115757] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (28 preceding siblings ...)
  2023-12-15 20:20 ` [ruby-core:115756] " jeremyevans0 (Jeremy Evans) via ruby-core
@ 2023-12-15 20:30 ` ioquatix (Samuel Williams) via ruby-core
  2023-12-15 21:41 ` [ruby-core:115759] " Dan0042 (Daniel DeLorme) via ruby-core
  30 siblings, 0 replies; 32+ messages in thread
From: ioquatix (Samuel Williams) via ruby-core @ 2023-12-15 20:30 UTC (permalink / raw)
  To: ruby-core; +Cc: ioquatix (Samuel Williams)

Issue #18083 has been updated by ioquatix (Samuel Williams).


Giving `$!` dynamic scope is equally likely to cause subtle bugs, so no matter what we do here, there is a chance to cause compatibility issues. Deprecation is explicit and gives people a chance to fix the code, which in most cases that I reviewed, is buggy.

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105695

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [ruby-core:115759] [Ruby master Feature#18083] Capture error in ensure block.
  2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
                   ` (29 preceding siblings ...)
  2023-12-15 20:30 ` [ruby-core:115757] " ioquatix (Samuel Williams) via ruby-core
@ 2023-12-15 21:41 ` Dan0042 (Daniel DeLorme) via ruby-core
  30 siblings, 0 replies; 32+ messages in thread
From: Dan0042 (Daniel DeLorme) via ruby-core @ 2023-12-15 21:41 UTC (permalink / raw)
  To: ruby-core; +Cc: Dan0042 (Daniel DeLorme)

Issue #18083 has been updated by Dan0042 (Daniel DeLorme).


> Giving `$!` dynamic scope mitigates the problem but still seems to encourage an approach that @matz has said he was against. Specifically, he was against `ensure => exception` and this is just a variant of that approach.

I believe Matz said he was against adding extra **syntax**, and that he was open to experimenting with ko1's proposal.

Giving $! dynamic scope is indeed likely to cause subtle bugs, just less than currently. I think that almost all code currently written with `$!` in the ensure clause assume the exception is triggered within the same begin-end block. Using a dynamic scope would "magically" fix all of this currently-incorrect code. So it's a net win.

And considering backwards compatibility I think it's unrealistic to consider deprecation. `$!` is used fairly often in gems. I also use it fairly often in my code, usually in some variants of the form `expr rescue $!`

----------------------------------------
Feature #18083: Capture error in ensure block.
https://bugs.ruby-lang.org/issues/18083#change-105697

* Author: ioquatix (Samuel Williams)
* Status: Open
* Priority: Normal
----------------------------------------
As discussed in https://bugs.ruby-lang.org/issues/15567 there are some tricky edge cases.

As a general model, something like the following would be incredibly useful:

``` ruby
begin
 ...
ensure => error
  pp "error occurred" if error
end
```

Currently you can get similar behaviour like this:

``` ruby
begin
  ...
rescue Exception => error
  raise
ensure
  pp "error occurred" if error
end
```

The limitation of this approach is it only works if you don't need any other `rescue` clause. Otherwise, it may not work as expected or require extra care. Also, Rubocop will complain about it.

Using `$!` can be buggy if you call some method from `rescue` or `ensure` clause, since it would be set already. It was discussed extensively in https://bugs.ruby-lang.org/issues/15567 if you want more details.



-- 
https://bugs.ruby-lang.org/
 ______________________________________________
 ruby-core mailing list -- ruby-core@ml.ruby-lang.org
 To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org
 ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2023-12-15 21:41 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-18  3:03 [ruby-core:104960] [Ruby master Feature#18083] Capture error in ensure block ioquatix (Samuel Williams)
2021-08-18  3:09 ` [ruby-core:104962] " ioquatix (Samuel Williams)
2021-08-18 17:04 ` [ruby-core:104971] " Eregon (Benoit Daloze)
2021-08-18 17:09 ` [ruby-core:104972] " Eregon (Benoit Daloze)
2021-08-18 23:04 ` [ruby-core:104980] " ioquatix (Samuel Williams)
2021-08-19  0:45 ` [ruby-core:104981] " duerst
2021-08-19  0:51 ` [ruby-core:104982] " ioquatix (Samuel Williams)
2021-08-19  1:00 ` [ruby-core:104984] " ioquatix (Samuel Williams)
2021-08-19  8:04 ` [ruby-core:105001] " ioquatix (Samuel Williams)
2021-08-19  8:19 ` [ruby-core:105002] " matz (Yukihiro Matsumoto)
2021-08-19  9:57 ` [ruby-core:105005] " ioquatix (Samuel Williams)
2021-08-19 16:42 ` [ruby-core:105013] " Eregon (Benoit Daloze)
2021-08-20  1:09 ` [ruby-core:105019] " ioquatix (Samuel Williams)
2021-08-20 22:00 ` [ruby-core:105033] " Eregon (Benoit Daloze)
2021-08-20 22:03 ` [ruby-core:105034] " ioquatix (Samuel Williams)
2021-08-20 22:09 ` [ruby-core:105035] " Eregon (Benoit Daloze)
2021-08-21  0:52 ` [ruby-core:105036] " ioquatix (Samuel Williams)
2021-10-19  4:46 ` [ruby-core:105667] " ko1 (Koichi Sasada)
2021-10-19 17:51 ` [ruby-core:105682] " Eregon (Benoit Daloze)
2021-10-21  6:13 ` [ruby-core:105715] " ko1 (Koichi Sasada)
2021-10-21  6:28 ` [ruby-core:105716] " matz (Yukihiro Matsumoto)
2021-10-21 10:24 ` [ruby-core:105727] " Eregon (Benoit Daloze)
2021-10-21 11:15 ` [ruby-core:105728] " ioquatix (Samuel Williams)
2021-10-21 11:17 ` [ruby-core:105729] " ioquatix (Samuel Williams)
2021-10-21 13:00 ` [ruby-core:105733] " nobu (Nobuyoshi Nakada)
2023-12-14 19:40 ` [ruby-core:115747] " ioquatix (Samuel Williams) via ruby-core
2023-12-14 20:08 ` [ruby-core:115748] " kddnewton (Kevin Newton) via ruby-core
2023-12-15 14:46 ` [ruby-core:115752] " Dan0042 (Daniel DeLorme) via ruby-core
2023-12-15 20:15 ` [ruby-core:115755] " ioquatix (Samuel Williams) via ruby-core
2023-12-15 20:20 ` [ruby-core:115756] " jeremyevans0 (Jeremy Evans) via ruby-core
2023-12-15 20:30 ` [ruby-core:115757] " ioquatix (Samuel Williams) via ruby-core
2023-12-15 21:41 ` [ruby-core:115759] " Dan0042 (Daniel DeLorme) via ruby-core

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).