From 4fc154d2b7698bfefa54793179a842602fb03079 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Tue, 30 Aug 2022 00:28:24 -0700 Subject: [PATCH 1/8] Raise proper error message instead of error code for S3 errors --- cpp/src/arrow/filesystem/s3_internal.h | 84 +++++++++++++++++++++++++- 1 file changed, 81 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index ae938c17601d4..3a090b948f492 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -88,6 +88,84 @@ inline bool IsAlreadyExists(const Aws::Client::AWSError& erro error_type == Aws::S3::S3Errors::BUCKET_ALREADY_OWNED_BY_YOU); } +std::string S3ErrorToString(int error_type) { + switch (static_cast(error_type)) { + case Aws::S3::S3Errors::INCOMPLETE_SIGNATURE: + return "INCOMPLETE_SIGNATURE"; + case Aws::S3::S3Errors::INTERNAL_FAILURE: + return "INTERNAL_FAILURE"; + case Aws::S3::S3Errors::INVALID_ACTION: + return "INVALID_ACTION"; + case Aws::S3::S3Errors::INVALID_CLIENT_TOKEN_ID: + return "INVALID_CLIENT_TOKEN_ID"; + case Aws::S3::S3Errors::INVALID_PARAMETER_COMBINATION: + return "INVALID_PARAMETER_COMBINATION"; + case Aws::S3::S3Errors::INVALID_QUERY_PARAMETER: + return "INVALID_QUERY_PARAMETER"; + case Aws::S3::S3Errors::INVALID_PARAMETER_VALUE: + return "INVALID_PARAMETER_VALUE"; + case Aws::S3::S3Errors::MISSING_ACTION: + return "MISSING_ACTION"; + case Aws::S3::S3Errors::MISSING_AUTHENTICATION_TOKEN: + return "MISSING_AUTHENTICATION_TOKEN"; + case Aws::S3::S3Errors::MISSING_PARAMETER: + return "MISSING_PARAMETER"; + case Aws::S3::S3Errors::OPT_IN_REQUIRED: + return "OPT_IN_REQUIRED"; + case Aws::S3::S3Errors::REQUEST_EXPIRED: + return "REQUEST_EXPIRED"; + case Aws::S3::S3Errors::SERVICE_UNAVAILABLE: + return "SERVICE_UNAVAILABLE"; + case Aws::S3::S3Errors::THROTTLING: + return "THROTTLING"; + case Aws::S3::S3Errors::VALIDATION: + return "VALIDATION"; + case Aws::S3::S3Errors::ACCESS_DENIED: + return "ACCESS_DENIED"; + case Aws::S3::S3Errors::RESOURCE_NOT_FOUND: + return "RESOURCE_NOT_FOUND"; + case Aws::S3::S3Errors::UNRECOGNIZED_CLIENT: + return "UNRECOGNIZED_CLIENT"; + case Aws::S3::S3Errors::MALFORMED_QUERY_STRING: + return "MALFORMED_QUERY_STRING"; + case Aws::S3::S3Errors::SLOW_DOWN: + return "SLOW_DOWN"; + case Aws::S3::S3Errors::REQUEST_TIME_TOO_SKEWED: + return "REQUEST_TIME_TOO_SKEWED"; + case Aws::S3::S3Errors::INVALID_SIGNATURE: + return "INVALID_SIGNATURE"; + case Aws::S3::S3Errors::SIGNATURE_DOES_NOT_MATCH: + return "SIGNATURE_DOES_NOT_MATCH"; + case Aws::S3::S3Errors::INVALID_ACCESS_KEY_ID: + return "INVALID_ACCESS_KEY_ID"; + case Aws::S3::S3Errors::REQUEST_TIMEOUT: + return "REQUEST_TIMEOUT"; + case Aws::S3::S3Errors::NETWORK_CONNECTION: + return "NETWORK_CONNECTION"; + case Aws::S3::S3Errors::UNKNOWN: + return "UNKNOWN"; + case Aws::S3::S3Errors::BUCKET_ALREADY_EXISTS: + return "BUCKET_ALREADY_EXISTS"; + case Aws::S3::S3Errors::BUCKET_ALREADY_OWNED_BY_YOU: + return "BUCKET_ALREADY_OWNED_BY_YOU"; + case Aws::S3::S3Errors::INVALID_OBJECT_STATE: + return "INVALID_OBJECT_STATE"; + case Aws::S3::S3Errors::NO_SUCH_BUCKET: + return "NO_SUCH_BUCKET"; + case Aws::S3::S3Errors::NO_SUCH_KEY: + return "NO_SUCH_KEY"; + case Aws::S3::S3Errors::NO_SUCH_UPLOAD: + return "NO_SUCH_UPLOAD"; + case Aws::S3::S3Errors::OBJECT_ALREADY_IN_ACTIVE_TIER: + return "OBJECT_ALREADY_IN_ACTIVE_TIER"; + case Aws::S3::S3Errors::OBJECT_NOT_IN_ACTIVE_TIER: + return "OBJECT_NOT_IN_ACTIVE_TIER"; + default: + DCHECK(false); + return "[code " + std::to_string(error_type) + "]"; + } +} + // TODO qualify error messages with a prefix indicating context // (e.g. "When completing multipart upload to bucket 'xxx', key 'xxx': ...") template @@ -96,9 +174,9 @@ Status ErrorToStatus(const std::string& prefix, const std::string& operation, // XXX Handle fine-grained error types // See // https://sdk.amazonaws.com/cpp/api/LATEST/namespace_aws_1_1_s3.html#ae3f82f8132b619b6e91c88a9f1bde371 - return Status::IOError(prefix, "AWS Error [code ", - static_cast(error.GetErrorType()), "] during ", operation, - " operation: ", error.GetMessage()); + return Status::IOError(prefix, "AWS Error ", + S3ErrorToString(static_cast(error.GetErrorType())), + " during ", operation, " operation: ", error.GetMessage()); } template From aa095b3277f3d51754961a17a358dfdf6917b2c9 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Tue, 30 Aug 2022 00:56:48 -0700 Subject: [PATCH 2/8] lint --- cpp/src/arrow/filesystem/s3_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 3a090b948f492..a72c61ef8aaf8 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -175,7 +175,7 @@ Status ErrorToStatus(const std::string& prefix, const std::string& operation, // See // https://sdk.amazonaws.com/cpp/api/LATEST/namespace_aws_1_1_s3.html#ae3f82f8132b619b6e91c88a9f1bde371 return Status::IOError(prefix, "AWS Error ", - S3ErrorToString(static_cast(error.GetErrorType())), + S3ErrorToString(static_cast(error.GetErrorType())), " during ", operation, " operation: ", error.GetMessage()); } From 56ec88fc704088ec3e3e894c15f942bc14f5050c Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 31 Aug 2022 01:33:18 -0700 Subject: [PATCH 3/8] fixes --- cpp/src/arrow/filesystem/s3_internal.h | 120 ++++++++++--------------- 1 file changed, 45 insertions(+), 75 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 3a090b948f492..6bf60a1eafbe8 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -88,81 +88,51 @@ inline bool IsAlreadyExists(const Aws::Client::AWSError& erro error_type == Aws::S3::S3Errors::BUCKET_ALREADY_OWNED_BY_YOU); } -std::string S3ErrorToString(int error_type) { - switch (static_cast(error_type)) { - case Aws::S3::S3Errors::INCOMPLETE_SIGNATURE: - return "INCOMPLETE_SIGNATURE"; - case Aws::S3::S3Errors::INTERNAL_FAILURE: - return "INTERNAL_FAILURE"; - case Aws::S3::S3Errors::INVALID_ACTION: - return "INVALID_ACTION"; - case Aws::S3::S3Errors::INVALID_CLIENT_TOKEN_ID: - return "INVALID_CLIENT_TOKEN_ID"; - case Aws::S3::S3Errors::INVALID_PARAMETER_COMBINATION: - return "INVALID_PARAMETER_COMBINATION"; - case Aws::S3::S3Errors::INVALID_QUERY_PARAMETER: - return "INVALID_QUERY_PARAMETER"; - case Aws::S3::S3Errors::INVALID_PARAMETER_VALUE: - return "INVALID_PARAMETER_VALUE"; - case Aws::S3::S3Errors::MISSING_ACTION: - return "MISSING_ACTION"; - case Aws::S3::S3Errors::MISSING_AUTHENTICATION_TOKEN: - return "MISSING_AUTHENTICATION_TOKEN"; - case Aws::S3::S3Errors::MISSING_PARAMETER: - return "MISSING_PARAMETER"; - case Aws::S3::S3Errors::OPT_IN_REQUIRED: - return "OPT_IN_REQUIRED"; - case Aws::S3::S3Errors::REQUEST_EXPIRED: - return "REQUEST_EXPIRED"; - case Aws::S3::S3Errors::SERVICE_UNAVAILABLE: - return "SERVICE_UNAVAILABLE"; - case Aws::S3::S3Errors::THROTTLING: - return "THROTTLING"; - case Aws::S3::S3Errors::VALIDATION: - return "VALIDATION"; - case Aws::S3::S3Errors::ACCESS_DENIED: - return "ACCESS_DENIED"; - case Aws::S3::S3Errors::RESOURCE_NOT_FOUND: - return "RESOURCE_NOT_FOUND"; - case Aws::S3::S3Errors::UNRECOGNIZED_CLIENT: - return "UNRECOGNIZED_CLIENT"; - case Aws::S3::S3Errors::MALFORMED_QUERY_STRING: - return "MALFORMED_QUERY_STRING"; - case Aws::S3::S3Errors::SLOW_DOWN: - return "SLOW_DOWN"; - case Aws::S3::S3Errors::REQUEST_TIME_TOO_SKEWED: - return "REQUEST_TIME_TOO_SKEWED"; - case Aws::S3::S3Errors::INVALID_SIGNATURE: - return "INVALID_SIGNATURE"; - case Aws::S3::S3Errors::SIGNATURE_DOES_NOT_MATCH: - return "SIGNATURE_DOES_NOT_MATCH"; - case Aws::S3::S3Errors::INVALID_ACCESS_KEY_ID: - return "INVALID_ACCESS_KEY_ID"; - case Aws::S3::S3Errors::REQUEST_TIMEOUT: - return "REQUEST_TIMEOUT"; - case Aws::S3::S3Errors::NETWORK_CONNECTION: - return "NETWORK_CONNECTION"; - case Aws::S3::S3Errors::UNKNOWN: - return "UNKNOWN"; - case Aws::S3::S3Errors::BUCKET_ALREADY_EXISTS: - return "BUCKET_ALREADY_EXISTS"; - case Aws::S3::S3Errors::BUCKET_ALREADY_OWNED_BY_YOU: - return "BUCKET_ALREADY_OWNED_BY_YOU"; - case Aws::S3::S3Errors::INVALID_OBJECT_STATE: - return "INVALID_OBJECT_STATE"; - case Aws::S3::S3Errors::NO_SUCH_BUCKET: - return "NO_SUCH_BUCKET"; - case Aws::S3::S3Errors::NO_SUCH_KEY: - return "NO_SUCH_KEY"; - case Aws::S3::S3Errors::NO_SUCH_UPLOAD: - return "NO_SUCH_UPLOAD"; - case Aws::S3::S3Errors::OBJECT_ALREADY_IN_ACTIVE_TIER: - return "OBJECT_ALREADY_IN_ACTIVE_TIER"; - case Aws::S3::S3Errors::OBJECT_NOT_IN_ACTIVE_TIER: - return "OBJECT_NOT_IN_ACTIVE_TIER"; +std::string_view S3ErrorToString(Aws::S3::S3Errors error_type) { + switch (error_type) { +#define S3_ERROR_CASE(NAME) \ + case Aws::S3::S3Errors::NAME: \ + return # NAME; + + S3_ERROR_CASE(INCOMPLETE_SIGNATURE) + S3_ERROR_CASE(INTERNAL_FAILURE) + S3_ERROR_CASE(INVALID_ACTION) + S3_ERROR_CASE(INVALID_CLIENT_TOKEN_ID) + S3_ERROR_CASE(INVALID_PARAMETER_COMBINATION) + S3_ERROR_CASE(INVALID_QUERY_PARAMETER) + S3_ERROR_CASE(INVALID_PARAMETER_VALUE) + S3_ERROR_CASE(MISSING_ACTION) + S3_ERROR_CASE(MISSING_AUTHENTICATION_TOKEN) + S3_ERROR_CASE(MISSING_PARAMETER) + S3_ERROR_CASE(OPT_IN_REQUIRED) + S3_ERROR_CASE(REQUEST_EXPIRED) + S3_ERROR_CASE(SERVICE_UNAVAILABLE) + S3_ERROR_CASE(THROTTLING) + S3_ERROR_CASE(VALIDATION) + S3_ERROR_CASE(ACCESS_DENIED) + S3_ERROR_CASE(RESOURCE_NOT_FOUND) + S3_ERROR_CASE(UNRECOGNIZED_CLIENT) + S3_ERROR_CASE(MALFORMED_QUERY_STRING) + S3_ERROR_CASE(SLOW_DOWN) + S3_ERROR_CASE(REQUEST_TIME_TOO_SKEWED) + S3_ERROR_CASE(INVALID_SIGNATURE) + S3_ERROR_CASE(SIGNATURE_DOES_NOT_MATCH) + S3_ERROR_CASE(INVALID_ACCESS_KEY_ID) + S3_ERROR_CASE(REQUEST_TIMEOUT) + S3_ERROR_CASE(NETWORK_CONNECTION) + S3_ERROR_CASE(UNKNOWN) + S3_ERROR_CASE(BUCKET_ALREADY_EXISTS) + S3_ERROR_CASE(BUCKET_ALREADY_OWNED_BY_YOU) + S3_ERROR_CASE(INVALID_OBJECT_STATE) + S3_ERROR_CASE(NO_SUCH_BUCKET) + S3_ERROR_CASE(NO_SUCH_KEY) + S3_ERROR_CASE(NO_SUCH_UPLOAD) + S3_ERROR_CASE(OBJECT_ALREADY_IN_ACTIVE_TIER) + S3_ERROR_CASE(OBJECT_NOT_IN_ACTIVE_TIER) + +#undef S3_ERROR_CASE default: - DCHECK(false); - return "[code " + std::to_string(error_type) + "]"; + return "[code " + std::to_string(static_cast(error_type)) + "]"; } } @@ -175,7 +145,7 @@ Status ErrorToStatus(const std::string& prefix, const std::string& operation, // See // https://sdk.amazonaws.com/cpp/api/LATEST/namespace_aws_1_1_s3.html#ae3f82f8132b619b6e91c88a9f1bde371 return Status::IOError(prefix, "AWS Error ", - S3ErrorToString(static_cast(error.GetErrorType())), + S3ErrorToString(static_cast(error.GetErrorType())), " during ", operation, " operation: ", error.GetMessage()); } From 574e4ac9897e54bbc2799a431dcf2e3a75d1985b Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 31 Aug 2022 02:15:02 -0700 Subject: [PATCH 4/8] lint --- cpp/src/arrow/filesystem/s3_internal.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 5fab91fab6ae8..91205a93064ec 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -90,9 +90,9 @@ inline bool IsAlreadyExists(const Aws::Client::AWSError& erro std::string_view S3ErrorToString(Aws::S3::S3Errors error_type) { switch (error_type) { -#define S3_ERROR_CASE(NAME) \ - case Aws::S3::S3Errors::NAME: \ - return # NAME; +#define S3_ERROR_CASE(NAME) \ + case Aws::S3::S3Errors::NAME: \ + return # NAME; S3_ERROR_CASE(INCOMPLETE_SIGNATURE) S3_ERROR_CASE(INTERNAL_FAILURE) @@ -144,9 +144,10 @@ Status ErrorToStatus(const std::string& prefix, const std::string& operation, // XXX Handle fine-grained error types // See // https://sdk.amazonaws.com/cpp/api/LATEST/namespace_aws_1_1_s3.html#ae3f82f8132b619b6e91c88a9f1bde371 - return Status::IOError(prefix, "AWS Error ", - S3ErrorToString(static_cast(error.GetErrorType())), - " during ", operation, " operation: ", error.GetMessage()); + return Status::IOError( + prefix, "AWS Error ", + S3ErrorToString(static_cast(error.GetErrorType())), " during ", + operation, " operation: ", error.GetMessage()); } template From 29da835a5e851ccd8729483b1306da9065abaf5b Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 31 Aug 2022 02:18:04 -0700 Subject: [PATCH 5/8] fix --- cpp/src/arrow/filesystem/s3_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 91205a93064ec..3507a187f8f5d 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -88,7 +88,7 @@ inline bool IsAlreadyExists(const Aws::Client::AWSError& erro error_type == Aws::S3::S3Errors::BUCKET_ALREADY_OWNED_BY_YOU); } -std::string_view S3ErrorToString(Aws::S3::S3Errors error_type) { +std::string S3ErrorToString(Aws::S3::S3Errors error_type) { switch (error_type) { #define S3_ERROR_CASE(NAME) \ case Aws::S3::S3Errors::NAME: \ From a6847bf25eb8d04e602a01c3c26af8bdfd2bef62 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 31 Aug 2022 02:30:56 -0700 Subject: [PATCH 6/8] add inline --- cpp/src/arrow/filesystem/s3_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 3507a187f8f5d..890e34654f680 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -88,7 +88,7 @@ inline bool IsAlreadyExists(const Aws::Client::AWSError& erro error_type == Aws::S3::S3Errors::BUCKET_ALREADY_OWNED_BY_YOU); } -std::string S3ErrorToString(Aws::S3::S3Errors error_type) { +inline std::string S3ErrorToString(Aws::S3::S3Errors error_type) { switch (error_type) { #define S3_ERROR_CASE(NAME) \ case Aws::S3::S3Errors::NAME: \ From d061980c1eeb974159069bdd59622e8012921d11 Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 31 Aug 2022 02:55:42 -0700 Subject: [PATCH 7/8] final lint --- cpp/src/arrow/filesystem/s3_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 890e34654f680..47826e8f4bad8 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -92,7 +92,7 @@ inline std::string S3ErrorToString(Aws::S3::S3Errors error_type) { switch (error_type) { #define S3_ERROR_CASE(NAME) \ case Aws::S3::S3Errors::NAME: \ - return # NAME; + return #NAME; S3_ERROR_CASE(INCOMPLETE_SIGNATURE) S3_ERROR_CASE(INTERNAL_FAILURE) From e24dd72f00c9e25bec084ad25c895be09ed976df Mon Sep 17 00:00:00 2001 From: Philipp Moritz Date: Wed, 31 Aug 2022 14:50:11 -0700 Subject: [PATCH 8/8] fix --- cpp/src/arrow/filesystem/s3_internal.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/s3_internal.h b/cpp/src/arrow/filesystem/s3_internal.h index 47826e8f4bad8..0943037aef0ef 100644 --- a/cpp/src/arrow/filesystem/s3_internal.h +++ b/cpp/src/arrow/filesystem/s3_internal.h @@ -123,7 +123,15 @@ inline std::string S3ErrorToString(Aws::S3::S3Errors error_type) { S3_ERROR_CASE(UNKNOWN) S3_ERROR_CASE(BUCKET_ALREADY_EXISTS) S3_ERROR_CASE(BUCKET_ALREADY_OWNED_BY_YOU) - S3_ERROR_CASE(INVALID_OBJECT_STATE) + // The following is the most recent addition to S3Errors + // and is not supported yet for some versions of the SDK + // that Apache Arrow is using. This is not a big deal + // since this error will happen only in very specialized + // settings and we will print the correct numerical error + // code as per the "default" case down below. We should + // put it back once the SDK has been upgraded in all + // Apache Arrow build configurations. + // S3_ERROR_CASE(INVALID_OBJECT_STATE) S3_ERROR_CASE(NO_SUCH_BUCKET) S3_ERROR_CASE(NO_SUCH_KEY) S3_ERROR_CASE(NO_SUCH_UPLOAD)