Skip to content

Commit

Permalink
Merge pull request #1248 from sul-dlss/timeouts
Browse files Browse the repository at this point in the history
Add timeouts to try to stop things from jamming up if image server or /stacks aren't available
  • Loading branch information
jcoyne authored Aug 20, 2024
2 parents 68ab6ba + aea0286 commit e68389f
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 21 deletions.
3 changes: 3 additions & 0 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ Style/StringLiterals:
RSpec/MultipleMemoizedHelpers:
Enabled: false

RSpec/MessageChain:
Enabled: false

Layout/EmptyLinesAroundAttributeAccessor:
Enabled: true
Layout/SpaceAroundMethodCallOperator:
Expand Down
6 changes: 5 additions & 1 deletion app/models/cocina.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,11 @@ class Cocina
def self.find(druid, version = :head)
data = Rails.cache.fetch(metadata_cache_key(druid, version), expires_in: 10.minutes) do
benchmark "Fetching public json for #{druid} version #{version}" do
response = Faraday.get(public_json_url(druid, version))
connection = Faraday.new({ url: public_json_url(druid, version),
headers: { user_agent: Settings.user_agent },
request: { open_timeout: 5 } })

response = connection.get
raise Purl::Exception, response.status unless response.success?

JSON.parse(response.body)
Expand Down
5 changes: 4 additions & 1 deletion app/models/iiif_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ def initialize(stacks_file:, transformation:, base_uri: Settings.imageserver.bas
def response
with_retries max_tries: 3, rescue: [HTTP::ConnectionError] do
benchmark "Fetch #{image_url}" do
HTTP.use({ normalize_uri: { normalizer: lambda(&:itself) } }).get(image_url)
HTTP.timeout(connect: 15)
.headers(user_agent: "#{HTTP::Request::USER_AGENT} (#{Settings.user_agent})")
.use({ normalize_uri: { normalizer: lambda(&:itself) } })
.get(image_url)
end
end
end
Expand Down
4 changes: 3 additions & 1 deletion app/services/iiif_metadata_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,9 @@ def retrieve
handle_response(
# Disable url normalization as an upstream bug in addressable causes issues for `+`
# https://github.com/sporkmonger/addressable/issues/386
HTTP.use({ normalize_uri: { normalizer: lambda(&:itself) } }).get(@url)
HTTP.timeout(connect: 15)
.headers(user_agent: "#{HTTP::Request::USER_AGENT} (#{Settings.user_agent})")
.use({ normalize_uri: { normalizer: lambda(&:itself) } }).get(@url)
)
end
end
Expand Down
7 changes: 5 additions & 2 deletions app/services/metrics_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ def event_data(name, properties)
end

def default_headers
{ 'Content-Type': 'application/json' }
{
'Content-Type': 'application/json',
'User-Agent': Settings.user_agent
}
end

def post_json(url, data, headers)
Expand All @@ -50,6 +53,6 @@ def post_json(url, data, headers)
end

def connection
@connection ||= Faraday.new(base_url)
@connection ||= Faraday.new({ url: base_url, request: { open_timeout: 5 } })
end
end
14 changes: 14 additions & 0 deletions config/initializers/okcomputer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,20 @@

OkComputer::Registry.register 'rails_cache', OkComputer::GenericCacheCheck.new

class DirectoryCheckWithTimeout < OkComputer::DirectoryCheck
def check
Timeout.timeout(5) do
super
end
rescue Timeout::Error
mark_failure
mark_message "Timed out after 5 seconds"
end
end

OkComputer::Registry.register 'stacks_root_dir',
DirectoryCheckWithTimeout.new(Settings.stacks.storage_root)

OkComputer::Registry.register 'stacks_mounted_dir',
OkComputer::DirectoryCheck.new(Settings.stacks.storage_root)

Expand Down
2 changes: 2 additions & 0 deletions config/settings.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,5 @@ cors:

token:
default_expiry_time: <%= 1.hour %>

user_agent: 'stacks.stanford.edu'
17 changes: 10 additions & 7 deletions spec/controllers/object_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@
allow_any_instance_of(StacksFile).to receive(:path).and_return(Rails.root + 'Gemfile')
end

let(:connection) { instance_double(Faraday::Connection) }

describe '#show' do
context 'when not logged in' do
context "with an invalid druid" do
let(:druid) { 'foo' }

it 'returns a 404 Not Found' do
allow(Faraday).to receive(:get).with('https://purl.stanford.edu/foo.json').and_return(
allow(Faraday).to receive(:new).with(hash_including(url: 'https://purl.stanford.edu/foo.json')).and_return(connection)
allow(connection).to receive(:get).and_return(
instance_double(Faraday::Response, status: 404, success?: false)
)
get :show, params: { id: 'foo' }
Expand Down Expand Up @@ -89,8 +92,8 @@
end

before do
allow(Faraday).to receive(:get).with('https://purl.stanford.edu/fd063dh3727.json')
.and_return(instance_double(Faraday::Response, success?: true, body: json))
allow(Faraday).to receive(:new).with(hash_including(url: 'https://purl.stanford.edu/fd063dh3727.json')).and_return(connection)
allow(connection).to receive(:get).and_return(instance_double(Faraday::Response, success?: true, body: json))
end

it 'creates a zip' do
Expand Down Expand Up @@ -145,8 +148,8 @@
end

before do
allow(Faraday).to receive(:get).with('https://purl.stanford.edu/bb142ws0723.json')
.and_return(instance_double(Faraday::Response, success?: true, body: json))
allow(Faraday).to receive(:new).with(hash_including(url: 'https://purl.stanford.edu/bb142ws0723.json')).and_return(connection)
allow(connection).to receive(:get).and_return(instance_double(Faraday::Response, success?: true, body: json))
end

it 'redirects to login' do
Expand Down Expand Up @@ -209,8 +212,8 @@
end

before do
allow(Faraday).to receive(:get).with('https://purl.stanford.edu/bb142ws0723.json')
.and_return(instance_double(Faraday::Response, success?: true, body: json))
allow(Faraday).to receive(:new).with(hash_including(url: 'https://purl.stanford.edu/bb142ws0723.json')).and_return(connection)
allow(connection).to receive(:get).and_return(instance_double(Faraday::Response, success?: true, body: json))
end

it 'creates a zip' do
Expand Down
12 changes: 6 additions & 6 deletions spec/models/projection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@
let(:options) { { size: 'max', region: 'full' } }

it 'allows the user to see the full-resolution image' do
allow(HTTP).to receive(:use).and_return(http_client)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use).and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(http_client).to have_received(:get).with(%r{/full/max/0/default.jpg})
Expand All @@ -121,7 +121,7 @@
let(:options) { { size: '!850,700', region: 'full' } }

it 'returns original size when requested dimensions are larger' do
allow(HTTP).to receive(:use).and_return(http_client)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use).and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
expect(http_client).to have_received(:get).with(%r{/full/!800,600/0/default.jpg})
Expand All @@ -138,7 +138,7 @@
let(:options) { { size: 'max', region: 'full' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:use)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
Expand All @@ -150,7 +150,7 @@
let(:options) { { size: '!100,100', region: 'full' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:use)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
Expand All @@ -162,7 +162,7 @@
let(:options) { { size: '!800,880', region: 'full' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:use)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
Expand All @@ -174,7 +174,7 @@
let(:options) { { size: '100,100', region: 'square' } }

it 'limits users to a thumbnail' do
allow(HTTP).to receive(:use)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use)
.and_return(http_client)
allow(http_client).to receive(:get).and_return(double(body: nil))
subject.response
Expand Down
2 changes: 1 addition & 1 deletion spec/requests/iiif_auth_request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
describe "#show" do
before do
allow_any_instance_of(Projection).to receive(:valid?).and_return(true)
allow(HTTP).to receive(:use).and_return(http_client)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use).and_return(http_client)
allow(http_client).to receive(:get).and_return(instance_double(HTTP::Response, status: 200, body: StringIO.new))
allow_any_instance_of(IiifController).to receive(:current_user).and_return(current_user)
allow_any_instance_of(IiifController).to receive(:current_image).and_return(current_image)
Expand Down
4 changes: 2 additions & 2 deletions spec/services/iiif_metadata_service_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
let(:response) { instance_double(HTTP::Response, code: 200, body: json) }

before do
allow(HTTP).to receive(:use).and_return(http_client)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use).and_return(http_client)
allow(http_client).to receive(:get)
.with("https://sul-imageserver-uat.stanford.edu/cantaloupe/iiif/2/#{image_server_path(druid, file_name)}/info.json")
.and_return(response)
Expand Down Expand Up @@ -57,7 +57,7 @@
let(:empty_json) { '' }
let(:bad_response) { instance_double(HTTP::Response, code: 200, body: empty_json) }
before do
allow(HTTP).to receive(:use)
allow(HTTP).to receive_message_chain(:timeout, :headers, :use)
.and_return(http_client)
allow(http_client).to receive(:get)
.with("https://sul-imageserver-uat.stanford.edu/cantaloupe/iiif/2/#{image_server_path(druid, file_name)}/info.json")
Expand Down

0 comments on commit e68389f

Please sign in to comment.