Add retryCodes option in HttpMiddlewareOptions

[ajimae]

Summary

• add retryCodes option to HttpMiddlewareOptions
• add unit test
• add documentation
• add flow types
• remove deprecated options from .flowconfig

Description

This adds options to allow customers retry requests on a given status (error) codes.

• Tests
  • Unit
• Documentation

Related Issue:

#1743

[changeset-bot]

⚠️ No Changeset found

Latest commit: 17b36c5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[codecov]

Codecov Report

Merging #1764 (17b36c5) into master (ab6986b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1764   +/-   ##
=======================================
  Coverage   94.28%   94.29%           
=======================================
  Files         137      137           
  Lines        4811     4817    +6     
  Branches     1275     1280    +5     
=======================================
+ Hits         4536     4542    +6     
  Misses        271      271           
  Partials        4        4           

Impacted Files	Coverage Δ
packages/sdk-middleware-http/src/http.js	96.24% <100.00%> (+0.17%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab6986b...17b36c5. Read the comment docs.

[lojzatran]

Hi @ajimae I don't think this retry is enough. As I wrote in the issue (#1743 (comment)), it should be able to retry also at network errors OR at 4xx errors if it contains a particular error message. Also we should be able to define the retry rules. Basically it should work the same as in the old sphere-node-sdk (https://github.com/commercetools/sphere-node-sdk/blob/master/src/coffee/repeater-task-queue.coffee#L7).

[ajimae]

Hi @ajimae I don't think this retry is enough. As I wrote in the issue (#1743 (comment)), it should be able to retry also at network errors OR at 4xx errors if it contains a particular error message. Also we should be able to define the retry rules. Basically it should work the same as in the old sphere-node-sdk (https://github.com/commercetools/sphere-node-sdk/blob/master/src/coffee/repeater-task-queue.coffee#L7).

Hi Lam,

Thanks for the feedback, I will work to include retries for the network errors and also specific error messages.
For the retry rules, we already have this defined and configurable, except you have a rules that is not part of what we currently have, then please you can as well let us know.

Thanks

[jenschude]

I would have made the retryCodes a property of the retryConfig. And use a default value array with the 503 status code like:

retryConfig: {
   retryCodes: [503],
  ...
}

[ajimae]

Why I added the 503 in the if() statement is because the default 503 will be overridden by the option passed in by the customer, which might not contain the 503 status code.

[jenschude]

But on the other side if you see there is a default of 503 and you don't pass it. I would call it bad luck :)

[jenschude]

retryConfig: {
   retryCodes: [503],
  ...
}

[lojzatran]

Hi @ajimae do you have an estimation when this PR will be merged and released? Thank you.

[ajimae]

Hi @ajimae do you have an estimation when this PR will be merged and released? Thank you.

I really can't say because we need @jenschude to approve.