ruby / did_you_mean

The gem that has been saving people from typos since 2014
MIT License
1.87k stars 113 forks source link

Rescue failing corrections with an empty array #146

Closed p8 closed 4 years ago

p8 commented 4 years ago

If an error is raised in Correctable#to_s it is rescued and the super method is called instead. If Correctable#corrections is called it should be rescued as well and return an empty array.

This allows 3rd party gems to customize how suggestions are displayed:

   error = NameError.new("uninitialized constant Object")
   puts error.original_message
   puts "Did you mean:"
   puts error.corrections.join(", ")

I'm trying to use #original_message and #corrections in the Rails error pages (https://github.com/rails/rails/pull/39363), but calling #corrections sometimes fails while #to_s works (but it doesn't show the suggestions).

yuki24 commented 4 years ago

Hmm, I don't think we should ever add a recsue that broadly catches errors like this unless there is a strong reason to do so. If there is a failure, it should simply just be fixed. What are the errors you have seen?

p8 commented 4 years ago

I'm not a fan of the rescue either. 😄 There already is a rescue in the to_s and I wonder if that rescues errors caused by calling corrections in most cases.

I was trying to call corrections in the Rails error templates but the following test failed with a "No receiver" ArgumentError. It currently works because to_s rescues the error. So you won't see any suggestions. https://github.com/rails/rails/blob/master/actionpack/test/dispatch/debug_exceptions_test.rb#L367

yuki24 commented 4 years ago

I'm closing this issue. If there is an issue in the code you actively work on, it should just be fixed there rather than silently falling back to the original behavior in DYM. I'll follow up on the rails/rails side.

p8 commented 4 years ago

So the "No receiver" ArgumentError was caused by an incorrect override of ParameterMissing#initialize. Instead of:

def initialize(param, keys = nil)

It should have matched the superclass KeyError#initialize:

def initialize(param, receiver: nil)