handle async error #225
handle async error #225
Conversation
include/aws/s3/private/s3_util.h
Outdated
| @@ -158,6 +158,15 @@ extern const struct aws_byte_cursor g_head_method; | |||
| AWS_S3_API | |||
| extern const struct aws_byte_cursor g_delete_method; | |||
|
|
|||
| AWS_S3_API | |||
There was a problem hiding this comment.
do we really need to expose those in api?
include/aws/s3/private/s3_util.h
Outdated
| struct aws_string *get_top_level_xml_tag_value( | ||
| struct aws_allocator *allocator, | ||
| const struct aws_byte_cursor *tag_name, | ||
| struct aws_byte_cursor *xml_body); | ||
|
|
||
| /* Get a top-level (exists directly under the root tag) tag value with expected root name. */ | ||
| AWS_S3_API | ||
| struct aws_string *get_top_level_xml_tag_value_with_root_name( |
There was a problem hiding this comment.
minor: it feels a bit awkward that function is checking the expected name. return root name and value pair instead?
There was a problem hiding this comment.
hmmm, it's a non-public helper function. I feel either way is kind awkward from the use case...
It serves the use case of getting the error code from the body if exist. If the function is as you suggested, we cannot tell the difference between an real failure or the body is a success response that don't have the tag.
There was a problem hiding this comment.
we should really come up with a better way to test helper functions without exposing them in the library interface.
Anyways, im fine with it with the new naming
| allocator, | ||
| tag_name, | ||
| expected_root_name, | ||
| &root_name_mismatch, |
There was a problem hiding this comment.
minor: can just pass in out variable here
source/s3_meta_request.c
Outdated
| /* Check the error code. Map the S3 error code to CRT error code. */ | ||
| int error_code = aws_s3_crt_error_code_from_server_error_code_string(error_code_string); | ||
| if (error_code != AWS_ERROR_S3_INTERNAL_ERROR) { | ||
| /* All error besides of internal error from async error doesn't recoverable from retry for now. */ |
There was a problem hiding this comment.
nit: doesn't -> are not
Issue #, if available:
Description of changes:
TODO: Don't have tests for the corner case.
Note: The default type may receive async error as well. CRT S3 client DO NOT handle those, and never will parse the body for default type of meta request. It's the caller's responsibility to handle it properly.-> #233By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.