Skip to content

OJ-3201: Add retry handling to nino-check lambda#647

Merged
mikebeeby merged 2 commits into
mainfrom
OJ-3201-check-lambda-retry
Jul 8, 2025
Merged

OJ-3201: Add retry handling to nino-check lambda#647
mikebeeby merged 2 commits into
mainfrom
OJ-3201-check-lambda-retry

Conversation

@mikebeeby
Copy link
Copy Markdown
Contributor

@mikebeeby mikebeeby commented Jul 4, 2025

Proposed changes

What changed

Add logic to handle retry attempts in the nino-check lambda
Refactor Pdv Response parsing

Why did it change

Handle the case where the user has previously failed a NINo check and is either retrying (second or final attempt) or reaching the maximum allowed attempts.

Issue tracking

@mikebeeby mikebeeby marked this pull request as ready for review July 4, 2025 12:58
@mikebeeby mikebeeby requested review from a team as code owners July 4, 2025 12:58
Comment thread lambdas/nino-check/src/helpers/nino.ts Outdated

if (pdvRes.httpStatus === 401 && "errors" in (pdvRes.parsedBody ?? {})) {
logger.info(`Failed PDV match received.`);
const errorText = (pdvRes.parsedBody as PdvApiErrorBody).errors;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Appreciate this is existing but I'm finding this parsedBody thing really confusing because I was expecting the pdvRes to be the response that we got from HMRC. I'm also not totally clear why we need it like is it incase the response body isn't JSON? Is fetch not able to handle that for us?

Could we either change the name of pdvRes to indicate that we've added stuff to it or get rid of it?

Copy link
Copy Markdown
Contributor Author

@mikebeeby mikebeeby Jul 4, 2025

Choose a reason for hiding this comment

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

Sure I'll take a look to see if I can refactor this pdvRes logic to be clearer, or if a re-name will do.

I believe the handling of the response body got a little bit more fiddly because of this bug, which looks like a bug on HMRCs side that we have to handle. https://govukverify.atlassian.net/browse/OJ-2806

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Jul 7, 2025

Copy link
Copy Markdown
Contributor

@Caitlin-cooling Caitlin-cooling left a comment

Choose a reason for hiding this comment

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

Nice stuff!

@mikebeeby mikebeeby merged commit 0eb1bb2 into main Jul 8, 2025
17 checks passed
@mikebeeby mikebeeby deleted the OJ-3201-check-lambda-retry branch July 8, 2025 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants