Retry policies don't sleep after operations time out

[chlowell]

After an operation times out, _configure_timeout raises a subclass of AzureError. Async/RetryPolicy handles AzureError by sleeping and continuing the retry_active loop. The policy won't actually send another request, because _configure_timeout will raise at the start of every loop iteration, but it will pointlessly sleep after each iteration until it exhausts its allowed retries.

This PR prevents this pointless sleeping by re-raising _configure_timeout exceptions. This is safe today because no other policy raises those exceptions. For example, transports raise ServiceRequest/ResponseError when connections time out. However, in principle something downstream from RetryPolicy could raise e.g. ServiceResponseTimeoutError given a request that should be retried; with this change, the request would not be retried. I don't expect that scenario, but I point out the possibility so you can disagree with me as you like 😸

[xiangyan99]

Given this scenario, timeout setting is 10 min and default timeout is 5 min.

We should fail on the second TimeoutError but not the first one, right?

[chlowell]

I don't understand what you mean by a first vs. second error. _configure_timeout raises one of these errors when absolute_timeout <= 0. I believe we never want to retry when that's true.

[xiangyan99]

I meant (ServiceRequestTimeoutError, ServiceResponseTimeoutError) might be raised from transport. In this case, absolute_timeout may still >0 and we do want to retry.

[chlowell]

Nothing else in the library raises these errors today but it's a fair point that a transport or policy could do so. I've removed this except block in favor of checking absolute_timeout in the broader handler.

[xiangyan99]

If absolute_timeout <= 0, we already raised in https://github.com/Azure/azure-sdk-for-python/blob/master/sdk/core/azure-core/azure/core/pipeline/policies/_retry.py#L295?

Not true?

[chlowell]

True, but the problem is that we handle that error here by sleeping and iterating again.

[xiangyan99]

I might mis-understand, why absolute_timeout can be <= 0 here?

Do you want to move

end_time = time.time() 
if absolute_timeout:
    absolute_timeout -= (end_time - start_time)

here?

[chlowell]

This code is tricky to refactor without breaking something. For now we need to decrement absolute_timeout in the finally block so it's done after every iteration, including ones that end with the continue in the try block. I'm picking low hanging fruit in this PR. I think fixing everything (e.g. #18552) will require more thought and a larger change.

As for how absolute_timeout can be non-positive here, note that nothing breaks the loop when it is. _configure_timeout raises an exception when absolute_timeout <= 0 but this except block handles that exception because it's an AzureError. If the request is retryable, the except block sleeps and we go back to the try block, where _configure_timeout raises again. Repeat until retries are exhausted.

[xiangyan99]

LGTM.

Please update changelog. :)