Skip to content
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

private/protocol/query: Add parsing of ServiceUnavailableException #596

Merged
merged 1 commit into from
Mar 17, 2016

Conversation

jasdel
Copy link
Contributor

@jasdel jasdel commented Mar 16, 2016

Adds support to fallback and attempt to parse the
ServiceUnavailableException out of the response body. This is helpful
when the request failed and the service returned unavailable error.
Previously the error unmarshaller would fail with a serialization error.

Fix #331

Adds support to fallback and attempt to parse the
ServiceUnavailableException out of the response body. This is helpful
when the request failed and the service returned unavailable error.
Previously the error unmarshaller would fail with a serialization error.

Fix #331

// Check for unhandled error
servUnavailResp := xmlServiceUnavailableResponse{}
unavailErr := xml.Unmarshal(bodyBytes, &servUnavailResp)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be possible to just update

type xmlErrorResponse struct {
    XMLName   xml.Name `xml:"ErrorResponse,ServiceUnavailableException"`
    Code      string   `xml:"Error>Code"`
    Message   string   `xml:"Error>Message"`
    RequestID string   `xml:"RequestId"`
}

then check the name rather than unmarshal twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting didn't know you could do that with XML tags, ServiceUnavailableException isn't an attribute, but a different xml tag than ErrorResponse.

I'm not sure if xml can do that. I didn't see any mention of it in the godoc, and this play example hits an error, http://play.golang.org/p/NCECBedwc5

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. We could just get rid of the ErrorResponse and ServiceUnavailableException and just check the name that way? But I don't know if I like that, what do you think?

Other than that, :shipit:!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing the name tag all together might work if the only tags were ErrorResponse or ServiceUnavailableException i think. If there were any other tag before the two error tags that we are expecting I think the xml will not unmarshal the error we're actually looking for. The additional unmarshal should be an extremely rare event and only occurs if ErrorResponse failed to parse the error body.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, thanks for the clarification. LGTM

jasdel added a commit that referenced this pull request Mar 17, 2016
private/protocol/query: Add parsing of ServiceUnavailableException
@jasdel jasdel merged commit bd1647d into master Mar 17, 2016
@jasdel jasdel deleted the fixIssue331ServiceUnavailable branch March 17, 2016 00:07
jasdel added a commit that referenced this pull request Mar 18, 2016
xibz pushed a commit to xibz/aws-sdk-go that referenced this pull request Apr 18, 2016
xibz pushed a commit to xibz/aws-sdk-go that referenced this pull request Apr 18, 2016
skotambkar added a commit to skotambkar/aws-sdk-go that referenced this pull request May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants