-
Notifications
You must be signed in to change notification settings - Fork 1
OJ-3546: Update Experian Response Timeouts #615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… included metric and logs
845e1e5 to
bbeff6a
Compare
|
|
||
| public class KBVGateway { | ||
| private static final Logger LOGGER = LogManager.getLogger(); | ||
| private static final int RESPONSE_TIMEOUT = 24000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we do more than 24 seconds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the reason why I set it to 24s was because of this "this will need to be set to slightly less than 25-28 seconds" in the ticket - should it be 25s or over?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok, lets go with 28 seconds actually. We can reduce it if we still see lambda timeouts.
| } catch (Exception e) { | ||
| LOGGER.error("Question retrieval to the third party API threw an exception", e); | ||
| metricsService.sendErrorMetric(EXPERIAN_INITIAL_QUESTION_ERROR, "EXCEPTION"); | ||
| return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, what happens if null is returned here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When going back to the Question handler null will be the response in either the getQuestions here: return this.kbvService.getQuestions(identityIQWebServiceSoap, questionRequest); which should end up going to this throw new QuestionNotFoundException("No questions available"); since null is returned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated it now to throw the timeout at the start if the questionResponse is null, rather than going through the lambda to another error message.
| IdentityIQWebServiceSoap identityIQWebServiceSoap, SAARequest saaRequest) { | ||
| return identityIQWebServiceSoap.saa(saaRequest); | ||
|
|
||
| implementTimeout(identityIQWebServiceSoap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does the retrieval from the soap service, so I guess I can add the timeout implementation in the IdentityIQWebServiceSoapCache file preventing me from adding this implementTimeout method to two places (both the getQuestions and submitAnswers).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've moved the method and updated the unit tests for the IdentityIQWebServiceSoapCache file
…vice based on GH comments with associated tests
…on to stop earlier when response is null
…and update gateway file
3fd02ec to
2ea7204
Compare
2ea7204 to
4067e12
Compare
|



Proposed changes
What changed
IdentityIQWebServiceSoapCacheget method with 28 seconds so that if timeout occurs it can be caught in thegetQuestionsandsubmitAnswermethod.privatetoprotectedso that it can be used for unit testsquestionandquestionAnswerlambdasWhy did it change
Occasionally the Question and Answer lambdas time out due to the question and answer requests to Experian taking >29 seconds. This means the error is not actually handled as the lambda errors/fails.
Issue tracking
Checklists
Environment variables or secrets
Other considerations