Retry policy unable to react to unexpected HTTP status codes

[rylev]

Currently we validate HTTP status codes outside of the pipeline (e.g., here. This means the retry policy has no way of retrying requests that failed by returning an HTTP status code that represents an error (e.g., 500s).

Some questions:

• Should the retry policy inspect the incoming response and retry if the HTTP status code is 5xx?
• Should the client immediately error (i.e., not retry) if the error is 4xx?
• Should we validate the status code if it is 2xx or 3xx? For example, if the Cosmos create database endpoint returns 200 instead of 201, should we error? We currently will, but this seems very restrictive.

cc @heaths @MindFlavor

[heaths]

When in doubt, I like to check with .NET: https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/core/Azure.Core/src/Pipeline/Internal/RetryPolicy.cs

Apart from retrying based on headers (we also have plans to allow customization via the response classifier, since some services like Key Vault uses a nonstandard header like "RetryAfter" (no dash), it does support an overridable check: https://github.com/Azure/azure-sdk-for-net/blob/04b28e36433fe4ada5d75c9bf2309e4503c96960/sdk/core/Azure.Core/src/ResponseClassifier.cs#L45

By default, it retries on any RequestFailedException, which in turn is thrown for any non-success: https://github.com/Azure/azure-sdk-for-net/blob/04b28e36433fe4ada5d75c9bf2309e4503c96960/sdk/core/Azure.Core/src/Pipeline/HttpClientTransport.cs#L127-L130 (seems HttpClient does this for any 400- or 500-level error).

Though it's not used by default (it was, and probably still used by overrides), we also have these retriable defaults: https://github.com/Azure/azure-sdk-for-net/blob/04b28e36433fe4ada5d75c9bf2309e4503c96960/sdk/core/Azure.Core/src/ResponseClassifier.cs#L17-L31