Fixing function forwardToMsLink(), issue id: #21 #116
Conversation
This file contains hidden or 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
…retries Implemented exponential backoff in the `forwardToMsLink` method to handle network-related errors. - Retries are attempted up to 2 times for network errors using exponential delays. - Added delay calculation based on an exponential backoff strategy. - Improved error handling to distinguish between network and non-network errors. - Logs retry attempts for better debugging. BREAKING CHANGE: The method now enforces specific retry logic, and errors are thrown differently depending on the error type. Ensure that `isNetworkError` is properly defined when using this update.
acrosett
reviewed
Dec 14, 2024
| message: error.message, | ||
| }, | ||
| error.statusCode, | ||
| ); |
There was a problem hiding this comment.
You can do throw e here for better visibility.
Additionally, to avoid recursive retries, a non-network error should be thrown in case of attempt > retries. A bad request or a special code could do.
To clarify: we don't want ms-A to retry 3 times, then throw a network error, for ms-B to retry calling ms-A 3 times (and so on).
There was a problem hiding this comment.
Also is-network-error doesn't seems to work with axios.
const errorMessages = new Set([
'network error', // Chrome
'Failed to fetch', // Chrome
'NetworkError when attempting to fetch resource.', // Firefox
'The Internet connection appears to be offline.', // Safari 16
'Load failed', // Safari 17+
'Network request failed', // `cross-fetch`
'fetch failed', // Undici (Node.js)
'terminated', // Undici (Node.js)
]);
My best bet would be to check the Axios docs: axios/axios#2986
https://github.com/axios/axios?tab=readme-ov-file#error-types
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 can
2C51
not be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented exponential backoff in the
forwardToMsLinkmethod to handle network-related errors.BREAKING CHANGE: The method now enforces specific retry logic, and errors are thrown differently depending on the error type. Ensure that
isNetworkErroris properly defined when using this update.PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
enhancement, microservice
What is the current behavior?
Throws error at first error encounter of the function.
Issue Number: #21
What is the new behavior?
Instead of immediately throwing error, If the error is network related retry twice with exponential backoff and check if it works or else then throw the error. If the error isn't network related then throw error immediately.
Does this PR introduce a breaking change?
The method now enforces specific retry logic, and errors are thrown differently depending on the error type. Ensure that
isNetworkError(e)is properly defined when using this update.Other information