Potential breaking change to stub validation logic in #3204

[jackkates]

Describe the bug

When upgrading to aws-sdk-core-3.220.1, our Ruby unit test began failing with an ArgumentError from within the aws sdk gem. It seems this error is due to new validations introduced in #3204. The change altered the handling of data that is not a Hash. Previously it was just wrapped and returned, but now http_response_stub is called which calls data_to_http_resp and triggers the ParamValidator.

ArgumentError:  
    expected params to be a hash, got class OpenStruct instead.
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/param_validator.rb:35:in `validate!'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/client_stubs.rb:278:in `data_to_http_resp'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/client_stubs.rb:263:in `http_response_stub'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/client_stubs.rb:251:in `convert_stub'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/client_stubs.rb:229:in `next_stub'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/stub_responses.rb:82:in `stub_responses'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/stub_responses.rb:73:in `block in call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/stub_responses.rb:138:in `block in span_wrapper'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/telemetry/no_op.rb:29:in `in_span'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/stub_responses.rb:134:in `span_wrapper'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/stub_responses.rb:72:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/seahorse/client/plugins/content_length.rb:24:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/streaming_retry.rb:71:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/s3_signer.rb:78:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/s3_host_id.rb:17:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/http_200_errors.rb:17:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/xml/error_handler.rb:10:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/sign.rb:53:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/transfer_encoding.rb:27:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/helpful_socket_errors.rb:12:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/s3_signer.rb:53:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/redirects.rb:20:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/user_agent.rb:78:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/http_checksum.rb:20:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/endpoint_pattern.rb:30:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/checksum_algorithm.rb:202:in `block in call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/user_agent.rb:69:in `metric'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/checksum_algorithm.rb:212:in `with_metrics'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/checksum_algorithm.rb:202:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/request_compression.rb:94:in `block in call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/request_compression.rb:104:in `with_metric'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/request_compression.rb:94:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/rest/content_type_handler.rb:27:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/express_session_auth.rb:43:in `block in call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/express_session_auth.rb:49:in `with_metric'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/express_session_auth.rb:43:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/expect_100_continue.rb:23:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/bucket_name_restrictions.rb:21:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/rest/handler.rb:10:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/recursion_detection.rb:18:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/stub_responses.rb:66:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/endpoints.rb:52:in `block in call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/user_agent.rb:69:in `metric'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/endpoints.rb:66:in `with_metrics'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/endpoints.rb:52:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/endpoint_discovery.rb:84:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/seahorse/client/plugins/endpoint.rb:46:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/param_validator.rb:26:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/seahorse/client/plugins/raise_response_errors.rb:16:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/sse_cpk.rb:24:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/dualstack.rb:21:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/checksum_algorithm.rb:17:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/plugins/accelerate.rb:43:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/checksum_algorithm.rb:169:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/jsonvalue_converter.rb:16:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/invocation_id.rb:16:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/idempotency_token.rb:19:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/param_converter.rb:26:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/response_paging.rb:12:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/seahorse/client/plugins/response_target.rb:24:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/telemetry.rb:39:in `block in call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/telemetry/no_op.rb:29:in `in_span'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/telemetry.rb:53:in `span_wrapper'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/aws-sdk-core/plugins/telemetry.rb:39:in `call'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-core-3.220.1/lib/seahorse/client/request.rb:72:in `send_request'
# /usr/local/rvm/gems/ruby-2.7.7/gems/aws-sdk-s3-1.182.0/lib/aws-sdk-s3/client.rb:8892:in `get_object'

Approximate code that sets up the test:

file_contents = "test_file"
body = double("object")
allow(body).to receive(:read).and_return(file_contents)
mock_aws_client = Aws::S3::Client.new(stub_responses: true)
mock_aws_client.stub_responses(
    :get_object,
    OpenStruct.new(body: body),
)
expect(Aws::S3::Client).to receive(:new).and_return(mock_aws_client)

and the test itself runs

s3 = Aws::S3::Client.new({region: region})
s3.get_object({bucket: bucket, key: file_name}).body.read

where the call to Client.new gets the stubbed client.

We can alter our test not to use OpenStruct, but it still appears to be a regression.

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

Existing object validation behavior when using stubs to continue

Current Behavior

New sdk version threw ArgumentError

Reproduction Steps

See description

Possible Solution

No response

Additional Information/Context

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

awk-sdk 3.220.1

Environment details (Version of Ruby, OS environment)

Linux, Ruby 2.7.7

[mullermp]

I'm sorry this broke you. The stub responses feature expects hash params as you would pass them into operations. May I ask why you are using OpenStruct for that? You could either change it to a hash or call to_h on it.

[jackkates]

Good question, I'm not really sure, I'm working in a large Ruby codebase so this test was written a few years ago and just popped on my radar today. My best guess is that the code being tested calls .body.read on the result and so an OpenStruct allows dot access and a hash doesn't.

[mullermp]

When I read that test, I'm confused on what it's actually accomplishing or testing. It seems to just be testing that the SDK works. What the test really should do is just return the body as a stubbed response, then in your application code, assert you are doing the thing you expect to do when you receive that body.

[jackkates]

Ah I see, when I first read the code I thought it would just spit the value that you give to stub_responses back out, but you actually get back the #<struct Aws::S3::Types::GetObjectOutput like normal (seemingly with a body that is a StringIO which can do .read even if the input was just a string). Therefore the OpenStruct is not needed to get the ability to do .body.

Apologies, I'm not very familiar with all this.

The snippet I posted was not complete, there's an actual test with another expect with a class using the s3 client. Still, it seems like mocking for the sake of mocking...of course if you mock the body you'll get back the body. Maybe OpenStruct was fashionable when someone wrote this test or something.

Thanks for the help, I think at least for us this is quite a minor regression that can be fixed with a hash (or better yet, deleting a bad test), you can close this if you want.

[mullermp]

No problem. I don't know if it was an "openstruct is fashionable" kind of thing - quite frankly that's just a bad test I think 😅 .

[github-actions]

This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.