From 7ffb83b8a52db20ecc48dbd35c1ecc68058e1e80 Mon Sep 17 00:00:00 2001 From: Juli Tera <57973151+jterapin@users.noreply.github.com> Date: Thu, 8 Aug 2024 08:24:15 -0700 Subject: [PATCH] Handle expected boolean values for waiter error matcher (#3073) --- gems/aws-sdk-core/CHANGELOG.md | 2 + .../lib/aws-sdk-core/waiters/poller.rb | 13 +- gems/aws-sdk-core/spec/aws/waiters_spec.rb | 301 +++++++++++++----- .../spec/fixtures/waiters/dynamodb.json | 35 -- .../spec/fixtures/waiters/service.json | 61 ++++ .../spec/fixtures/waiters/waiters.json | 180 +++++++++++ 6 files changed, 470 insertions(+), 122 deletions(-) delete mode 100644 gems/aws-sdk-core/spec/fixtures/waiters/dynamodb.json create mode 100644 gems/aws-sdk-core/spec/fixtures/waiters/service.json create mode 100644 gems/aws-sdk-core/spec/fixtures/waiters/waiters.json diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 6987342bafe..c83ab7e8ec0 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Update waiters to handle expected boolean values when matching errors. + 3.201.3 (2024-07-23) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/waiters/poller.rb b/gems/aws-sdk-core/lib/aws-sdk-core/waiters/poller.rb index e03bdde3559..654350f54b1 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/waiters/poller.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/waiters/poller.rb @@ -29,7 +29,7 @@ def initialize(options = {}) # * `:retry` - The waiter may be retried. # * `:error` - The waiter encountered an un-expected error. # - # @example A trival (bad) example of a waiter that polls indefinetly. + # @example A trivial (bad) example of a waiter that polls indefinetly. # # loop do # @@ -96,8 +96,13 @@ def matches_status?(acceptor, response) end def matches_error?(acceptor, response) - Aws::Errors::ServiceError === response.error && - response.error.code == acceptor['expected'].delete('.') + case acceptor['expected'] + when 'false' then response.error.nil? + when 'true' then !response.error.nil? + else + response.error.is_a?(Aws::Errors::ServiceError) && + response.error.code == acceptor['expected'].delete('.') + end end def path(acceptor) @@ -107,7 +112,7 @@ def path(acceptor) def non_empty_array(acceptor, response, &block) if response.data values = JMESPath.search(path(acceptor), response.data) - Array === values && values.count > 0 ? yield(values) : false + values.is_a?(Array) && values.count > 0 ? yield(values) : false else false end diff --git a/gems/aws-sdk-core/spec/aws/waiters_spec.rb b/gems/aws-sdk-core/spec/aws/waiters_spec.rb index a8c9e7273ef..0b0af9143b2 100644 --- a/gems/aws-sdk-core/spec/aws/waiters_spec.rb +++ b/gems/aws-sdk-core/spec/aws/waiters_spec.rb @@ -5,49 +5,38 @@ module Aws module Waiters describe 'Waiters' do - dir = File.expand_path('../../fixtures', __FILE__) WaiterTest = ApiHelper.sample_service( - api: JSON.load_file("#{dir}/apis/dynamodb.json"), - waiters: JSON.load_file("#{dir}/waiters/dynamodb.json") + api: JSON.load_file("#{dir}/waiters/service.json"), + waiters: JSON.load_file("#{dir}/waiters/waiters.json") ) - let(:client) { - WaiterTest::Client.new(stub_responses: true) - } + let(:client) { WaiterTest::Client.new(stub_responses: true) } describe 'unknown waiters' do - it 'raises an error when attempting to wait for an unknown state' do - expect { + expect do client.wait_until(:fake_waiter_name) - }.to raise_error(Waiters::Errors::NoSuchWaiterError) + end.to raise_error(Waiters::Errors::NoSuchWaiterError) end it 'lists available waiters in the error message' do error = nil begin client.wait_until(:fake_waiter_name) - rescue => error + rescue Errors::NoSuchWaiterError => error + # Ignored end - expect(client.waiter_names.size).to be > 0 + expect(client.waiter_names.size).to be_positive client.waiter_names.each do |waiter_name| expect(error.message).to match(waiter_name.inspect) end end - end describe 'Client#wait_until' do - - it 'can match an path value' do - client.stub_responses(:describe_table, table: { table_status: 'ACTIVE' }) - r = client.wait_until(:table_exists, table_name:'foo') - expect(!!r).to be(true) - end - it 'yields the waiter to the client #wait_until block' do - client.wait_until(:table_exists) do |waiter| + client.wait_until(:generic_waiter) do |waiter| expect(waiter.delay).to eq(20) expect(waiter.interval).to eq(20) # aliased `#delay` expect(waiter.max_attempts).to eq(25) @@ -58,7 +47,7 @@ module Waiters end it 'returns true when :success thrown' do - result = client.wait_until(:table_exists) do |waiter| + result = client.wait_until(:generic_waiter) do |waiter| waiter.before_attempt do throw :success end @@ -67,7 +56,7 @@ module Waiters end it 'returns the thrown value when :success thrown' do - result = client.wait_until(:table_exists) do |waiter| + result = client.wait_until(:generic_waiter) do |waiter| waiter.before_attempt do throw :success, 'sweet!' end @@ -76,148 +65,294 @@ module Waiters end it 'triggers callbacks before sending and before waiting' do - client.stub_responses(:describe_table, 'ResourceNotFoundException') + client.stub_responses(:waiter_operation, 'ResourceNotFoundException') yielded = [] - expect { - client.wait_until(:table_exists, table_name: 'name') do |w| + expect do + client.wait_until(:generic_waiter) do |w| w.interval = 0 w.max_attempts = 2 w.before_attempt do |attempts| yielded << "before attempt #{attempts + 1}" end - w.before_wait do |attempts, resp| + w.before_wait do |attempts, _resp| yielded << "before wait #{attempts}" end end - }.to raise_error(Waiters::Errors::WaiterFailed) - expect(yielded).to eq([ - "before attempt 1", - "before wait 1", - "before attempt 2", - ]) + end.to raise_error(Waiters::Errors::WaiterFailed) + expect(yielded).to eq( + [ + 'before attempt 1', + 'before wait 1', + 'before attempt 2' + ] + ) end it 'returns when successful' do - client.stub_responses(:describe_table, table: {table_status:'ACTIVE'}) - expect { - client.wait_until(:table_exists, table_name:'table') - }.not_to raise_error + client.stub_responses(:waiter_operation, table: { table_status: 'ACTIVE' }) + expect { client.wait_until(:generic_waiter) } + .not_to raise_error end it 'returns the client response' do - client.stub_responses(:describe_table, table:{table_status:'ACTIVE'}) - resp = client.wait_until(:table_exists, table_name:'table') + client.stub_responses(:waiter_operation, table: { table_status: 'ACTIVE' }) + resp = client.wait_until(:generic_waiter) expect(resp.table.table_status).to eq('ACTIVE') end it 'raises an error when failed' do - client.stub_responses(:describe_tables, 'ResourceNotFoundException') - expect { - client.wait_until(:table_exists, {table_name:'table'}, delay:0) - }.to raise_error(Errors::WaiterFailed) + client.stub_responses(:waiter_operation, 'ResourceNotFoundException') + expect do + client.wait_until(:generic_waiter, {}, delay: 0) + end.to raise_error(Errors::WaiterFailed) end it 'raises a max attempts after the configured attempt count' do - expect { - client.wait_until(:table_exists, table_name:'table') do |w| + expect do + client.wait_until(:generic_waiter) do |w| w.interval = 0 w.max_attempts = 4 end - }.to raise_error(Errors::TooManyAttemptsError, - /stopped waiting after 4 attempts/) + end.to raise_error( + Errors::TooManyAttemptsError, + /stopped waiting after 4 attempts/ + ) end it 'sleeps between attempts' do - expect { - client.wait_until(:table_exists, table_name:'table') do |w| + expect do + client.wait_until(:generic_waiter) do |w| w.interval = 1.234 w.max_attempts = 4 expect(w).to receive(:sleep).with(1.234).exactly(3).times end - }.to raise_error(Errors::TooManyAttemptsError) + end.to raise_error(Errors::TooManyAttemptsError) end it 'catches :failure from before attempt callbacks' do - expect { - client.wait_until(:table_exists, table_name:'table') do |w| + expect do + client.wait_until(:generic_waiter) do |w| expect(w).not_to receive(:sleep) - w.before_attempt do |attempts| + w.before_attempt do throw :failure, 'custom-message' end end - }.to raise_error(Errors::WaiterFailed, 'custom-message') + end.to raise_error(Errors::WaiterFailed, 'custom-message') end it 'catches :failure from before wait callbacks' do - expect { - client.wait_until(:table_exists, table_name:'table') do |w| + expect do + client.wait_until(:generic_waiter) do |w| expect(w).not_to receive(:sleep) - w.before_wait do |attempts, response| + w.before_wait do throw :failure, 'custom-message' end end - }.to raise_error(Errors::WaiterFailed, 'custom-message') + end.to raise_error(Errors::WaiterFailed, 'custom-message') end it 'catches :success from before attempt callbacks' do - expect(client).not_to receive(:describe_table) - client.wait_until(:table_exists, table_name:'table') do |w| - w.before_attempt do |attempt| + expect(client).not_to receive(:waiter_operation) + client.wait_until(:generic_waiter) do |w| + w.before_attempt do throw :success end end end it 'catches :success from before wait callbacks' do - expect(client).not_to receive(:describe_table) - client.wait_until(:table_exists, table_name:'table') do |w| - w.before_attempt do |attempt| + expect(client).not_to receive(:waiter_operation) + client.wait_until(:generic_waiter) do |w| + w.before_attempt do throw :success end end end - describe 'matching on expectd errors' do + it 'does not trap errors raised during request' do + client.handle do |_c| + raise 'Error' + end + expect { client.wait_until(:generic_waiter) }.to raise_error('Error') + end + + it 'raises response errors' do + client.stub_responses(:waiter_operation, RuntimeError.new('oops')) + expect { client.wait_until(:generic_waiter) } + .to raise_error(Errors::WaiterFailed, /oops/) + end - it 'succeedes when an expected error is encountered' do + describe 'status matcher' do + it 'succeeds when an expected status is encountered' do attempts = 0 client.handle do |context| attempts += 1 context.http_response.status_code = 404 - Seahorse::Client::Response.new(context:context) + Seahorse::Client::Response.new(context: context) end - client.wait_until(:table_not_exists, table_name:'table') + client.wait_until(:status_matcher_waiter) expect(attempts).to be(1) end - it 'fails when an expected error is not encountered' do - expect { - client.wait_until(:table_not_exists, table_name:'table') do |w| + it 'fails when there are no matches encountered' do + expect do + client.wait_until(:status_matcher_waiter) do |w| + w.max_attempts = 5 w.delay = 0 end - }.to raise_error(Errors::TooManyAttemptsError) + end.to raise_error(Errors::TooManyAttemptsError) + end + end + + describe 'error matcher' do + context 'expected is an error code' do + it 'succeeds when matched' do + client.stub_responses(:waiter_operation, 'ResourceNotFoundException') + expect { client.wait_until(:error_matcher_with_error_code) } + .not_to raise_error + end + + it 'fails when there are no matches' do + expect { client.wait_until(:error_matcher_with_error_code) } + .to raise_error(Errors::TooManyAttemptsError) + end end + context 'expected is true' do + it 'succeeds when matched with any error' do + client.stub_responses(:waiter_operation, RuntimeError.new) + expect { client.wait_until(:error_matcher_with_true) } + .not_to raise_error + end + end + + context 'expected is false' do # meaning no errors were encountered + it 'eventually succeeds when a response with no error is received' do + client.stub_responses( + :waiter_operation, + 'ResourceNotFoundException', + { table: { table_status: 'ACTIVE' } } + ) + expect { client.wait_until(:error_matcher_with_false) } + .not_to raise_error + end + + it 'fails when matched' do + client.stub_responses(:waiter_operation, table: { table_status: 'ACTIVE' }) + expect { client.wait_until(:error_matcher_with_false_fails) } + .to raise_error(Errors::WaiterFailed) + end + + it 'retries when matched' do + client.stub_responses( + :waiter_operation, + { table: { table_status: 'ACTIVE' } }, + 'ResourceNotFoundException' + ) + retries = 0 + client.wait_until(:error_matcher_with_false_retries) { retries += 1 } + expect(retries).to be(1) + end + end end - describe 'errors' do + describe 'path matcher' do + context 'path' do + it 'retries and succeed when matched' do + client.stub_responses( + :waiter_operation, + { table: { table_status: 'UPDATING' } }, + { table: { table_status: 'ACTIVE' } } + ) + retries = 0 + expect do + client.wait_until(:path_matcher) { retries += 1 } + end.not_to raise_error + expect(retries).to be(1) + end - it 'does not trap errors raised during reqest' do - client.handle do |context| - raise 'runtime-error' + it 'fails when matched' do + client.stub_responses(:waiter_operation, table: { table_status: 'FAILED' }) + expect { client.wait_until(:path_matcher) } + .to raise_error(Errors::FailureStateError) end - expect { - client.wait_until(:table_exists, table_name:'table') - }.to raise_error('runtime-error') end - it 'raises response errors' do - client.stub_responses(:describe_table, RuntimeError.new('oops')) - expect { - client.wait_until(:table_exists, table_name:'table') - }.to raise_error(Errors::WaiterFailed, /oops/) + context 'pathAll' do + it 'retries and succeed when matched' do + client.stub_responses( + :waiter_operation, + { table_list: [{ table_status: 'UPDATING' }] }, + { table_list: [{ table_status: 'ACTIVE' }] } + ) + retries = 0 + expect do + client.wait_until(:path_all_matcher) { retries += 1 } + end.not_to raise_error + expect(retries).to be(1) + end + + it 'fails when matched' do + client.stub_responses( + :waiter_operation, + table_list: [{ table_status: 'FAILED' }] + ) + expect { client.wait_until(:path_all_matcher) } + .to raise_error(Errors::FailureStateError) + end + + it 'fails when none are matched' do + client.stub_responses( + :waiter_operation, + table_list: [{ table_status: 'UPDATING' }, { table_status: 'ACTIVE' }] + ) + expect { client.wait_until(:path_all_matcher) } + .to raise_error(Errors::TooManyAttemptsError) + end + + it 'fails when none of the path matches' do + client.stub_responses( + :waiter_operation, + table_list: [{ table_status: 'UPDATING' }, { table_status: 'ACTIVE' }] + ) + expect do + client.wait_until(:path_all_matcher) { |w| w.max_attempts = 1 } + end.to raise_error(Errors::TooManyAttemptsError) + end end + context 'pathAny' do + it 'retries and succeed when matched' do + client.stub_responses( + :waiter_operation, + { table_list: [{ table_status: 'UPDATING' }, { table_status: 'CREATING' }] }, + { table_list: [{ table_status: 'ACTIVE' }, { table_status: 'CREATING' }] } + ) + retries = 0 + expect do + client.wait_until(:path_any_matcher) { retries += 1 } + end.not_to raise_error + expect(retries).to be(1) + end + + it 'fails when matched' do + client.stub_responses( + :waiter_operation, + { table_list: [{ table_status: 'FAILED' }, { table_status: 'CREATING' }] } + ) + expect { client.wait_until(:path_any_matcher) } + .to raise_error(Errors::FailureStateError) + end + + it 'fails when none of the path matches' do + client.stub_responses( + :waiter_operation, + table_list: [{ table_status: 'CREATING' }, { table_status: 'FOO' }] + ) + expect { client.wait_until(:path_any_matcher) } + .to raise_error(Errors::TooManyAttemptsError) + end + end end end end diff --git a/gems/aws-sdk-core/spec/fixtures/waiters/dynamodb.json b/gems/aws-sdk-core/spec/fixtures/waiters/dynamodb.json deleted file mode 100644 index 46235511a56..00000000000 --- a/gems/aws-sdk-core/spec/fixtures/waiters/dynamodb.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "version": 2, - "waiters": { - "TableExists": { - "delay": 20, - "operation": "DescribeTable", - "maxAttempts": 25, - "acceptors": [ - { - "expected": "ACTIVE", - "matcher": "path", - "state": "success", - "argument": "Table.TableStatus" - }, - { - "expected": "ResourceNotFoundException", - "matcher": "error", - "state": "retry" - } - ] - }, - "TableNotExists": { - "delay": 20, - "operation": "DescribeTable", - "maxAttempts": 25, - "acceptors": [ - { - "expected": 404, - "matcher": "status", - "state": "success" - } - ] - } - } -} diff --git a/gems/aws-sdk-core/spec/fixtures/waiters/service.json b/gems/aws-sdk-core/spec/fixtures/waiters/service.json new file mode 100644 index 00000000000..9f17129bcef --- /dev/null +++ b/gems/aws-sdk-core/spec/fixtures/waiters/service.json @@ -0,0 +1,61 @@ +{ + "version":"2.0", + "metadata":{ + "endpointPrefix":"restjson", + "protocol":"rest-json" + }, + "operations": { + "WaiterOperation": { + "name": "WaiterOperation", + "http":{ + "method":"POST", + "requestUri":"/" + }, + "input":{"shape":"DescribeTableInput"}, + "output":{"shape":"DescribeTableOutput"}, + "errors":[ + {"shape":"ResourceNotFoundException"} + ] + } + }, + "shapes": { + "DescribeTableInput":{ + "type":"structure", + "members":{} + }, + "DescribeTableOutput":{ + "type":"structure", + "members":{ + "Table":{"shape":"TableDescription"}, + "TableList":{"shape":"TableList"} + } + }, + "TableDescription":{ + "type":"structure", + "members":{ + "TableStatus":{"shape":"TableStatus"} + } + }, + "TableStatus":{ + "type":"string", + "enum":[ + "CREATING", + "UPDATING", + "ACTIVE", + "FAILED" + ] + }, + "TableList":{ + "type":"list", + "member":{"shape":"TableDescription"} + }, + "ResourceNotFoundException":{ + "type":"structure", + "members":{ + "message":{"shape":"ErrorMessage"} + }, + "exception":true + }, + "ErrorMessage":{"type":"string"} + } +} \ No newline at end of file diff --git a/gems/aws-sdk-core/spec/fixtures/waiters/waiters.json b/gems/aws-sdk-core/spec/fixtures/waiters/waiters.json new file mode 100644 index 00000000000..6a2d31c8f7b --- /dev/null +++ b/gems/aws-sdk-core/spec/fixtures/waiters/waiters.json @@ -0,0 +1,180 @@ +{ + "version": 2, + "waiters": { + "GenericWaiter": { + "delay": 20, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "expected": "ACTIVE", + "matcher": "path", + "state": "success", + "argument": "Table.TableStatus" + }, + { + "expected": "ResourceNotFoundException", + "matcher": "error", + "state": "retry" + } + ] + }, + "StatusMatcherWaiter": { + "delay": 1, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "status", + "expected": 404, + "state": "success" + } + ] + }, + "ErrorMatcherWithErrorCode": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "error", + "expected": "ResourceNotFoundException", + "state": "success" + } + ] + }, + "ErrorMatcherWithTrue": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "error", + "expected": "true", + "state": "success" + } + ] + }, + "ErrorMatcherWithFalse": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "error", + "expected": "ResourceNotFoundException", + "state": "retry" + }, + { + "matcher": "error", + "expected": "false", + "state": "success" + } + ] + }, + "ErrorMatcherWithFalseFails": { + "delay": 1, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "error", + "expected": "false", + "state": "failure" + } + ] + }, + "ErrorMatcherWithFalseRetries": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher" : "error", + "expected" : "false", + "state" : "retry" + }, + { + "matcher": "error", + "expected": "ResourceNotFoundException", + "state": "success" + } + ] + }, + "PathMatcher": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "path", + "expected": "ACTIVE", + "state": "success", + "argument": "Table.TableStatus" + }, + { + "matcher": "path", + "expected": "FAILED", + "state": "failure", + "argument": "Table.TableStatus" + }, + { + "matcher": "path", + "expected": "UPDATING", + "state": "retry", + "argument": "Table.TableStatus" + } + ] + }, + "PathAllMatcher": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "pathAll", + "expected": "ACTIVE", + "state": "success", + "argument": "TableList[].TableStatus" + }, + { + "matcher": "pathAll", + "expected": "FAILED", + "state": "failure", + "argument": "TableList[].TableStatus" + }, + { + "matcher": "pathAll", + "expected": "UPDATING", + "state": "retry", + "argument": "TableList[].TableStatus" + } + ] + }, + "PathAnyMatcher": { + "delay": 0, + "operation": "WaiterOperation", + "maxAttempts": 25, + "acceptors": [ + { + "matcher": "pathAny", + "expected": "ACTIVE", + "state": "success", + "argument": "TableList[].TableStatus" + }, + { + "matcher": "pathAny", + "expected": "FAILED", + "state": "failure", + "argument": "TableList[].TableStatus" + }, + { + "matcher": "pathAny", + "expected": "UPDATING", + "state": "retry", + "argument": "TableList[].TableStatus" + } + ] + } + } +} \ No newline at end of file