Fix DeleteObjects to throw DeleteObjectsException instead of AmazonS3Exception #3839
Conversation
|
|
||
| // For standard S3 error responses (like KeyTooLongError), create a DeleteObjectsException | ||
| // with the error information wrapped in a DeleteObjectsResponse | ||
| var errorResponse = S3ErrorResponseUnmarshaller.Instance.Unmarshall(context); |
There was a problem hiding this comment.
@AlexDaines does this solution guarantee that object under valid key gets deleted when sent along with invalid key in a single DeleteObjectsRequest?
|
Discussed offline, this should target V4 instead as it's not a blocking issue in V3. |
peterrsongg
changed the base branch from
aws-sdk-net-v3.7-development
to
development
peterrsongg
changed the base branch from
development
to
aws-sdk-net-v3.7-development
peterrsongg
left a comment
peterrsongg
left a comment
There was a problem hiding this comment.
Functionally, looks good. Just needs an integ test and needs to target the development branch.
| /// <param name="innerException"></param> | ||
| /// <param name="statusCode"></param> | ||
| /// <returns></returns> | ||
| public override AmazonServiceException UnmarshallException(XmlUnmarshallerContext context, Exception innerException, HttpStatusCode statusCode) |
There was a problem hiding this comment.
Can you add an integration test that would trigger this code path and asserts that a DeleteObjectsException is thrown?
| var errorResponse = S3ErrorResponseUnmarshaller.Instance.Unmarshall(context); | ||
|
|
||
| // Create a DeleteObjectsResponse containing the error information | ||
| var deleteObjectsResponse = new DeleteObjectsResponse(); |
There was a problem hiding this comment.
Since we are now implementing unmarshallException properly can we remove this logic here as part of this PR?
It seems useless since UnmarshallException will be called first and the sdk will throw an exception.
| { | ||
| var responseBodyBytes = context.GetResponseBodyBytes(); | ||
|
|
||
| // First, try to parse as a standard DeleteObjects response (for cases where S3 returns mixed results) |
There was a problem hiding this comment.
can you explain what you mean by "mixed results"?
|
Closing because this work will be taken care of as part of the effort the team is doing to put S3 on the generator which will make sure all the exception unmarshallers are written. |
5 participants
Description
Added UnmarshallException method override to
DeleteObjectsResponseUnmarshallerto ensure that allDeleteObjectsoperation failures consistently throwDeleteObjectsExceptioninstead of the genericAmazonS3Exception.The fix creates a
DeleteObjectsResponsecontaining structured error information and wraps it in aDeleteObjectsException, preserving all error details (ErrorCode,RequestId,StatusCode) while providing the expected exception type.Motivation and Context
Fixes issue #3828 reported inconsistent exception handling for
DeleteObjectsoperations.Testing
DeleteObjectsExceptionis now thrownResponse.DeleteErrorscollectionErrorCode,RequestId, andStatusCodeare properly preservedTypes of changes
Checklist
License