From dc6b851b232a779e1aebbbb5cd1b5fe28a78b283 Mon Sep 17 00:00:00 2001 From: "yuuji.yaginuma" Date: Mon, 7 May 2018 09:50:46 +0900 Subject: [PATCH] Consolidate errors when loading adapter gem fails into `LoadError` If not using Bundler and the specified adapter gem does not exist, `rubygems` raises `Gem::MissingSpecError`. `Gem::MissingSpecError` inherits `LoadError`, but the argument of `initialize` is different. https://github.com/rubygems/rubygems/blob/9054079ce8a1ad05dd206a3562339b74712c7fc6/lib/rubygems/errors.rb#L27..L31 As a result, `ArugmentError` occurs when re-raising the exception. Although it is possible to check the error class and specify appropriate arguments for each of them, I think that it is sufficient to just raise `LoadError`. Together, when using Bundler, fixed that gem which error occurred could not be checked correctly. Fixes #32830. --- actioncable/lib/action_cable/server/configuration.rb | 7 ++++--- .../connection_adapters/connection_specification.rb | 7 ++++--- .../test/cases/connection_specification/resolver_test.rb | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb index 26209537df378..3eb0c3feeeabb 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -35,15 +35,16 @@ def pubsub_adapter rescue LoadError => e # We couldn't require the adapter itself. Raise an exception that # points out config typos and missing gems. - if e.path == path_to_adapter + # When `Gem::LoadError` occurs, possible that `path` is not set correctly. Instead can get the gem name from `name` attribute. + if (e.path == path_to_adapter) || (e.respond_to?(:name) && e.name == adapter) # We can assume that a non-builtin adapter was specified, so it's # either misspelled or missing from Gemfile. - raise e.class, "Could not load the '#{adapter}' Action Cable pubsub adapter. Ensure that the adapter is spelled correctly in config/cable.yml and that you've added the necessary adapter gem to your Gemfile.", e.backtrace + raise LoadError, "Could not load the '#{adapter}' Action Cable pubsub adapter. Ensure that the adapter is spelled correctly in config/cable.yml and that you've added the necessary adapter gem to your Gemfile or installed.", e.backtrace # Bubbled up from the adapter require. Prefix the exception message # with some guidance about how to address it and reraise. else - raise e.class, "Error loading the '#{adapter}' Action Cable pubsub adapter. Missing a gem it depends on? #{e.message}", e.backtrace + raise LoadError, "Error loading the '#{adapter}' Action Cable pubsub adapter. Missing a gem it depends on? #{e.message}", e.backtrace end end diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 901717ae3de87..ad8f71db50371 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -191,15 +191,16 @@ def spec(config) rescue LoadError => e # We couldn't require the adapter itself. Raise an exception that # points out config typos and missing gems. - if e.path == path_to_adapter + # When `Gem::LoadError` occurs, possible that `path` is not set correctly. Instead can get the gem name from `name` attribute. + if (e.path == path_to_adapter) || (e.respond_to?(:name) && e.name == spec[:adapter]) # We can assume that a non-builtin adapter was specified, so it's # either misspelled or missing from Gemfile. - raise e.class, "Could not load the '#{spec[:adapter]}' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile.", e.backtrace + raise LoadError, "Could not load the '#{spec[:adapter]}' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile or installed.", e.backtrace # Bubbled up from the adapter require. Prefix the exception message # with some guidance about how to address it and reraise. else - raise e.class, "Error loading the '#{spec[:adapter]}' Active Record adapter. Missing a gem it depends on? #{e.message}", e.backtrace + raise LoadError, "Error loading the '#{spec[:adapter]}' Active Record adapter. Missing a gem it depends on? #{e.message}", e.backtrace end end diff --git a/activerecord/test/cases/connection_specification/resolver_test.rb b/activerecord/test/cases/connection_specification/resolver_test.rb index 5b80f16a44102..1986d8c8e21ff 100644 --- a/activerecord/test/cases/connection_specification/resolver_test.rb +++ b/activerecord/test/cases/connection_specification/resolver_test.rb @@ -19,7 +19,7 @@ def test_url_invalid_adapter spec "ridiculous://foo?encoding=utf8" end - assert_match "Could not load the 'ridiculous' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile.", error.message + assert_match "Could not load the 'ridiculous' Active Record adapter. Ensure that the adapter is spelled correctly in config/database.yml and that you've added the necessary adapter gem to your Gemfile or installed.", error.message end # The abstract adapter is used simply to bypass the bit of code that