From: eregontp@...
Date: 2014-01-17T12:09:41+00:00
Subject: [ruby-core:59825] [ruby-trunk - Feature #7688] Error hiding with rb_rescue() on Comparable#==, #coerce and others

Issue #7688 has been updated by Benoit Daloze.


Aaron Patterson wrote:
> This also broke the RDoc tests.  Since RDoc was broken, I couldn't install gems:
> 
>   https://github.com/rdoc/rdoc/issues/284
> 
> Can we *please* consider issuing a warning in trunk rather than raising?  I think raising is fine in the future, but removing functionality without giving people notice seems bad.  I understand that some people don't read warnings, but I don't think throwing our hands in the air and giving up on warnings is the right course of action.

I will add a warning then and still rescue until next release, thanks for the feedback!

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

* 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?



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