From: eregontp@... Date: 2017-04-25T11:47:42+00:00 Subject: [ruby-core:80859] [Ruby trunk Feature#7688] Error hiding with rb_rescue() on Comparable#==, #coerce and others Issue #7688 has been updated by Eregon (Benoit Daloze). All rb_rescue() hiding user exceptions and/or re-raising a generic errors (mentioned above) have now been removed. Please do not introduce rb_rescue() call in MRI, hiding user exceptions is harmful. This changes make it much easier to debug as for instance one gets the raised by the user code (such a a NoMethodError because of a typo) instead of a generic "bad value for range" exception. I updated NEWS, tests and ruby/spec accordingly. ---------------------------------------- Feature #7688: Error hiding with rb_rescue() on Comparable#==, #coerce and others https://bugs.ruby-lang.org/issues/7688#change-64465 * Author: Eregon (Benoit Daloze) * Status: Closed * Priority: Normal * Assignee: Eregon (Benoit Daloze) * Target version: ---------------------------------------- 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/ Unsubscribe: