-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Improve error message for providers #3352
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: version-3
Are you sure you want to change the base?
Conversation
| @status_code = status_code | ||
| @body = body | ||
| msg = "HTTP #{status_code}" | ||
| msg += ": #{body[0..200]}" if body && !body.empty? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume the ..200 is to truncate large bodies/limit the size of the message?
If so, may want to use a constant for 200 - otherwise at a quick glance it looks like it matches the 200 response code.
I can't remember do we do truncation like this anywhere else? We might want to make it clear when the body is truncated (something like "..." or "[truncated]") - otherwise, it might look like an incomplete response from the service was part of the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I added it in case error messages got really long but I don't think the truncation is necessary. I'll remove it.
| set_authorization_token(request) | ||
| response = connection.request(request) | ||
| raise Non200Response unless response.code.to_i == 200 | ||
| raise Non200Response.new(response.code.to_i, response.body) unless response.code.to_i == 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to double check - .body will always give a string and not an IO right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I believe it will always return a string. From https://ruby-doc.org/3.3.2/stdlibs/net/Net/HTTP.html regarding Net::HTTP.get:
Sends a GET request and returns the HTTP response body as a string.
Fixes #3350.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
To make sure we include your contribution in the release notes, please make sure to add description entry for your changes in the "unreleased changes" section of the
CHANGELOG.mdfile (at corresponding gem). For the description entry, please make sure it lives in one line and starts withFeatureorIssuein the correct format.For generated code changes, please checkout below instructions first:
https://github.com/aws/aws-sdk-ruby/blob/version-3/CONTRIBUTING.md
Thank you for your contribution!