Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
72 changes: 33 additions & 39 deletions app/controllers/idv/in_person/address_search_controller.rb
Original file line number Diff line number Diff line change
@@ -1,25 +1,18 @@
module Idv
module InPerson
class AddressSearchController < ApplicationController
rescue_from Faraday::ConnectionFailed, Faraday::TimeoutError,
Faraday::ClientError, StandardError,
with: :report_errors

def index
response = addresses(params[:address])

render(**response)
end

protected

def api_status_from_error(err)
if err.instance_of?(Faraday::ClientError)
:bad_request
elsif err.instance_of?(Faraday::ConnectionFailed)
:bad_request
elsif err.instance_of?(Faraday::TimeoutError)
:bad_request
else
:internal_server_error
end
end

def addresses(search_term)
addresses = geocoder.find_address_candidates(SingleLine: search_term).slice(0, 1)
if addresses.empty?
Expand All @@ -29,42 +22,43 @@ def addresses(search_term)
end

{ json: addresses, status: :ok }
rescue StandardError => err
api_status = api_status_from_error(err)
api_status_code = Rack::Utils::SYMBOL_TO_STATUS_CODE[api_status]
exception_message = err.message
response_status_code = err.respond_to?(:response_status) && err.response_status
errors = if err.instance_of?(Faraday::ClientError)
err.response_body && err.response_body[:details]
end
end

def geocoder
@geocoder ||= IdentityConfig.store.arcgis_mock_fallback ?
ArcgisApi::Mock::Geocoder.new : ArcgisApi::Geocoder.new
end

Copy link
Contributor

Choose a reason for hiding this comment

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

my pr refactors this a bit. i'm currently working through a merge conflict with my pr and the usps controller error handling refactor. Can we pause a moment before merging this so I can take a look and figure out how this works with my changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@svalexander sure. Feel free to copy the approach if you want. We should keep it consistent with how we changed things in USPS. Open to different approaches!

def report_errors(error)
remapped_error = {
Faraday::ClientError => :bad_request,
Faraday::ConnectionFailed => :bad_request,
Faraday::TimeoutError => :bad_request,
}[error.class] || :internal_server_error

errors = if error.instance_of?(Faraday::ClientError)
error.response_body && error.response_body[:details]
end
errors ||= 'ArcGIS error performing operation'

# log search event
analytics.idv_in_person_locations_searched(
success: false,
errors: errors,
api_status_code: api_status_code,
exception_class: err.class,
exception_message: exception_message,
response_status_code: response_status_code,
api_status_code: Rack::Utils.status_code(remapped_error),
exception_class: error.class,
exception_message: error.message,
response_status_code: error.respond_to?(:response_status) && error.response_status,
)

# log the request failure
analytics.idv_arcgis_request_failure(
exception_class: err.class,
exception_message: exception_message,
response_body_present: err.respond_to?(:response_body) && err.response_body.present?,
response_body: err.respond_to?(:response_body) && err.response_body,
response_status_code: response_status_code,
api_status_code: api_status_code,
api_status_code: Rack::Utils.status_code(remapped_error),
exception_class: error.class,
exception_message: error.message,
response_body_present: error.respond_to?(:response_body) && error.response_body.present?,
response_body: error.respond_to?(:response_body) && error.response_body,
response_status_code: error.respond_to?(:response_status) && error.response_status,
)

{ json: [], status: api_status }
end

def geocoder
@geocoder ||= IdentityConfig.store.arcgis_mock_fallback ?
ArcgisApi::Mock::Geocoder.new : ArcgisApi::Geocoder.new
render json: [], status: remapped_error
end
end
end
Expand Down
27 changes: 27 additions & 0 deletions spec/controllers/idv/in_person/address_search_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@
end

context 'with a timeout error' do
let(:server_error) { Faraday::TimeoutError.new }

before do
stub_analytics
exception = Faraday::TimeoutError.new
allow(geocoder).to receive(:find_address_candidates).and_raise(exception)
end
Expand All @@ -137,6 +140,17 @@
expect(response.status).to eq(400)
addresses = JSON.parse(response.body)
expect(addresses.length).to eq 0

expect(@analytics).to have_logged_event(
'Request ArcGIS Address Candidates: request failed',
api_status_code: response.status,
exception_class: server_error.class,
exception_message: server_error.message,
response_body_present:
server_error.response_body.present?,
response_body: server_error.response_body,
response_status_code: server_error.response_status,
)
end

it 'logs search analytics' do
Expand Down Expand Up @@ -180,6 +194,19 @@
response_status_code: false,
)
end

it 'logs the arcgis request failure' do
get :index
expect(@analytics).to have_logged_event(
'Request ArcGIS Address Candidates: request failed',
api_status_code: 500,
exception_class: StandardError,
exception_message: 'error',
response_status_code: false,
response_body_present: false,
response_body: false,
)
end
end
end
end