Issue 87 - Handle some RequestRateTooLarge (429) errors with retries

[jasonchester]

• Added -RetryOnRequestRateTooLarge parameter to handle Response Code 429 errors in Invoke-CosmosDbRequest, Get-CosmosDbDocument and Invoke-CosmosDbStoredProcedure. This resolves some of Issue #87
• Added RU Output to Verbose Output Stream in Invoke-CosmosDbRequest with -UseWebRequest

  VERBOSE: Request Charge: 1570.5 RUs
  

• Improved ScriptLogResults when using Invoke-CosmosDbStoredProcedure -Debug -Verbose

  VERBOSE: ScriptLogResults:
  enter my procedure
  some console.log() messages
  my procedure complete
  

---

This change is 

[codecov]

Codecov Report

Merging #122 into dev will decrease coverage by 3%.
The diff coverage is 20%.

@@        Coverage Diff         @@
##           dev   #122   +/-   ##
==================================
- Coverage   89%    85%   -4%     
==================================
  Files       11     11           
  Lines      690    698    +8     
==================================
- Hits       618    600   -18     
- Misses      72     98   +26

[PlagueHO]

Sorry about taking so long with this! I'll get this reviewed in the next couple of days. Thanks again!

[jasonchester]

No problem @PlagueHO, definitely understand that this probably isn't your day job.

I know there are a lot of different ways this could be enhanced to handle retry counters and delays but I decided to start with the simplest implementation I could.

I have used this mvp a couple of times already and so far it has worked great for my use cases of getting all documents out in a continuation loop based on a simple query.

I have also been thinking about how to add unit tests as well and I would probably need to mock start-sleep in order to test. Do you know if there's any limitations on mocking built-in cmdlets? Any other ideas on testing would be welcome as well.

Thanks again for your work on this project, it's been extremely useful for me and I'm sure the rest of the community as well.

[PlagueHO]

I've been thinking about this one a bit and I think a more strategic model would be to allow the handling of the 429 to be governed by settings provided in the Context object. This would allow easy support for all functions rather than require this to be implemented on a per function basis (which will require a lot of work and many more unit tests to keep the coverage high). I've also got requirements from my performance test teams to be able to allow them to control this.

My plan for this was to allow a more feature rich retry (closer to the DocumentDB SDK) model with:

1. Max retries to be specified (0 = unlimited)
2. Back-off model: Linear, Logarithmic, Random
3. Retry Rate: (initial retry rate for logarithmic)

I'd implement this by creating a new custom CosmosDBRetryProperties class containing the properties above and allow it to be attached to the Context object. It could also be possible (in future) to allow this object to also be passed into individual functions to override anything set in the Context.

Would this be something that would work for you?

Again, I am grateful for the help and support on this and happy it is useful 😁

---

Reviewed 1 of 1 files at r2.
Review status: 1 of 4 files reviewed, all discussions resolved (waiting on @PlagueHO)

---

Comments from Reviewable

[PlagueHO]

I'm also converting the whole module to stop using Invoke-RestMethod which is going to refactor all this code.

[PlagueHO]

I've actually now ended up merging the policy based model including documentation. So unfortunately it has superseded this one 😢

[jasonchester]

@PlagueHO no worries, this looks like a much more complete implementation.