From ac417381e2777e4acf9051bfeb70a8a2fbd67df3 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Wed, 9 Aug 2023 13:04:48 -0700 Subject: [PATCH 1/6] return 422 instead of 400 on ArcGIS errors keeps us consistent with the location controller --- .../idv/in_person/address_search_controller.rb | 2 +- .../idv/in_person/address_search_controller_spec.rb | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/idv/in_person/address_search_controller.rb b/app/controllers/idv/in_person/address_search_controller.rb index fe6dba20f93..e14db409395 100644 --- a/app/controllers/idv/in_person/address_search_controller.rb +++ b/app/controllers/idv/in_person/address_search_controller.rb @@ -33,7 +33,7 @@ def report_errors(error) remapped_error = case error when Faraday::Error, ActionController::InvalidAuthenticityToken - :bad_request + :unprocessable_entity else :internal_server_error end diff --git a/spec/controllers/idv/in_person/address_search_controller_spec.rb b/spec/controllers/idv/in_person/address_search_controller_spec.rb index 09aea7ee5d5..5689604056c 100644 --- a/spec/controllers/idv/in_person/address_search_controller_spec.rb +++ b/spec/controllers/idv/in_person/address_search_controller_spec.rb @@ -86,7 +86,7 @@ expect(addresses.length).to eq 0 expect(@analytics).to have_logged_event( 'IdV: in person proofing location search submitted', - api_status_code: 400, + api_status_code: 422, success: false, errors: 'request is too many characters', result_total: 0, @@ -106,7 +106,7 @@ it 'gets an empty pilot response' do response = get :index - expect(response.status).to eq(400) + expect(response.status).to eq(422) addresses = JSON.parse(response.body) expect(addresses.length).to eq 0 end @@ -115,7 +115,7 @@ response expect(@analytics).to have_logged_event( 'IdV: in person proofing location search submitted', - api_status_code: 400, + api_status_code: 422, success: false, errors: 'ArcGIS error performing operation', result_total: 0, @@ -137,7 +137,7 @@ it 'returns an error code' do response = get :index - expect(response.status).to eq(400) + expect(response.status).to eq(422) addresses = JSON.parse(response.body) expect(addresses.length).to eq 0 @@ -157,7 +157,7 @@ response expect(@analytics).to have_logged_event( 'IdV: in person proofing location search submitted', - api_status_code: 400, + api_status_code: 422, success: false, errors: 'ArcGIS error performing operation', result_total: 0, From 4981f539c5f264f7f6f9a1a7a96d3ca3994db1a0 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Wed, 9 Aug 2023 13:06:12 -0700 Subject: [PATCH 2/6] re-indent case conditionals --- .../idv/in_person/address_search_controller.rb | 12 ++++++------ .../idv/in_person/usps_locations_controller.rb | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/controllers/idv/in_person/address_search_controller.rb b/app/controllers/idv/in_person/address_search_controller.rb index e14db409395..bce4af41d70 100644 --- a/app/controllers/idv/in_person/address_search_controller.rb +++ b/app/controllers/idv/in_person/address_search_controller.rb @@ -31,12 +31,12 @@ def geocoder def report_errors(error) remapped_error = case error - when Faraday::Error, - ActionController::InvalidAuthenticityToken - :unprocessable_entity - else - :internal_server_error - end + when Faraday::Error, + ActionController::InvalidAuthenticityToken + :unprocessable_entity + else + :internal_server_error + end errors = if error.respond_to?(:response_body) error.response_body && error.response_body[:details] diff --git a/app/controllers/idv/in_person/usps_locations_controller.rb b/app/controllers/idv/in_person/usps_locations_controller.rb index f85b6853b41..f2181284127 100644 --- a/app/controllers/idv/in_person/usps_locations_controller.rb +++ b/app/controllers/idv/in_person/usps_locations_controller.rb @@ -63,12 +63,12 @@ def add_proofing_component def handle_error(err) remapped_error = case err - when ActionController::InvalidAuthenticityToken, - Faraday::Error - :unprocessable_entity - else - :internal_server_error - end + when ActionController::InvalidAuthenticityToken, + Faraday::Error + :unprocessable_entity + else + :internal_server_error + end analytics.idv_in_person_locations_request_failure( api_status_code: Rack::Utils.status_code(remapped_error), From a9fa4a41dd6ef206bba2c64e556a045352418f41 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Wed, 9 Aug 2023 13:08:02 -0700 Subject: [PATCH 3/6] handle a malformed response body changelog: Internal, In-person proofing, Handle a malformed response body from ArcGIS service --- .../in_person/address_search_controller.rb | 2 +- .../address_search_controller_spec.rb | 23 ++++++++++++++++++- 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/app/controllers/idv/in_person/address_search_controller.rb b/app/controllers/idv/in_person/address_search_controller.rb index bce4af41d70..9597e711999 100644 --- a/app/controllers/idv/in_person/address_search_controller.rb +++ b/app/controllers/idv/in_person/address_search_controller.rb @@ -39,7 +39,7 @@ def report_errors(error) end errors = if error.respond_to?(:response_body) - error.response_body && error.response_body[:details] + error.response_body.is_a?(Hash) && error.response_body[:details] end errors ||= 'ArcGIS error performing operation' diff --git a/spec/controllers/idv/in_person/address_search_controller_spec.rb b/spec/controllers/idv/in_person/address_search_controller_spec.rb index 5689604056c..5080a5ee60b 100644 --- a/spec/controllers/idv/in_person/address_search_controller_spec.rb +++ b/spec/controllers/idv/in_person/address_search_controller_spec.rb @@ -68,13 +68,14 @@ 'message' => 'Unable to complete operation.', } } end + let(:parsed_response_body) { { details: response_body['error']['details'].join(', ') } } before do exception = Faraday::ClientError.new( RuntimeError.new(response_body['error']['message']), { status: response_body['error']['code'], - body: { details: response_body['error']['details'].join(', ') }, + body: parsed_response_body, }, ) allow(geocoder).to receive(:find_address_candidates).and_raise(exception) @@ -95,6 +96,26 @@ response_status_code: 400, ) end + + context 'with malformed response body details' do + let(:parsed_response_body) { response_body['error']['details'] } + + it 'logs analytics event' do + response = get :index + addresses = JSON.parse(response.body) + expect(addresses.length).to eq 0 + expect(@analytics).to have_logged_event( + 'IdV: in person proofing location search submitted', + api_status_code: 422, + success: false, + errors: 'ArcGIS error performing operation', + result_total: 0, + exception_class: Faraday::ClientError, + exception_message: 'Unable to complete operation.', + response_status_code: 400, + ) + end + end end end From ee58a91edf19cf4212ea406a3242e20563462caa Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Wed, 9 Aug 2023 13:09:23 -0700 Subject: [PATCH 4/6] status code should be nil not false when missing --- .../idv/in_person/address_search_controller.rb | 8 ++++++-- .../idv/in_person/address_search_controller_spec.rb | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/controllers/idv/in_person/address_search_controller.rb b/app/controllers/idv/in_person/address_search_controller.rb index 9597e711999..73cf9c7c499 100644 --- a/app/controllers/idv/in_person/address_search_controller.rb +++ b/app/controllers/idv/in_person/address_search_controller.rb @@ -44,13 +44,17 @@ def report_errors(error) errors ||= 'ArcGIS error performing operation' + response_status_code = if error.respond_to?(:response_status) + error.response_status + end + analytics.idv_in_person_locations_searched( success: false, errors: errors, 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, + response_status_code: response_status_code, ) analytics.idv_arcgis_request_failure( @@ -59,7 +63,7 @@ def report_errors(error) 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, + response_status_code: response_status_code, ) render json: [], status: remapped_error end diff --git a/spec/controllers/idv/in_person/address_search_controller_spec.rb b/spec/controllers/idv/in_person/address_search_controller_spec.rb index 5080a5ee60b..2c8679b326b 100644 --- a/spec/controllers/idv/in_person/address_search_controller_spec.rb +++ b/spec/controllers/idv/in_person/address_search_controller_spec.rb @@ -212,7 +212,7 @@ result_total: 0, exception_class: StandardError, exception_message: 'error', - response_status_code: false, + response_status_code: nil, ) end @@ -223,7 +223,7 @@ api_status_code: 500, exception_class: StandardError, exception_message: 'error', - response_status_code: false, + response_status_code: nil, response_body_present: false, response_body: false, ) From 45e1df64897ba6e077515197c130931ca6088f81 Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Wed, 9 Aug 2023 13:09:45 -0700 Subject: [PATCH 5/6] add new test for invalid token --- .../address_search_controller_spec.rb | 30 ++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/spec/controllers/idv/in_person/address_search_controller_spec.rb b/spec/controllers/idv/in_person/address_search_controller_spec.rb index 2c8679b326b..511d1f5fdb4 100644 --- a/spec/controllers/idv/in_person/address_search_controller_spec.rb +++ b/spec/controllers/idv/in_person/address_search_controller_spec.rb @@ -119,7 +119,7 @@ end end - context 'with unsuccessful fetch' do + context 'with connection failed exception' do before do exception = Faraday::ConnectionFailed.new('connection failed') allow(geocoder).to receive(:find_address_candidates).and_raise(exception) @@ -147,6 +147,34 @@ end end + context 'with invalid authenticity token exception' do + before do + exception = ActionController::InvalidAuthenticityToken.new('invalid token') + allow(geocoder).to receive(:find_address_candidates).and_raise(exception) + end + + it 'gets an empty pilot response' do + response = get :index + expect(response.status).to eq(422) + addresses = JSON.parse(response.body) + expect(addresses.length).to eq 0 + end + + it 'logs search analytics' do + response + expect(@analytics).to have_logged_event( + 'IdV: in person proofing location search submitted', + api_status_code: 422, + success: false, + errors: 'ArcGIS error performing operation', + result_total: 0, + exception_class: ActionController::InvalidAuthenticityToken, + exception_message: 'invalid token', + response_status_code: nil, + ) + end + end + context 'with a timeout error' do let(:server_error) { Faraday::TimeoutError.new } From a46cced1c6b5f8fccec0c36e7d989b303ff103eb Mon Sep 17 00:00:00 2001 From: Tomas Apodaca Date: Wed, 9 Aug 2023 14:13:39 -0700 Subject: [PATCH 6/6] use try --- .../idv/in_person/address_search_controller.rb | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/app/controllers/idv/in_person/address_search_controller.rb b/app/controllers/idv/in_person/address_search_controller.rb index 73cf9c7c499..256940c760c 100644 --- a/app/controllers/idv/in_person/address_search_controller.rb +++ b/app/controllers/idv/in_person/address_search_controller.rb @@ -44,17 +44,13 @@ def report_errors(error) errors ||= 'ArcGIS error performing operation' - response_status_code = if error.respond_to?(:response_status) - error.response_status - end - analytics.idv_in_person_locations_searched( success: false, errors: errors, api_status_code: Rack::Utils.status_code(remapped_error), exception_class: error.class, exception_message: error.message, - response_status_code: response_status_code, + response_status_code: error.try(:response_status), ) analytics.idv_arcgis_request_failure( @@ -63,7 +59,7 @@ def report_errors(error) 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: response_status_code, + response_status_code: error.try(:response_status), ) render json: [], status: remapped_error end