Skip to content

Commit

Permalink
Better handling of errors from proxy / prevent infinite retries when …
Browse files Browse the repository at this point in the history
…exceptions occur

An exception that the Proxy might return will not contiain a `message` property
within the exception's `InnerMessage`, but the current error reporting logic
assumed that there would be a `message` property and then tried to call `Trim()`
on it.  This resulted in a `You cannot call a method on a null-valued expression.`
exception.

Because that exception occurred within a try/catch that was within a catch statement,
the outer catch ended up eating the exception, resulting in the retry loop continuing
to try the command, over and over again.

There are two distinct fixes in here as a result:

1. Only try to call `Trim()` if `InnerMessage.message` exists, otherwise,
   just report back `InnerMessage` itself.

2. Make sure that the outer catch statement ends with a `throw` to ensure
   that any inner exceptions thrown will propagate.  As a result, we will
   also explicitly `continue` the loop if we are going to perform an auto-retry.
  • Loading branch information
HowardWolosky committed Feb 8, 2018
1 parent 59d15a3 commit c2837ae
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 2 deletions.
2 changes: 1 addition & 1 deletion StoreBroker/StoreBroker.psd1
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
CompanyName = 'Microsoft Corporation'
Copyright = 'Copyright (C) Microsoft Corporation. All rights reserved.'

ModuleVersion = '1.14.3'
ModuleVersion = '1.14.4'
Description = 'Provides command-line access to the Windows Store Submission REST API.'

RootModule = 'StoreIngestionApi'
Expand Down
11 changes: 10 additions & 1 deletion StoreBroker/StoreIngestionApi.psm1
Original file line number Diff line number Diff line change
Expand Up @@ -2023,14 +2023,20 @@ function Invoke-SBRestMethod
{
$output += $innerMessageJson.Trim()
}
else
elseif (-not [String]::IsNullOrWhiteSpace($innerMessageJson.message))
{
$output += "$($innerMessageJson.code) : $($innerMessageJson.message.Trim())"
if ($innerMessageJson.details)
{
$output += "$($innerMessageJson.details | Format-Table | Out-String)"
}
}
else
{
# In this case, it's probably not a normal message from the API
# (possibly it's an invalid client secret error)
$output += ($innerMessageJson | Out-String)
}
}
catch [System.ArgumentException]
{
Expand Down Expand Up @@ -2095,6 +2101,7 @@ function Invoke-SBRestMethod
Write-Log -Message "This status code ($statusCode) is configured to auto-retry (via `$global:SBAutoRetryErrorCodes). StoreBroker will auto-retry (attempt #$numRetries) in $retryDelayMin minute(s). Sleeping..." -Level Warning
Start-Sleep -Seconds ($retryDelayMin * 60)
$retryDelayMin = $retryDelayMin * 2 # Exponential sleep increase for next retry
continue # let's get back to the start of the loop again, no need to process anything further in this catch
}
}
else
Expand All @@ -2103,6 +2110,8 @@ function Invoke-SBRestMethod
Set-TelemetryException -Exception $ex -ErrorBucket $errorBucket -Properties $localTelemetryProperties
throw $newLineOutput
}

throw # ensure that any inner exception that was thrown continues to propagate
}
}
while ($true) # infinite loop for retrying is ok, since we early return in the postive case, and throw an exception in the failure case.
Expand Down

0 comments on commit c2837ae

Please sign in to comment.