Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate out handling of Solr timeout errors with it's own exception class #2597

Merged
merged 5 commits into from
Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/blacklight/exceptions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class ExpiredSessionToken < StandardError

class ECONNREFUSED < ::Errno::ECONNREFUSED; end

class RepositoryTimeout < Timeout::Error; end

class IconNotFound < StandardError
end
end
Expand Down
5 changes: 5 additions & 0 deletions lib/blacklight/solr/repository.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ def send_and_receive(path, solr_params = {})
Blacklight.logger&.debug("Solr response: #{solr_response.inspect}") if defined?(::BLACKLIGHT_VERBOSE_LOGGING) && ::BLACKLIGHT_VERBOSE_LOGGING
solr_response
end
rescue *[(defined?(RSolr::Error::Timeout) ? RSolr::Error::Timeout : nil)].compact => e
# RSolr 2.4.0+ has a RSolr::Error::Timeout that we'd like to treat specially instead of
# lumping into RSolr::Error::Http
raise Blacklight::Exceptions::RepositoryTimeout, "Timeout connecting to Solr instance using #{connection.inspect}: #{e.inspect}"
rescue Errno::ECONNREFUSED => e
# intended for and likely to be a RSolr::Error:ConnectionRefused, specifically.
raise Blacklight::Exceptions::ECONNREFUSED, "Unable to connect to Solr instance using #{connection.inspect}: #{e.inspect}"
rescue RSolr::Error::Http => e
raise Blacklight::Exceptions::InvalidRequest, e.message
Expand Down
9 changes: 9 additions & 0 deletions spec/services/blacklight/search_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,15 @@
expect { service.repository.search }.to raise_exception(/Unable to connect to Solr instance/)
end

it "raises a Blacklight exception if RSolr raises a timeout error connecting to Solr instance" do
# rsolr api is mess, let's
Copy link
Member

Choose a reason for hiding this comment

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

let's...?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure what I was thinking a month ago. Let's mock a timeout to give us a way to test it? Or shall I just delete this comment?

Copy link
Member

Choose a reason for hiding this comment

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

I'd just remove it.

rsolr_timeout = RSolr::Error::Timeout.new(nil, nil)
allow(rsolr_timeout).to receive(:to_s).and_return("mocked RSolr timeout")

allow(blacklight_solr).to receive(:send_and_receive).and_raise(rsolr_timeout)
expect { service.repository.search }.to raise_exception(Blacklight::Exceptions::RepositoryTimeout, /Timeout connecting to Solr instance/)
end

describe "#previous_and_next_documents_for_search" do
let(:user_params) { { q: '', per_page: 100 } }

Expand Down