Waiter throw ClientError for unexpected error response

[ushatil]

Waiter should throw ClientError for unexpected error response

Problem

During a waiters wait() routine, if the botocore client throws a ClientError, and there is no Acceptor in the JSON configured to handle that specific ClientError response, then a less informative WaiterError is thrown.

In doing so, error response information is lost (such as the Error Code), making it impossible to accurately detect certain errors (namely, 5xx errors and ThrottlingException, per http://docs.aws.amazon.com/general/latest/gr/api-retries.html

This "WaiterError" is caused by a regular ClientError, and has no specific connection to Waiter logic, other than it happened during waiting (unlike other WaiterErrors, which are for waiter-specific failure conditions like "max retries exceeded")

Solution

If a ClientError occurs, and there is no Waiter logic ("acceptors") that handles it, then propagate a ClientError with all of the original information

BTW...

tests/unit/test_waters.TestWaitersObjects.test_unspecified_errors_propagate_error_code does not actually test that unspecified errors propogate the Error Code; it tests that they propogate the Error Message (won't matter if this change gets submitted)

If this change is rejected, please consider actually including the Error Code somewhere in WaiterError. Maybe also the HTTP Response Code?

[codecov-io]

Current coverage is 97.31%

Merging #957 into develop will not change coverage

@@            develop       #957   diff @@
==========================================
  Files            43         43          
  Lines          7014       7014          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           6826       6826          
  Misses          188        188          
  Partials          0          0          

Powered by Codecov. Last updated by ab22547...cb8e16f

[JordonPhillips]

Changing the waiter class would constitute a breaking change. I think it would be better if the error response was added as a property of the WaiterError.

[ushatil]

Agreed. I think I have a good solution; will update shortly

[jamesls]

Here's what I propose:

• Continue raising WaiterError to keep backwards compatibility
• Add a last_response attribute on a waiter error that contains the last response we received before raising a WaiterError

The rationale behind just adding a last_response instead of exposing some sort of error_code and error_message is that a WaiterError can also be thrown in non-error states (transitioning to a failure state, hitting max-attempts), and while we can always guarantee a last response, we can't necessarily guarantee that response is a "error" response.

ec2 = s.create_client('ec2')
w = ec2.get_waiter('instance_running')
try:
    w.wait(InstanceIds=['i-1234'])
except WaiterError as e:
    if 'Error' in e.last_response:
        print "Received an error:", e.last_response['Error']['Code']