-
Notifications
You must be signed in to change notification settings - Fork 99
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
make best effort to infer unmodeled error message and name #91
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If incoming error response is unknow error, we only make best effort to infer the error type. This change puts error response body to error message so that users have the information they need. This change is necessary for Node because when error response is unknown, the body stream will never be consumed, which would use a lot of resources before stream is flushed.
mtdowling
requested changes
Jan 12, 2020
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.
Looks good! One minor suggestion.
writer.write("const parsedBody = await parseBody(output.body, context);"); | ||
} | ||
writer.write("errorCode = errorCode || \"UnknownError\";") | ||
.openBlock("response = {", "} as any;", () -> { |
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.
nit: this indentation is a little weird. I'd just do:
writer.write("errorCode = errorCode || \"UnknownError\";");
writer.openBlock("response = {", "} as any;", () -> {
mtdowling
approved these changes
Jan 13, 2020
Can anyone merge it? @mtdowling |
trivikr
added a commit
to trivikr/aws-sdk-js-v3
that referenced
this pull request
Jan 13, 2020
trivikr
added a commit
to aws/aws-sdk-js-v3
that referenced
this pull request
Jan 13, 2020
AllanZhengYP
pushed a commit
to AllanZhengYP/aws-sdk-js-v3
that referenced
this pull request
Mar 20, 2020
trivikr
added a commit
to trivikr/aws-sdk-js-v3
that referenced
this pull request
Mar 20, 2020
trivikr
added a commit
to trivikr/aws-sdk-js-v3
that referenced
this pull request
Mar 24, 2020
trivikr
added a commit
to trivikr/aws-sdk-js-v3
that referenced
this pull request
Mar 24, 2020
srchase
pushed a commit
to srchase/smithy-typescript
that referenced
this pull request
Mar 17, 2023
…ng#91) If incoming error response is unknown error, we only make best effort to infer the error type. This change puts error response body to error message so that users have the information they need. This change is necessary for Node because when error response is unknown, the body stream will never be consumed, which would use a lot of resources before stream is flushed.
srchase
pushed a commit
that referenced
this pull request
Mar 23, 2023
* add supports for service exception interface and error parser * generate new clients
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This change makes SDK parses more information when unmodeled exception comes in:
name
andmessage
onto the JS ErrorParsing error response body is required because body stream will never be consumed
otherwise. The stream will be around in memory for a whilre before they are flushed
eventually, which would use a lot of resources.
fix: #86
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.