vsteam icon indicating copy to clipboard operation
vsteam copied to clipboard

[Suggestion] Add retry mechanism for Invoke-RestMethod in _callAPI

Open MaxClaessen opened this issue 6 years ago • 6 comments

I suggest adding a retry mechanism to the _callAPI method to avoid connection issues when using _callAPI.

A crude example of this would be:

 try {
      $tries = 0;
      $maximumTries = 5;
      
      #execute this for a maximum of $maximumTries invocations. This retries till successful
      do {
         $tries ++
         try {
            Write-Verbose "attempt number: '$tries' of '$maximumTries'"
            $resp = Invoke-RestMethod @params
            if ($resp) {
               Write-Verbose "return type: $($resp.gettype())"
               Write-Verbose $resp
            }
               return $resp
         } catch {
            Write-Warning "Attempt '$tries' of '$maximumTries' failed: trying again in 2 seconds"
            Start-Sleep -Seconds 2
         }
      } while ($tries -lt $maximumTries)
   
      # Throw an error after $maximumTries unsuccessful invocations. Doesn't need
      # a condition, since the function returns resp upon successful invocation.
      throw 'Execution failed.'
   }
   catch {
      _handleException $_
      throw
   }

MaxClaessen avatar Nov 21 '19 15:11 MaxClaessen

This might be a (on prem) server issue. @DarqueWarrior who can triage?

dickvdm avatar Nov 22 '19 08:11 dickvdm

What was the error you were getting? Did you lose connection to the server? Before I can determine if this is a good solution I need to make sure I understand the problem.

DarqueWarrior avatar Dec 21 '19 23:12 DarqueWarrior

Sorry for the late response, I've been out of the office for the past weeks due to holidays.

The error that happened was:

Invoke-RestMethod : The request was aborted: Could not create SSL/TLS secure channel.
At <path>\VSTeam\vsteam.functions.ps1:685 char:
15
+ $resp = Invoke-RestMethod @params
+ ~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidOperation: (System.Net.HttpWebRequest:HttpWebRequest) [Invoke-RestMethod], WebExc 
eption
+ FullyQualifiedErrorId : WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand
PowerShell exited with code '1'.

My assumption is that the connection to the server was terminated / unable to be processed.

To figure out if the issue is on our side I created a test script just before the holidays to keep looping calls. I ran a few tests with these scripts just before the holiday break and the API did not give a single error. Also from my own experience (and a few colleagues) the own connection to TFS was very good on this day.

Theres 2 factors that might play into the load being lower during this test:

  • Quiet in the office due to holidays
  • Recently a backup has been moved in timeslot which was causing some performance issues (delays) with TFS before. Since I just got back from my holiday break I'm gonna continue with some more testing in this area.

The improvement I'm proposing fixes not only our specific problem, but can avoid script errors with all kinds of connection issues/hiccups by retrying.

MaxClaessen avatar Jan 06 '20 09:01 MaxClaessen

@DarqueWarrior is there anything that we can do to provide (more?) information to help understand the problem?

japj avatar Feb 10 '20 14:02 japj

SSL issues are usually not transient IMHO

MichelZ avatar Feb 20 '20 19:02 MichelZ

I would be a friend of making things more stable. In cloud business, this is standard anyways.

We just need to make sure that the following things do not apply:

  • When a fault is likely to be long-lasting because this can affect the responsiveness of an application. The application might be wasting time and resources trying to repeat a request that's likely to fail.
  • For handling failures that aren't due to transient faults, such as internal exceptions caused by errors in the business logic of an application.
  • As an alternative to addressing scalability issues in a system. If an application experiences frequent busy faults, it's often a sign that the service or resource being accessed should be scaled up.

taken from: https://docs.microsoft.com/en-us/azure/architecture/patterns/retry#when-to-use-this-pattern

SebastianSchuetze avatar Feb 24 '20 19:02 SebastianSchuetze