LG-7241: Handle errors when creating USPS enrollment by sheldon-b · Pull Request #6629 · 18F/identity-idp

sheldon-b · 2022-07-26T07:23:53Z

📋 Changes

Adds error handling to the fsm v1 and fsm v2 flows so that if the call to create an enrollment in the USPS API fails the error is logged, the user is shown an error message, and the user can try again
Adds mock behavior for simulating errors from the USPS API:
- Timeout
- Client error
- Server error
- Invalid response (missing enrollment code)

📺 Demo

FSM v1 password confirm page

Note: when the user encounters this error the page is reloaded and their password is cleared from the form

FSM v2 password confirm page

Client error log example

{"name":"USPS IPPaaS enrollment failed","properties":{"event_properties":{"context":"authentication","enrollment_id":17,"exception_class":"UspsInPersonProofing::Exception::RequestEnrollException","exception_message":"Sponsor for sponsorID 5 not found","reason":"Request exception","original_exception_class":"Faraday::BadRequestError"},"new_event":false,"new_session_path":false,"new_session_success_state":false,"success_state":"PUT|/verify/review|USPS IPPaaS enrollment failed","path":"/verify/review","session_duration":209.360014,"user_id":"01d764fa-5145-48dd-8113-9e7f326dcf7a","locale":"en","user_ip":"::1","hostname":"localhost","pid":3265,"service_provider":null,"trace_id":null,"git_sha":"244c456e","git_branch":"sbachstein/lg-6872-add-error-handling","user_agent":"Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36","browser_name":"Chrome","browser_version":"104.0.0.0","browser_platform_name":"macOS","browser_platform_version":"10.15.7","browser_device_name":"Unknown","browser_mobile":false,"browser_bot":false},"time":"2022-08-11T15:54:55.360Z","id":"c3de9f97-71aa-4341-9b29-79368d6f554a","visitor_id":"90e85b4f-4a40-4429-9bbd-d4a398c9fc58","visit_id":"e8a03ee5-5fc9-4ef2-b16b-a300d3c19004"}

🔍 Testing instructions

You can try this out locally by setting usps_mock_fallback to true in your application.yml.default file and using one of the special first names to trigger a particular error:

usps waiting - timeout error
usps client error - 400 error response
usps server error - 500 error response
usps invalid response - missing enrollment code from USPS

You can switch between fsm v1 and fsm v2 by changing the value of idv_api_enabled_steps in your application.yml.default file:

FSM v1: '[]'
FSM v2: '["password_confirm", "personal_key","personal_key_confirm"]'

…d-error-handling

…creating USPS enrollment

app/controllers/api/verify/password_confirm_controller.rb

…nts in FSM v1 path

NavaTim

Reviewing before WIP removed to help speed things along.

Seems like the following AC is already handled by #6624, but maybe @sheldon-b or @tomas-nava should confirm.

