Skip to content

Commit

Permalink
Ignore all exceptions for validates_acceptance_of columns fetch so it…
Browse files Browse the repository at this point in the history
… can run even without a database connection

Signed-off-by: Michael Koziarski <michael@koziarski.com>
  • Loading branch information
tarmo authored and NZKoz committed Sep 26, 2008
1 parent 4d9a7ab commit ea609b2
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/validations.rb
Expand Up @@ -472,7 +472,7 @@ def validates_acceptance_of(*attr_names)

db_cols = begin
column_names
rescue ActiveRecord::StatementInvalid
rescue Exception # To ignore both statement and connection errors
[]
end
names = attr_names.reject { |name| db_cols.include?(name.to_s) }
Expand Down

2 comments on commit ea609b2

@Manfred
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure catching Exception is such a good idea, it might mask all kinds of bugs like syntax errors thrown while executing the column_names method.

Why not just catch the errors thrown by ActiveRecord?

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on ea609b2 Sep 26, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it’s definitely less than ideal, but the problem is the variety of exceptions that get thrown when this thing is missing. There’s one for each of the adapters, and several different ones depending on the ‘backing gem’ i.e. pg vs postgres.

Hopefully for 2.3 we can tidy the boundaries between adapter and driver and make sure that only AR errors are thrown by the Adapters, but right now that line’s too fuzzy and making a far reaching change for this one case didn’t quite seem worth it.

Please sign in to comment.