Consistently populate request_id in request context metadata in XML requests

[thatothermitch]

Describe the feature

Populate request_id in the request context object when the server returns an error for an XML formatted request

Use Case

Consistently determine a request_id value for both failed and successful API responses.

For example:

begin
  response = asg.update_availability_zones(desired_zones)
rescue Aws::Errors::ServiceError => e
  say "ERROR: #{e.message} (req: #{e.context[:request_id]})", :red
else
  say "OK (req: #{response.context[:request_id]})", :green
end

instead of:

begin
  response = asg.update_availability_zones(desired_zones)
rescue Aws::Errors::ServiceError => e
  say "ERROR: #{e.message} (req: #{e.context.http_response.headers['x-amzn-requestid']})", :red
else
  say "OK (req: #{response.context[:request_id]})", :green
end

Proposed Solution

Populate request_id in request contexts in Aws::Query::Handler or populate request_id for error cases in Aws::XML::ErrorHandler

Given that Aws::JSON::Handler and Aws::REST::Handler both put this responsibility into the "protocol handler", it seems most consistent to put it in Aws::Query::Handler -- however both of those implementations rely on the HTTP response header to determine the request id, and are therefore not required to do any error message parsing to achieve this result.

It's not clear to me when it is preferable to use the HTTP header, or the XML response body, but if the response body is preferrable, perhaps Aws::XML::ErrorHandler would be a better place to locate this code.

Other Information

Aws::Json::Handler populates the request context's metadata field uning the value of the x-amzn-requestid header early in the handler list when errors are raised by the server:

    class Handler < Seahorse::Client::Handler
      ...

      # @param [Seahorse::Client::RequestContext] context
      # @return [Seahorse::Client::Response]
      def call(context)
        build_request(context)
        response = @handler.call(context)
        response.on(200..299) { |resp| parse_response(resp) }
        response.on(200..599) { |resp| apply_request_id(context) }
        response
      end

       ...

      def apply_request_id(context)
        context[:request_id] = context.http_response.headers['x-amzn-requestid']
      end

Aws::RESST::Handler also uses this pattern:

class Handler < Seahorse::Client::Handler

      def call(context)
        Rest::Request::Builder.new.apply(context)
        resp = @handler.call(context)
        resp.on(200..299) { |response| Response::Parser.new.apply(response) }
        resp.on(200..599) { |response| apply_request_id(context) } # <<<
        resp
      end

      private

      def apply_request_id(context)
        h = context.http_response.headers
        context[:request_id] = h['x-amz-request-id'] || h['x-amzn-requestid']
      end

    end

The XML-formatted equvalent Aws::Query::Handler also populates request_id, but using value returned in the response body, and only for successful responses:

     class Handler < Seahorse::Client::Handler

      ...

      # @param [Seahorse::Client::RequestContext] context
      # @return [Seahorse::Client::Response]
      def call(context)
        build_request(context)
        @handler.call(context).on_success do |response| # <<<<
          response.error = nil
          parsed = parse_xml(context) # <<<<
          if parsed.nil? || parsed == EmptyStructure
            response.data = EmptyStructure.new
          else
            response.data = parsed
          end
        end
      end

     ...
    
      def parse_xml(context)
        data = Xml::Parser.new(rules(context)).parse(xml(context))
        remove_wrapper(data, context) # <<<
      end,

     ...

      def remove_wrapper(data, context)
        if context.operation.output
          if data.response_metadata
            context[:request_id] = data.response_metadata.request_id # <<<<
          end
          data.result || Structure.new(*context.operation.output.shape.member_names)
        else
          data
        end
      end

Aws::XML::ErrorHandler seems to handle parsing of XML bodies during error cases, but it does not have any awareness of the request_id field.

As an aside, this may have been addressed specifically for EC2 as part of #1038

Acknowledgements

• I may be able to implement this feature request
• This feature might incur a breaking change

SDK version used

3.0.2

Environment details (OS name and version, etc.)

macOS Moneteray 12.3.1, various linuxes

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

[mullermp]

Thanks for opening an issue. This should be fixed and released in a new version of aws-sdk-core today.