Add Got retry hook to log when retries happen #354
Merged
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
https://eaflood.atlassian.net/browse/WATER-4083 We had an issue with the Midlands SROC supplementary bill run. It would error and when checking the logs we found it was because the [charging-module-api](https://github.com/DEFRA/sroc-charging-module-api) was rejecting a `POST /transaction` request. ```json { "result": { "succeeded": false, "response": { "statusCode": 409, "body": { "statusCode": 409, "error": "Conflict", "message": "A transaction with Client ID 'ebee5d48-3191-4615-b86a-5bebb9026bfe' for Regime 'wrls' already exists.", "clientId": "ebee5d48-3191-4615-b86a-5bebb9026bfe" } } } } ``` But this should be impossible. We generate the ID's during the bill run. If the bill run error's all transaction records get deleted. We then found a second attempt to run the bill run succeeded. We've been unable to replicate the failed scenario but have a working theory. We believe a `POST /transaction` request is failing due to a timeout. That doesn't mean the CHA didn't receive the request, just that [Got](https://github.com/sindresorhus/got) timed out waiting for it. We have it configured to automatically retry these. So, in this scenario the CHA receives the first request and creates the transaction but **Got** times out. It then sends the same request a second time, by which point the record already exists in the CHA so it rejects it. To confirm this, and see when **Got** is retrying this change adds a new [beforeRetry](https://github.com/sindresorhus/got/blob/main/documentation/9-hooks.md#beforeretry) hook to the **Got** instance we create. In it we can log that a retry has taken place and details about it.
We did try extending the Got instance we create `_importGot()`. But that never seemed to trigger the hook. But when we added to options it then started firing.
In our opinion its clearer to see what is happening, add documentation etc if we use named functions rather than anonymous ones.
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.
https://eaflood.atlassian.net/browse/WATER-4083
We had an issue with the Midlands SROC supplementary bill run. It would error and when checking the logs we found it was because the charging-module-api was rejecting a
POST /transactionrequest.{ "result": { "succeeded": false, "response": { "statusCode": 409, "body": { "statusCode": 409, "error": "Conflict", "message": "A transaction with Client ID 'ebee5d48-3191-4615-b86a-5bebb9026bfe' for Regime 'wrls' already exists.", "clientId": "ebee5d48-3191-4615-b86a-5bebb9026bfe" } } } }But this should be impossible. We generate the ID's during the bill run. If the bill run errors all transaction records get deleted. We then found a second attempt to run the bill run succeeded.
We've been unable to replicate the failed scenario but have a working theory. We believe a
POST /transactionrequest is failing due to a timeout. That doesn't mean the CHA didn't receive the request, just that Got timed out waiting for it. We have it configured to automatically retry these. So, in this scenario, the CHA receives the first request and creates the transaction but Got times out. It then sends the same request a second time, by which point the record already exists in the CHA so it rejects it.To confirm this, and see when Got is retrying this change adds a new beforeRetry hook to the Got instance we create. In it, we can log that a retry has taken place and details about it.