Skip to content

Commit 7e1fda7

Browse files
narahavisbaluja
narahavi
authored and
sbaluja
committed
updates based on binding feedback
1 parent 41fe21d commit 7e1fda7

2 files changed

Lines changed: 149 additions & 51 deletions

File tree

‎src/aws-cpp-sdk-s3-transfer/source/s3-transfer/internal/CrtOperations.cpp‎

Lines changed: 115 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <aws/core/http/HttpClientFactory.h>
1818
#include <aws/core/http/HttpRequest.h>
1919
#include <aws/core/http/URI.h>
20+
#include <aws/core/utils/HashingUtils.h>
2021
#include <aws/core/utils/StringUtils.h>
2122
#include <aws/core/utils/logging/LogMacros.h>
2223
#include <aws/core/utils/memory/stl/AWSStringStream.h>
@@ -41,51 +42,102 @@ static const char* const CRT_OPERATIONS_LOG_TAG = "CrtOperations";
4142

4243
namespace {
4344

44-
Aws::Client::CoreErrors MapCrtErrorCode(Aws::Crt::S3::S3ErrorCode crtErrorCode) {
45-
using Aws::Crt::S3::S3ErrorCode;
46-
switch (crtErrorCode) {
47-
case S3ErrorCode::MissingContentRangeHeader:
48-
case S3ErrorCode::MissingContentLengthHeader:
49-
case S3ErrorCode::MissingETag:
50-
case S3ErrorCode::MissingUploadId:
51-
return Aws::Client::CoreErrors::MISSING_PARAMETER;
52-
case S3ErrorCode::InvalidContentRangeHeader:
53-
case S3ErrorCode::InvalidContentLengthHeader:
54-
case S3ErrorCode::InvalidRangeHeader:
55-
case S3ErrorCode::MultirangeHeaderUnsupported:
56-
case S3ErrorCode::IncorrectContentLength:
57-
case S3ErrorCode::InvalidMemoryLimitConfig:
58-
return Aws::Client::CoreErrors::INVALID_PARAMETER_VALUE;
59-
case S3ErrorCode::InternalError:
60-
case S3ErrorCode::ProxyParseFailed:
61-
case S3ErrorCode::UnsupportedProxyScheme:
62-
case S3ErrorCode::NonRecoverableAsyncError:
63-
case S3ErrorCode::MetricDataNotAvailable:
64-
case S3ErrorCode::ExceedsMemoryLimit:
65-
return Aws::Client::CoreErrors::INTERNAL_FAILURE;
66-
case S3ErrorCode::SlowDown:
67-
return Aws::Client::CoreErrors::SLOW_DOWN;
68-
case S3ErrorCode::InvalidResponseStatus:
69-
case S3ErrorCode::ResponseChecksumMismatch:
70-
case S3ErrorCode::ChecksumCalculationFailed:
71-
case S3ErrorCode::ListPartsParseFailed:
72-
case S3ErrorCode::ResumedPartChecksumMismatch:
73-
case S3ErrorCode::FileModified:
74-
case S3ErrorCode::InternalPartSizeMismatchRetryingWithRange:
75-
case S3ErrorCode::RecvFileAlreadyExists:
76-
case S3ErrorCode::RecvFileNotFound:
77-
return Aws::Client::CoreErrors::VALIDATION;
78-
case S3ErrorCode::Canceled:
79-
return Aws::Client::CoreErrors::USER_CANCELLED;
80-
case S3ErrorCode::RequestTimeTooSkewed:
81-
return Aws::Client::CoreErrors::REQUEST_TIME_TOO_SKEWED;
82-
case S3ErrorCode::S3ExpressCreateSessionFailed:
83-
return Aws::Client::CoreErrors::CLIENT_SIGNING_FAILURE;
84-
case S3ErrorCode::RequestTimeout:
85-
return Aws::Client::CoreErrors::REQUEST_TIMEOUT;
86-
default:
87-
return Aws::Client::CoreErrors::INTERNAL_FAILURE;
88-
}
45+
// CRT error names, hashed once at startup so mapping a failure is an int comparison rather than a
46+
// string one. Mirrors the GetErrorForName mappers the generated service clients use.
47+
static const int MISSING_CONTENT_RANGE_HEADER_HASH =
48+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_MISSING_CONTENT_RANGE_HEADER");
49+
static const int MISSING_CONTENT_LENGTH_HEADER_HASH =
50+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_MISSING_CONTENT_LENGTH_HEADER");
51+
static const int MISSING_ETAG_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_MISSING_ETAG");
52+
static const int MISSING_UPLOAD_ID_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_MISSING_UPLOAD_ID");
53+
static const int INVALID_CONTENT_RANGE_HEADER_HASH =
54+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INVALID_CONTENT_RANGE_HEADER");
55+
static const int INVALID_CONTENT_LENGTH_HEADER_HASH =
56+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INVALID_CONTENT_LENGTH_HEADER");
57+
static const int INVALID_RANGE_HEADER_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INVALID_RANGE_HEADER");
58+
static const int MULTIRANGE_HEADER_UNSUPPORTED_HASH =
59+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_MULTIRANGE_HEADER_UNSUPPORTED");
60+
static const int INCORRECT_CONTENT_LENGTH_HASH =
61+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INCORRECT_CONTENT_LENGTH");
62+
static const int INVALID_MEMORY_LIMIT_CONFIG_HASH =
63+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INVALID_MEMORY_LIMIT_CONFIG");
64+
static const int INTERNAL_ERROR_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INTERNAL_ERROR");
65+
static const int PROXY_PARSE_FAILED_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_PROXY_PARSE_FAILED");
66+
static const int UNSUPPORTED_PROXY_SCHEME_HASH =
67+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_UNSUPPORTED_PROXY_SCHEME");
68+
static const int NON_RECOVERABLE_ASYNC_ERROR_HASH =
69+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_NON_RECOVERABLE_ASYNC_ERROR");
70+
static const int METRIC_DATA_NOT_AVAILABLE_HASH =
71+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_METRIC_DATA_NOT_AVAILABLE");
72+
static const int EXCEEDS_MEMORY_LIMIT_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_EXCEEDS_MEMORY_LIMIT");
73+
static const int BUFFER_ALLOCATION_FAILED_HASH =
74+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_BUFFER_ALLOCATION_FAILED");
75+
static const int SLOW_DOWN_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_SLOW_DOWN");
76+
static const int INVALID_RESPONSE_STATUS_HASH =
77+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INVALID_RESPONSE_STATUS");
78+
static const int RESPONSE_CHECKSUM_MISMATCH_HASH =
79+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_RESPONSE_CHECKSUM_MISMATCH");
80+
static const int CHECKSUM_CALCULATION_FAILED_HASH =
81+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_CHECKSUM_CALCULATION_FAILED");
82+
static const int LIST_PARTS_PARSE_FAILED_HASH =
83+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_LIST_PARTS_PARSE_FAILED");
84+
static const int RESUMED_PART_CHECKSUM_MISMATCH_HASH =
85+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_RESUMED_PART_CHECKSUM_MISMATCH");
86+
static const int FILE_MODIFIED_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_FILE_MODIFIED");
87+
static const int OBJECT_MODIFIED_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_OBJECT_MODIFIED");
88+
static const int INTERNAL_BUFFER_SIZE_MISMATCH_HASH =
89+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_INTERNAL_BUFFER_SIZE_MISMATCH_RETRYING_WITH_RANGE");
90+
static const int RECV_FILE_ALREADY_EXISTS_HASH =
91+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_RECV_FILE_ALREADY_EXISTS");
92+
static const int RECV_FILE_NOT_FOUND_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_RECV_FILE_NOT_FOUND");
93+
static const int CANCELED_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_CANCELED");
94+
static const int REQUEST_TIME_TOO_SKEWED_HASH =
95+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_REQUEST_TIME_TOO_SKEWED");
96+
static const int S3EXPRESS_CREATE_SESSION_FAILED_HASH =
97+
Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3EXPRESS_CREATE_SESSION_FAILED");
98+
static const int REQUEST_TIMEOUT_HASH = Aws::Utils::HashingUtils::HashString("AWS_ERROR_S3_REQUEST_TIMEOUT");
99+
100+
Aws::Client::CoreErrors MapCrtErrorCode(int crtErrorCode) {
101+
const int hashCode = Aws::Utils::HashingUtils::HashString(Aws::Crt::ErrorName(crtErrorCode));
102+
103+
if (hashCode == MISSING_CONTENT_RANGE_HEADER_HASH || hashCode == MISSING_CONTENT_LENGTH_HEADER_HASH ||
104+
hashCode == MISSING_ETAG_HASH || hashCode == MISSING_UPLOAD_ID_HASH) {
105+
return Aws::Client::CoreErrors::MISSING_PARAMETER;
106+
}
107+
if (hashCode == INVALID_CONTENT_RANGE_HEADER_HASH || hashCode == INVALID_CONTENT_LENGTH_HEADER_HASH ||
108+
hashCode == INVALID_RANGE_HEADER_HASH || hashCode == MULTIRANGE_HEADER_UNSUPPORTED_HASH ||
109+
hashCode == INCORRECT_CONTENT_LENGTH_HASH || hashCode == INVALID_MEMORY_LIMIT_CONFIG_HASH) {
110+
return Aws::Client::CoreErrors::INVALID_PARAMETER_VALUE;
111+
}
112+
if (hashCode == INTERNAL_ERROR_HASH || hashCode == PROXY_PARSE_FAILED_HASH ||
113+
hashCode == UNSUPPORTED_PROXY_SCHEME_HASH || hashCode == NON_RECOVERABLE_ASYNC_ERROR_HASH ||
114+
hashCode == METRIC_DATA_NOT_AVAILABLE_HASH || hashCode == EXCEEDS_MEMORY_LIMIT_HASH ||
115+
hashCode == BUFFER_ALLOCATION_FAILED_HASH) {
116+
return Aws::Client::CoreErrors::INTERNAL_FAILURE;
117+
}
118+
if (hashCode == SLOW_DOWN_HASH) {
119+
return Aws::Client::CoreErrors::SLOW_DOWN;
120+
}
121+
if (hashCode == INVALID_RESPONSE_STATUS_HASH || hashCode == RESPONSE_CHECKSUM_MISMATCH_HASH ||
122+
hashCode == CHECKSUM_CALCULATION_FAILED_HASH || hashCode == LIST_PARTS_PARSE_FAILED_HASH ||
123+
hashCode == RESUMED_PART_CHECKSUM_MISMATCH_HASH || hashCode == FILE_MODIFIED_HASH ||
124+
hashCode == OBJECT_MODIFIED_HASH || hashCode == INTERNAL_BUFFER_SIZE_MISMATCH_HASH ||
125+
hashCode == RECV_FILE_ALREADY_EXISTS_HASH || hashCode == RECV_FILE_NOT_FOUND_HASH) {
126+
return Aws::Client::CoreErrors::VALIDATION;
127+
}
128+
if (hashCode == CANCELED_HASH) {
129+
return Aws::Client::CoreErrors::USER_CANCELLED;
130+
}
131+
if (hashCode == REQUEST_TIME_TOO_SKEWED_HASH) {
132+
return Aws::Client::CoreErrors::REQUEST_TIME_TOO_SKEWED;
133+
}
134+
if (hashCode == S3EXPRESS_CREATE_SESSION_FAILED_HASH) {
135+
return Aws::Client::CoreErrors::CLIENT_SIGNING_FAILURE;
136+
}
137+
if (hashCode == REQUEST_TIMEOUT_HASH) {
138+
return Aws::Client::CoreErrors::REQUEST_TIMEOUT;
139+
}
140+
return Aws::Client::CoreErrors::INTERNAL_FAILURE;
89141
}
90142

91143
// SEP checksum validation drops error bodies on non-2xx GETs; recover RequestId from headers. Returns
@@ -111,10 +163,21 @@ Aws::Http::HeaderValueCollection ToHeaderCollection(const Aws::Crt::Vector<Aws::
111163
}
112164

113165
Aws::Client::AWSError<Aws::S3::S3Errors> MapCrtError(const Aws::Crt::S3::S3MetaRequestResult& result) {
114-
const Aws::Crt::S3::S3ErrorCode crtErrorCode = result.GetErrorCode();
115-
const bool crtFailed = crtErrorCode != Aws::Crt::S3::S3ErrorCode::Success;
166+
const bool crtFailed = !result.IsSuccess();
116167
const bool hasBody = result.errorResponseBody.ptr != nullptr && result.errorResponseBody.len > 0;
117168

169+
// A cancel usually lands before any response arrives, so classify it ahead of the transport-layer
170+
// case below; it is the caller's own doing and must not be reported as a retryable network error.
171+
if (crtFailed && Aws::Utils::HashingUtils::HashString(Aws::Crt::ErrorName(result.errorCode)) == CANCELED_HASH) {
172+
Aws::StringStream ss;
173+
ss << Aws::Crt::ErrorDebugString(result.errorCode);
174+
Aws::Client::AWSError<Aws::S3::S3Errors> error(
175+
static_cast<Aws::S3::S3Errors>(Aws::Client::CoreErrors::USER_CANCELLED), "", ss.str(), /*isRetryable*/ false);
176+
// No response code is set: AWSError defaults it to REQUEST_NOT_MADE, which is what a cancel
177+
// before any response is. result.responseStatus is 0 here, which is not a valid enumerator.
178+
return error;
179+
}
180+
118181
// Transport-layer failure: no HTTP response ever came back.
119182
if (crtFailed && result.responseStatus == 0) {
120183
Aws::StringStream ss;
@@ -131,13 +194,13 @@ Aws::Client::AWSError<Aws::S3::S3Errors> MapCrtError(const Aws::Crt::S3::S3MetaR
131194
ss << Aws::Crt::ErrorDebugString(result.errorCode);
132195

133196
Aws::Client::AWSError<Aws::S3::S3Errors> error;
134-
if (crtErrorCode == Aws::Crt::S3::S3ErrorCode::InvalidResponseStatus) {
197+
if (Aws::Utils::HashingUtils::HashString(Aws::Crt::ErrorName(result.errorCode)) == INVALID_RESPONSE_STATUS_HASH) {
135198
error = Aws::Client::CoreErrorsMapper::GetErrorForHttpResponseCode(
136199
static_cast<Aws::Http::HttpResponseCode>(result.responseStatus));
137200
error.SetMessage(ss.str());
138201
} else {
139202
error = Aws::Client::AWSError<Aws::S3::S3Errors>(
140-
static_cast<Aws::S3::S3Errors>(MapCrtErrorCode(crtErrorCode)), "", ss.str(),
203+
static_cast<Aws::S3::S3Errors>(MapCrtErrorCode(result.errorCode)), "", ss.str(),
141204
/*isRetryable*/ false);
142205
}
143206
error.SetResponseCode(static_cast<Aws::Http::HttpResponseCode>(result.responseStatus));
@@ -415,6 +478,7 @@ void NotifyEarlyDownloadFailure(const std::shared_ptr<DownloadTransferState>& st
415478

416479
} // namespace
417480

481+
418482
UploadHandle CrtOperations::DispatchUpload(S3TransferManagerImpl& impl, const UploadRequest& request) {
419483
auto state = Aws::MakeShared<UploadTransferState>(CRT_OPERATIONS_LOG_TAG, request);
420484

@@ -525,7 +589,7 @@ UploadHandle CrtOperations::DispatchUpload(S3TransferManagerImpl& impl, const Up
525589
});
526590

527591
options->SetFinishCallback([state](const Aws::Crt::S3::S3MetaRequestResult& result) {
528-
if (result.GetErrorCode() == Aws::Crt::S3::S3ErrorCode::Success) {
592+
if (result.IsSuccess()) {
529593
// A listener may retain the snapshot's response, so the future gets a separate one rather than
530594
// taking the value out of the shared object.
531595
const uint64_t total = state->totalBytes ? *state->totalBytes : 0;
@@ -653,7 +717,7 @@ DownloadHandle CrtOperations::DispatchDownload(S3TransferManagerImpl& impl, cons
653717
});
654718

655719
options->SetFinishCallback([state](const Aws::Crt::S3::S3MetaRequestResult& result) {
656-
if (result.GetErrorCode() == Aws::Crt::S3::S3ErrorCode::Success) {
720+
if (result.IsSuccess()) {
657721
Internal::OptionalError finalizeError = state->request.FinalizeOnSuccess(state);
658722
if (finalizeError) {
659723
NotifyListeners(state, &DownloadProgressListener::OnTransferFailed,

‎tests/aws-cpp-sdk-s3-transfer-integration-tests/TransferManagerTests.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <RecordingProgressListener.h>
1010
#include <S3TransferTestFixture.h>
1111

12+
#include <aws/core/client/CoreErrors.h>
1213
#include <aws/core/http/HttpResponse.h>
1314
#include <aws/core/utils/StringUtils.h>
1415
#include <aws/s3-transfer/S3TransferManager.h>
@@ -382,5 +383,38 @@ TEST_F(TransferManagerTests, DownloadWithIfNoneMatchReturns304) {
382383
EXPECT_EQ(Aws::Http::HttpResponseCode::NOT_MODIFIED, outcome.GetError().GetResponseCode());
383384
}
384385

386+
// -------- CRT error surface --------
387+
// These drive real failures through the transfer manager and assert what the CRT error is reported
388+
// as, so they cover the CRT-error-to-S3Errors mapping end to end.
389+
390+
TEST_F(TransferManagerTests, CancelledUploadReportsUserCancelled) {
391+
const uint64_t size = 100 * 1024 * 1024; // large enough that cancel usually wins the race
392+
const Aws::String key = UniqueKey();
393+
Aws::String sourcePath = MakeLocalFileOfSize(size, "cancel-error-type");
394+
395+
UploadRequest request(s_bucketName, key, sourcePath);
396+
397+
S3TransferManager manager(MakeConfig());
398+
UploadHandle handle = manager.Upload(request);
399+
handle.Cancel();
400+
UploadOutcome outcome = handle.CompletionFuture().get();
401+
402+
// A cancel that lands must surface as USER_CANCELLED rather than a generic failure. If the
403+
// transfer beat the cancel there is no error to classify.
404+
if (!outcome.IsSuccess()) {
405+
EXPECT_EQ(static_cast<Aws::S3::S3Errors>(Aws::Client::CoreErrors::USER_CANCELLED),
406+
outcome.GetError().GetErrorType());
407+
// The CRT error name is carried in the message for diagnosis.
408+
EXPECT_NE(Aws::String::npos, outcome.GetError().GetMessage().find("AWS_ERROR_S3_CANCELED"));
409+
// Retrying a transfer the caller cancelled would be wrong.
410+
EXPECT_FALSE(outcome.GetError().ShouldRetry());
411+
// No HTTP exchange completed, so there is no status to report.
412+
EXPECT_EQ(Aws::Http::HttpResponseCode::REQUEST_NOT_MADE, outcome.GetError().GetResponseCode());
413+
}
414+
415+
Aws::FileSystem::RemoveFileIfExists(sourcePath.c_str());
416+
}
417+
418+
385419

386420
} // namespace

0 commit comments

Comments
 (0)