feat: add bulk error handling notification callbacks #911
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
kleyow
commented
Jul 22, 2022
•
kleyow
commented
- Add duplicate request handling for bulk transfer request
- Add error notification handling for failed bulk transfer/failed validation/unexpected db errors.
- Improve participant validation for bulk transfers
kleyow
requested review from
vgenev,
mdebarros,
elnyry-sam-k,
lewisdaly,
oderayi,
shashi165 and
vijayg10
as code owners
kleyow
changed the title
elnyry-sam-k
reviewed
Aug 1, 2022
src/handlers/bulk/prepare/handler.js
Outdated
| if (hasDuplicateId && hasDuplicateHash) { | ||
| Logger.isErrorEnabled && Logger.error(Util.breadcrumb(location, `callbackErrorModified--${actionLetter}1`)) | ||
|
|
||
| const fspiopError = ErrorHandler.Factory.createFSPIOPError(ErrorHandler.Enums.FSPIOPErrorCodes.MODIFIED_REQUEST, 'Bulk transfer prepare duplicate') |
There was a problem hiding this comment.
The behavior on duplicate requests is that:
- If the ID (here bulkTransferID) and the message hash matches, it is essentially a "resend", so in this case - "IF the bulkTransfer is in a final state (final states for bulk - COMPLETED, REJECTED), we treat it as a GET /bulkTransfers/{ID} call and send out a PUT /bulkTransfers/{ID} with the same payload as that of a final notification. If however, the bulkTransfer is in a non-final state (the rest in the BulkTransferState enum - RECEIVED, PENDING, ACCEPTED, PROCESSING), then we do not send anything back, allowing the final notification to be sent once processing is done."
- On a bulkTransfer if a previously used ID is used but the message is different or modified (hash is different), then we send out the error callback saying its a "modified request"
Here's the section in the FSPIOP API spec discussing this behavior: https://docs.mojaloop.io/api/fspiop/v1.1/api-definition.html#duplicate-analysis-in-server-on-receiving-a-http-post-request
elnyry-sam-k
reviewed
Aug 1, 2022
src/handlers/bulk/prepare/handler.js
Outdated
| // Check to see if the resend is coming from a participant of involved | ||
| // with the bulk transfer to avoid potential data leakage. | ||
| const participants = await BulkTransferService.getParticipantsById(bulkTransferId) | ||
| if (![participants.payeeFsp, participants.payerFsp].includes(message.value.from)) { |
There was a problem hiding this comment.
This one and the next check should be unreachable code, isn't it? "hasDuplicateHash" should indicate that the headers and payload are exactly same as before.. So the check that this is coming from the same Payer FSP as before should already be covered there..
elnyry-sam-k
reviewed
Aug 1, 2022
src/handlers/bulk/prepare/handler.js
Outdated
| params.message.value.content.uriParams = { id: bulkTransferId } | ||
| // Resend should typically only occur by the Payer FSP, but lets | ||
| // handle if the same request is somehow sent by the Payer FSP. | ||
| const isPayeeRequest = participants.payeeFsp === message.value.from |
There was a problem hiding this comment.
shouldn't be needed, isn't it? (handled by "hasDuplicateHash")
elnyry-sam-k
approved these changes
Aug 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.