Skip to content

Commit 720ff31

Browse files
Add error handling to Tacos model
** Why are these changes being introduced: I realized that the Tacos model as originally proposed had no error handling when I refreshed a results page without my local TACOS app running. The result was a pretty ugly blob shown to the user. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-66 ** How does this address that need: This adds two catch statements to the call to TACOS - one for an HTTP error response, and one for a JSON parsing error. It also refactors the Tacos model to allow a separately-defined HTTP object, which we craft in the test suite to always return those two error conditions. ** Document any side effects to this change: This is yet another pattern for error handling, not using VCR cassettes or webmock, neither of which I could get working this afternoon. I'm not happy about that, but this does work.
1 parent a870cd4 commit 720ff31

File tree

2 files changed

+61
-7
lines changed

2 files changed

+61
-7
lines changed

app/models/tacos.rb

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
class Tacos
2-
def self.call(term)
3-
tacos_http = HTTP.persistent(tacos_url)
4-
.headers(accept: 'application/json',
5-
'Content-Type': 'application/json',
6-
origin: origins)
2+
# The tacos_client argument here is unused in production - it is provided for
3+
# our test suite so that we can mock various error conditions to ensure that
4+
# error handling happens as we intend.
5+
def self.call(term, tacos_client = nil)
6+
tacos_http = setup(tacos_client)
77
query = '{ "query": "{ logSearchEvent(searchTerm: \"' + clean_term(term) + '\", sourceSystem: \"' + tacos_source + '\" ) { phrase source detectors { suggestedResources { title url } } } }" }'
8-
raw_response = tacos_http.timeout(http_timeout).post(tacos_url, body: query)
9-
JSON.parse(raw_response.to_s)
8+
begin
9+
raw_response = tacos_http.timeout(http_timeout).post(tacos_url, body: query)
10+
JSON.parse(raw_response.to_s)
11+
rescue HTTP::Error
12+
{"error" => "A connection error has occurred"}
13+
rescue JSON::ParserError
14+
{"error" => "A parsing error has occurred"}
15+
end
1016
end
1117

1218
private
@@ -23,6 +29,16 @@ def self.origins
2329
ENV.fetch('ORIGINS', nil)
2430
end
2531

32+
# We define the HTTP connection this way so that it can be overridden during
33+
# testing, to make sure that the .call method can handle specific error
34+
# conditions.
35+
def self.setup(tacos_client)
36+
tacos_client || HTTP.persistent(tacos_url)
37+
.headers(accept: 'application/json',
38+
'Content-Type': 'application/json',
39+
origin: origins)
40+
end
41+
2642
def self.tacos_source
2743
ENV.fetch('TACOS_SOURCE', 'unset')
2844
end

test/models/tacos_test.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,25 @@
11
require 'test_helper'
22

3+
class TacosConnectionError
4+
def timeout(_)
5+
self
6+
end
7+
8+
def post(_url, body:)
9+
raise HTTP::ConnectionError, "forced connection failure"
10+
end
11+
end
12+
13+
class TacosParsingError
14+
def timeout(_)
15+
self
16+
end
17+
18+
def post(_url, body:)
19+
'This is not valid json'
20+
end
21+
end
22+
323
class TacosTest < ActiveSupport::TestCase
424
test 'TACOS model has a call method that reflects a search term back' do
525
VCR.use_cassette('tacos popcorn') do
@@ -21,4 +41,22 @@ class TacosTest < ActiveSupport::TestCase
2141
end
2242
end
2343
end
44+
45+
test 'TACOS model catches connection errors' do
46+
tacos_client = TacosConnectionError.new
47+
48+
result = Tacos.call('popcorn', tacos_client)
49+
50+
assert_instance_of Hash, result
51+
assert_equal 'A connection error has occurred', result['error']
52+
end
53+
54+
test 'TACOS model catches parsing errors' do
55+
tacos_client = TacosParsingError.new
56+
57+
result = Tacos.call('popcorn', tacos_client)
58+
59+
assert_instance_of Hash, result
60+
assert_equal 'A parsing error has occurred', result['error']
61+
end
2462
end

0 commit comments

Comments
 (0)