search

department-of-veterans-affairs / connect_vbms

VBMS SOAP API client
Other
17 stars 9 forks source link

Handle transient errors #227

Closed monfresh closed 5 years ago

monfresh commented 5 years ago

Why: There are certain types of errors we want to exclude from Sentry. By defining those errors, and providing a transient? method, it makes it easier for consumers of this gem to handle errors.

For example, instead of having logic in Caseflow to check if an error came from BGS or VBMS, and whether or not it can be ignored, we can move that logic to the ruby-bgs and connect_vbms gems, and Caseflow now only needs to check if error.transient?.

This also eliminates the duplication of the same error handling logic in both Caseflow and Efolder.

pkarman commented 5 years ago

we'll need to make sure we update Caseflow to check for the correct rescue_from parent class(es). that has bitten us before.

monfresh commented 5 years ago

From what I can tell, this gem only raises VBMS::HTTPError (or its child classes), so rescuing from either VBMS::ClientError or VBMS:HTTPError in Caseflow should work, right?

pkarman commented 5 years ago

Yes those 2 classes should be used. Right now we also check for VBMSError which is our Caseflow class.

pkarman commented 5 years ago

looks like just a couple rubocop offenses blocking this @monfresh

monfresh commented 5 years ago

Yep, I removed VBMSError in the Caseflow PR: https://github.com/department-of-veterans-affairs/caseflow/pull/11418

monfresh commented 5 years ago

@pkarman We need to upgrade Rubocop in order for the specs to pass in Travis. I opened a separate PR for that: https://github.com/department-of-veterans-affairs/connect_vbms/pull/228

monfresh commented 5 years ago

@pkarman We are all green now. Wanna do one final check and approve if all good?