From: eregontp@...
Date: 2014-06-07T13:01:10+00:00
Subject: [ruby-core:62985] [ruby-trunk - Feature #7688] Error hiding with rb_rescue() on Comparable#==, #coerce and others

Issue #7688 has been updated by Benoit Daloze.


I had another look at the other cases mentioned above.

* Comparable#==: A warning has been added when rescuing an exception of #<=>. There should be no more "rescue" after 2.2.0.

* numeric.c and #coerce: The cases where an error is not raised in do_coerce() yet coercion failed are handled by their (transitive) callers which all raise other exceptions (it would be nice to make the coercion failure exception the cause of the caller exception).
The possible exception thrown by #coerce was silently ignored. A warning has been added with plans in the next minor to not rescue the possible exceptions of #coerce anymore.

* range.c and range_init(): A good solution for this would be to make the exception in #<=> the *cause* of the "bad value for range" argument error currently raised. This already works by #8257 but the *cause* is not shown (see #9918).

----------------------------------------
Feature #7688: Error hiding with rb_rescue() on Comparable#==, #coerce and others
https://bugs.ruby-lang.org/issues/7688#change-47076

* Author: Benoit Daloze
* Status: Open
* Priority: Normal
* Assignee: Benoit Daloze
* Category: core
* Target version: next minor
----------------------------------------
Hello,

I believe error hiding is harmful because very dangerous
(it forgets errors which is likely unexpected) and hard to debug.

But I guess the compatibility is the main reason to keep these cases.

In the cases of Comparable#== and #coerce, I believe it is not worth to be compatible with this dangerous behavior
as it will at worse help to discover bugs in #<=> and #coerce methods which would raise an exception.

I doubt anyone rely on this and the #coerce spec (see #7645) itself makes me think this is unexpected behavior.
It would also simplify the spec, and no specific #coerce behavior would be needed to be defined as it would behave as a simple method call without edge cases.

So I think rb_rescue() or rb_rescue2(..., GenericErrorClass, ...) should be avoided if possible.
I analyzed these in the code base and it is used in a couple places:
* compar.c in cmp_equal(): this is the main offender in this regard with #coerce
* numeric.c in rb_num_coerce_{cmp,relop}() which call do_coerce(,,FALSE): This is the current subject of #7645.

* io.c in io_close(): to avoid raising if #close fails, which is likely less problematic,
  although it would be nicer to rescue only IO-related errors and warn when an exception is raised.
* range.c in range_init(): this is to provide a clearer error. I think it would be nice to show the original error as well.

Removing the general rescue in cmp_equal() revealed a couple bugs in RDoc due to this problem. I guess there are many others in the wild.

Can we please remove this anti-pattern?
I believe impact is only positive and that it should be done as soon as possible.

What do you think?



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