Coordinate with the engineer working on LG-6089 if necessary to establish a consistent format for saving the Post Office address to the session and database; then make sure that the work for LG-6348 (PR #6580) is modified to display the address format correctly.

app/controllers/api/verify/password_confirm_controller.rb

app/services/analytics_events.rb

tomas-nava · 2022-07-27T15:38:06Z

regarding...

Seems like the following AC is already handled by #6624

Yes, that's handled in #6624; the work in this PR isn't addressing all the AC in LG-6872, the primary PRs that did that were #6615 and #6619. This is follow-on work to beef up the error handling.

…stein/lg-6872-add-error-handling

…d-error-handling

app/controllers/idv/personal_key_controller.rb

app/services/usps_in_person_proofing/enrollment_helper.rb

…d-error-handling

Fixed the specs for password confirm controller

…d-error-handling

app/services/usps_in_person_proofing/mock.rb

aduth

Looks like there's a couple failures in the build (missing fixture file?), but otherwise LGTM 👍

app/services/analytics_events.rb

aduth · 2022-08-12T13:12:10Z

app/services/usps_in_person_proofing/enrollment_helper.rb

+        address = [pii['address1'], pii['address2']].select(&:present?).join(' ')
        applicant = UspsInPersonProofing::Applicant.new(
          {
            unique_id: enrollment.usps_unique_id,
            first_name: pii['first_name'],
            last_name: pii['last_name'],
-            address: pii['address1'],
-            # do we need address2?
+            address: address,


Based on some of the testing feedback we've received, I'm hopeful this change may help fix some issues we've seen with apartment / unit numbers!

https://gsa-tts.slack.com/archives/C03R8LXFPHR/p1659991062599009?thread_ts=1659990849.775479&cid=C03R8LXFPHR

https://gsa-tts.slack.com/archives/C03R8LXFPHR/p1660062918778159?thread_ts=1660062196.659789&cid=C03R8LXFPHR

aduth · 2022-08-12T13:17:35Z

app/services/usps_in_person_proofing/mock.rb

@@ -1,26 +1,36 @@
+require './spec/support/usps_ipp_fixtures'


It catches my attention that we're loading spec code from the app code, vs. the other way around. I suppose it's probably not an issue? The alternative could be to move the mock data somewhere in this folder (e.g. app/services/usps_in_person_proofing/mock/responses/request_facilities.json), but I also don't see much prior art for that, so could leave it as-is.

(Only after I wrote this did I notice you already self-reported at #6629 (comment) 😄 )

I like the idea of having a file in mock/ for this, it does feel weird to be pulling in a spec fixture.

Okay, I moved things around in baff25e

Move all of the response json files to app/services/usps_in_person_proofing/mock/responses/

Move the UspsIppFixtures class to UspsInPersonProofing::Mock::Fixtures

Update all of the references to the fixtures to the new class

app/services/usps_in_person_proofing/enrollment_helper.rb

spec/controllers/idv/review_controller_spec.rb

…d-error-handling

config/application.yml.default

aduth

LGTM 🚀

Sheldon Bachstein added 9 commits July 22, 2022 15:46

Move spec file

3f1ada4

Use enrollment when possible to deteremine ipp flow

1235a96

Begin adding error handling

cb33ad3

Cleanup and add specs

d18e7c0

Log an event when user enrolls in IPP

d274879

Pass user's address2 to USPS IPPaaS

de7f646

Fix lint failures

a239d77

Merge remote-tracking branch 'origin/main' into sbachstein/lg-6872-ad…

dd35004

…d-error-handling

changelog: Upcoming features, In-person proofing, handle errors when …

ad69cd7

…creating USPS enrollment

zachmargolis approved these changes Jul 26, 2022

View reviewed changes

app/controllers/api/verify/password_confirm_controller.rb Outdated Show resolved Hide resolved

x341x added 3 commits July 26, 2022 19:18

fixed proofer tests and added 400 and 500 tests

991fadb

changelog: Upcoming Features, In-Person Proofing, Create IPP enrollme…

5563c83

…nts in FSM v1 path

fixed lint errors

7e531fe

NavaTim reviewed Jul 27, 2022

View reviewed changes

app/controllers/api/verify/password_confirm_controller.rb Outdated Show resolved Hide resolved

app/services/analytics_events.rb Show resolved Hide resolved

x341x and others added 3 commits July 27, 2022 16:12

added analytics events

c5ce629

Merge branch 'main' of https://github.com/18F/identity-idp into sbach…

02a8268

…stein/lg-6872-add-error-handling

Merge remote-tracking branch 'origin/main' into sbachstein/lg-6872-ad…

24d645a

…d-error-handling

aduth reviewed Jul 28, 2022

View reviewed changes

app/controllers/idv/personal_key_controller.rb Outdated Show resolved Hide resolved

aduth reviewed Jul 28, 2022

View reviewed changes

app/services/usps_in_person_proofing/enrollment_helper.rb Outdated Show resolved Hide resolved

app/services/usps_in_person_proofing/enrollment_helper.rb Outdated Show resolved Hide resolved

aduth mentioned this pull request Jul 28, 2022

Use selected in-person location details from session #6645

Merged

Sheldon Bachstein added 9 commits July 28, 2022 10:25

Merge remote-tracking branch 'origin/main' into sbachstein/lg-6872-ad…

2ec2537

…d-error-handling

Merge remote-tracking branch 'origin/main' into sbachstein/lg-6872-ad…

277365d

…d-error-handling

Working error handling

980cadc

Begin using real proofer with mock http responses

996ed92

Fixed the specs for password confirm controller

Add spec for allowing the user to retry

bbf2606

Begin using usps proofer in spec

db292d3

Add error handling for fms v1 flow

dd5e69d

Cleanup comments

1493c1f

Reorganize in-person tests

d38154e

Sheldon Bachstein added 12 commits August 9, 2022 14:27

Check that enrollment helper uses the mock

ac0e279

Merge remote-tracking branch 'origin/main' into sbachstein/lg-6872-ad…

c38cb26

…d-error-handling

Fix lint failures

15695a5

re-trigger ci build

878be98

Fix broken specs

8968cae

Add displayable error and mock errors

b9db441

Remove unused event; simplify address2 logic

7240116

Show usps error message in fsm v1 flow

31222a1

Fix broken specs

5fabfc9

Simplify rescue logic

244c456

Simulate various failure modes for usps api

dbfb86e

Simplify specs

a0c8038

sheldon-b changed the title ~~(WIP) LG-6872: Handle errors when creating USPS enrollment~~ LG-7241: Handle errors when creating USPS enrollment Aug 11, 2022

Sheldon Bachstein added 3 commits August 11, 2022 12:55

Change name of 500 usps api mock response

6f05e64

Make private methods private

f0e7c74

Reorganize methods

f1040c2

sheldon-b commented Aug 11, 2022

View reviewed changes

app/services/usps_in_person_proofing/mock.rb Outdated Show resolved Hide resolved

sheldon-b marked this pull request as ready for review August 11, 2022 20:04

aduth approved these changes Aug 12, 2022

View reviewed changes

Sheldon Bachstein added 5 commits August 12, 2022 10:43

Rename fixture file

25f6269

Merge remote-tracking branch 'origin/main' into sbachstein/lg-6872-ad…

b7de160

…d-error-handling

Move USPS API fixtures into app mock directory

baff25e

Use new class name

1b06c39

Change analytics event doc type

946262f

aduth reviewed Aug 15, 2022

View reviewed changes

config/application.yml.default Outdated Show resolved Hide resolved

Revert accidentally config change

5436c45

aduth approved these changes Aug 15, 2022

View reviewed changes

sheldon-b merged commit 22cd26d into main Aug 15, 2022

sheldon-b deleted the sbachstein/lg-6872-add-error-handling branch August 15, 2022 19:15

allthesignals mentioned this pull request Oct 21, 2022

LG-7718: usps location suggest #7186

Merged

1 task

Conversation

sheldon-b commented Jul 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📋 Changes

📺 Demo

🔍 Testing instructions

Uh oh!

Uh oh!

NavaTim left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tomas-nava commented Jul 27, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aduth Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

aduth Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

aduth Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

tomas-nava Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

sheldon-b Aug 12, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aduth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

sheldon-b commented Jul 26, 2022 •

edited

Loading