S3Client::CompleteMultipartUpload returns success for failed requests

[dyfrgi]

Complete Multipart Upload is a bit weird in the REST API. Rather than return a failed HTTP code for a failure, it returns 200 OK immediately and then may later, at its leisure, send back an error. This is described in the API docs here:
http://docs.aws.amazon.com/AmazonS3/latest/API/mpUploadComplete.html

However, if an error is received for a Complete Multipart Upload along with a 200 OK, as described in the example given in the doc there, the CompleteMultipartUploadOutcome returned by S3Client::CompleteMultipartUpload will have have IsSuccess()==true.

We are specifically receiving 200 OK SlowDown errors, but I expect other errors may occur as there's one given as an example in the API doc.

[JonathanHenson]

Thanks for reporting will get it queued up for fix.

[marcomagdy]

This has been fixed in v1.3.30

[marcomagdy]

We'll have to revert this change f5c0f79 See #781 for more details.

[andriy-bulynko]

Hi! We're hitting this bug as well. Do you have any idea when you might post a fix? Thanks!

[pitrou]

Why isn't this still fixed? This sounds like a critical bug. We just got word that our users can be hit by it: https://issues.apache.org/jira/browse/ARROW-14523

[pitrou]

@wps132230 @sdavtaker @KaibaLopez

[pitrou]

I'll also note that the docstring, while stating the problem, is unhelpful as to how to solve it:

Processing of a Complete Multipart Upload request could take several minutes to complete. After Amazon S3 begins processing the request, it sends an HTTP response header that specifies a 200 OK response. While processing is in progress, Amazon S3 periodically sends white space characters to keep the connection from timing out. Because a request could fail after the initial 200 OK response has been sent, it is important that you check the response body to determine whether the request succeeded.

[pitrou]

Our own workaround is to intercept XML error response using a DataReceivedHandler. Unfortunately, we can't really test it (those errors seem quite sporadic), but it should theoretically work.

[sdavtaker]

It seems like the Bug in TinyXML2 was resolved and we recently and we should be able to merge this fix again.

[pitrou]

As a sidenote, the original fix seems costly if it blindly parses any HTTP response (even a huge binary payload) as XML.

[pitrou]

So, here is an example of how this could be fixed in the AWS SDK, IMHO:

• add a method virtual bool HasEmbeddedError(Aws::IOStream& body, Aws::Client::AWSError<Aws::Client::CoreErrors>* error) in AmazonWebServiceRequest; default implementation returns false and leaves error unchanged
• override that method in CompleteMultipartUploadRequest and make it parse the XML payload for embedded errors
• in AttemptOneRequest, call HasBodyError for successfull HTTP responses

[sdavtaker]

Thanks @pitrou for the proposed solution, it sounds like a good approach.

[jvictor0]

I agree with @pitrou that this is a critial issue. Our customers seem to have hit it as well.

[sbiscigl]

the fix has been merged and will be released with 1.9.333

[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.