-
Notifications
You must be signed in to change notification settings - Fork 81
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
feat: handle errors in 200 response from S3 #1266
Merged
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
c20332d
handle errors in 200 response from S3
dayaffe bd7f9a4
try to fix ktlint
dayaffe 6e90ccc
fix lint and move if statement for readability
dayaffe 0741581
replace data! with data and add a guard statement
dayaffe 97baadd
add unit tests for S3ErrorIn200
dayaffe 9783120
Merge branch 'main' into day/s3-200-errors
dayaffe 31744fd
Merge branch 'main' into day/s3-200-errors
dayaffe c9fb9dc
Merge branch 'main' into day/s3-200-errors
dayaffe 1210802
move s3 error test to integration tests folder
dayaffe 79938e7
add codegenerated changes to S3Client and Models
dayaffe 68b793d
move middleware code to AWSClientRuntime
dayaffe File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
64 changes: 64 additions & 0 deletions
64
IntegrationTests/Services/AWSS3IntegrationTests/S3ErrorIn200Test.swift
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import Foundation | ||
import XCTest | ||
import AWSS3 | ||
import AWSClientRuntime | ||
import AwsCommonRuntimeKit | ||
import ClientRuntime | ||
|
||
public class MockHttpClientEngine: HttpClientEngine { | ||
|
||
// Public initializer | ||
public init() {} | ||
|
||
func successHttpResponse(request: SdkHttpRequest) -> HttpResponse { | ||
let errorResponsePayload = """ | ||
<Error> | ||
<Code>SlowDown</Code> | ||
<Message>Please reduce your request rate.</Message> | ||
<RequestId>K2H6N7ZGQT6WHCEG</RequestId> | ||
<HostId>WWoZlnK4pTjKCYn6eNV7GgOurabfqLkjbSyqTvDMGBaI9uwzyNhSaDhOCPs8paFGye7S6b/AB3A=</HostId> | ||
</Error> | ||
""" | ||
return HttpResponse( | ||
headers: request.headers, | ||
body: ByteStream.data(errorResponsePayload.data(using: .utf8)), | ||
statusCode: HttpStatusCode.ok | ||
) | ||
} | ||
|
||
public func execute(request: SdkHttpRequest) async throws -> HttpResponse { | ||
return successHttpResponse(request: request) | ||
} | ||
} | ||
|
||
class S3ErrorIn200Test: XCTestCase { | ||
|
||
override class func setUp() { | ||
AwsCommonRuntimeKit.CommonRuntimeKit.initialize() | ||
} | ||
|
||
/// S3Client throws expected error in response (200) with <Error> tag | ||
func test_foundExpectedError() async throws { | ||
let config = try await S3Client.S3ClientConfiguration(region: "us-west-2") | ||
config.httpClientEngine = MockHttpClientEngine() | ||
let client = S3Client(config: config) | ||
|
||
do { | ||
// any method on S3Client where the output shape doesnt have a stream | ||
_ = try await client.listBuckets(input: .init()) | ||
XCTFail("Expected an error to be thrown, but it was not.") | ||
} catch let error as UnknownAWSHTTPServiceError { | ||
// check for the error we added in our mock client | ||
XCTAssertEqual("Please reduce your request rate.", error.message) | ||
} catch { | ||
XCTFail("Unexpected error: \(error)") | ||
} | ||
} | ||
} |
50 changes: 50 additions & 0 deletions
50
Sources/Core/AWSClientRuntime/Middlewares/AWSS3ErrorWith200StatusXMLMiddleware.swift
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
// | ||
// Copyright Amazon.com Inc. or its affiliates. | ||
// All Rights Reserved. | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
|
||
import ClientRuntime | ||
|
||
public struct AWSS3ErrorWith200StatusXMLMiddleware<OperationStackOutput>: Middleware { | ||
public let id: String = "AWSS3ErrorWith200StatusXMLMiddleware" | ||
|
||
public init() {} | ||
|
||
public func handle<H>(context: Context, | ||
input: SdkHttpRequest, | ||
next: H) async throws -> OperationOutput<OperationStackOutput> | ||
where H: Handler, | ||
Self.MInput == H.Input, | ||
Self.MOutput == H.Output, | ||
Self.Context == H.Context { | ||
|
||
// Let the next handler in the chain process the input | ||
let response = try await next.handle(context: context, input: input) | ||
|
||
// Check if the status code is OK (200) | ||
guard response.httpResponse.statusCode == .ok else { | ||
return response | ||
} | ||
|
||
// Check if the response body contains an XML Error | ||
guard let data = try await response.httpResponse.body.readData() else { | ||
return response | ||
} | ||
|
||
let xmlString = String(data: data, encoding: .utf8) ?? "" | ||
if xmlString.contains("<Error>") { | ||
// Handle the error as a 500 Internal Server Error | ||
var modifiedResponse = response | ||
modifiedResponse.httpResponse.statusCode = .internalServerError | ||
return modifiedResponse | ||
} | ||
|
||
return response | ||
} | ||
|
||
public typealias MInput = SdkHttpRequest | ||
public typealias MOutput = OperationOutput<OperationStackOutput> | ||
public typealias Context = HttpContext | ||
} |
Oops, something went wrong.
Oops, something went wrong.
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.
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.
I'd prefer not working with XML data directly.
This can stay for now, but as part of XML project I'll look at how we can do this within existing XML handling.