From f4e99416c9049b9db5c49023323517b5564a674e Mon Sep 17 00:00:00 2001 From: Larry Osterman Date: Fri, 11 Mar 2022 13:15:10 -0800 Subject: [PATCH] RequestFailedException includes error information #3398 (#3397) * RequestFailedException includes error information * Fixed test collateral * Added changelog entry for new core version; Added tests to verify response message contains all expected fields --- sdk/core/azure-core/CHANGELOG.md | 5 +-- .../azure-core/inc/azure/core/exception.hpp | 31 +++++++++------ sdk/core/azure-core/src/exception.cpp | 39 ++++++++++++------- .../azure-core/test/ut/exception_test.cpp | 26 +++++++++++-- 4 files changed, 69 insertions(+), 32 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 7380dd3a3..b79b2cf6e 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -3,10 +3,7 @@ ## 1.5.0-beta.1 (Unreleased) ### Features Added - -### Breaking Changes - -### Bugs Fixed +- When a `RequestFailedException` exception is thrown, the `what()` method now includes information about the HTTP request which failed. ### Other Changes diff --git a/sdk/core/azure-core/inc/azure/core/exception.hpp b/sdk/core/azure-core/inc/azure/core/exception.hpp index 83b76f834..5bb6701a6 100644 --- a/sdk/core/azure-core/inc/azure/core/exception.hpp +++ b/sdk/core/azure-core/inc/azure/core/exception.hpp @@ -22,6 +22,12 @@ namespace Azure { namespace Core { */ class RequestFailedException : public std::runtime_error { public: + /** + * @brief The entire HTTP raw response. + * + */ + std::unique_ptr RawResponse; + /** * @brief The HTTP response code. * @@ -61,12 +67,6 @@ namespace Azure { namespace Core { */ std::string Message; - /** - * @brief The entire HTTP raw response. - * - */ - std::unique_ptr RawResponse; - /** * @brief Constructs a new `%RequestFailedException` with a \p message string. * @@ -96,13 +96,14 @@ namespace Azure { namespace Core { * @param other The `%RequestFailedException` to be copied. */ RequestFailedException(const RequestFailedException& other) - : std::runtime_error(other.Message), StatusCode(other.StatusCode), - ReasonPhrase(other.ReasonPhrase), ClientRequestId(other.ClientRequestId), - RequestId(other.RequestId), ErrorCode(other.ErrorCode), Message(other.Message), + : std::runtime_error(other.Message), RawResponse( other.RawResponse ? std::make_unique(*other.RawResponse) - : nullptr) + : nullptr), + StatusCode(other.StatusCode), ReasonPhrase(other.ReasonPhrase), + ClientRequestId(other.ClientRequestId), RequestId(other.RequestId), + ErrorCode(other.ErrorCode), Message(other.Message) { } @@ -132,8 +133,14 @@ namespace Azure { namespace Core { ~RequestFailedException() = default; private: - std::string GetRawResponseField( - std::unique_ptr& rawResponse, + static std::string GetRawResponseField( + std::unique_ptr const& rawResponse, std::string fieldName); + + /** + * @brief Returns a descriptive string for this RawResponse. + */ + static std::string GetRawResponseErrorMessage( + std::unique_ptr const& rawResponse); }; }} // namespace Azure::Core diff --git a/sdk/core/azure-core/src/exception.cpp b/sdk/core/azure-core/src/exception.cpp index b29fe600d..330bb6001 100644 --- a/sdk/core/azure-core/src/exception.cpp +++ b/sdk/core/azure-core/src/exception.cpp @@ -21,25 +21,38 @@ namespace Azure { namespace Core { RequestFailedException::RequestFailedException( std::unique_ptr& rawResponse) - : RequestFailedException("Received an HTTP unsuccessful status code.") + : std::runtime_error(GetRawResponseErrorMessage(rawResponse)), + RawResponse(std::move(rawResponse)), + // These are guaranteed to always be present in the rawResponse. + StatusCode(RawResponse->GetStatusCode()), ReasonPhrase(RawResponse->GetReasonPhrase()), + // The response body may or may not have these fields + ErrorCode(GetRawResponseField(RawResponse, "code")), + Message(GetRawResponseField(RawResponse, "message")) { - const auto& headers = rawResponse->GetHeaders(); - - // These are guaranteed to always be present in the rawResponse. - StatusCode = rawResponse->GetStatusCode(); - ReasonPhrase = rawResponse->GetReasonPhrase(); - RawResponse = std::move(rawResponse); - - // The response body may or may not have these fields - ErrorCode = GetRawResponseField(RawResponse, "code"); - Message = GetRawResponseField(RawResponse, "message"); - + const auto& headers = RawResponse->GetHeaders(); ClientRequestId = HttpShared::GetHeaderOrEmptyString(headers, HttpShared::MsClientRequestId); RequestId = HttpShared::GetHeaderOrEmptyString(headers, HttpShared::MsRequestId); } + std::string RequestFailedException::GetRawResponseErrorMessage( + std::unique_ptr const& rawResponse) + { + std::string returnValue("Received an HTTP unsuccessful status code: "); + // The status code will always be present in the rawResponse. + returnValue += std::to_string( + static_cast::type>( + rawResponse->GetStatusCode())); + + // If there is a Reason phrase in the rawResponse, add it to the message. + if (!rawResponse->GetReasonPhrase().empty()) + { + returnValue += " Reason: " + rawResponse->GetReasonPhrase(); + } + return returnValue; + } + std::string RequestFailedException::GetRawResponseField( - std::unique_ptr& rawResponse, + std::unique_ptr const& rawResponse, std::string fieldName) { auto& headers = rawResponse->GetHeaders(); diff --git a/sdk/core/azure-core/test/ut/exception_test.cpp b/sdk/core/azure-core/test/ut/exception_test.cpp index 09b6db0bc..023d1e045 100644 --- a/sdk/core/azure-core/test/ut/exception_test.cpp +++ b/sdk/core/azure-core/test/ut/exception_test.cpp @@ -33,7 +33,14 @@ TEST(RequestFailedException, JSONError) EXPECT_EQ(exception.RequestId, "1"); EXPECT_EQ(exception.ClientRequestId, "2"); EXPECT_EQ(exception.ReasonPhrase, "retry please :"); - EXPECT_EQ(exception.what(), std::string("Received an HTTP unsuccessful status code.")); + EXPECT_EQ(std::string(exception.what()).find("Received an HTTP unsuccessful status code"), 0); + EXPECT_NE( + std::string(exception.what()) + .find(std::to_string( + static_cast::type>( + Azure::Core::Http::HttpStatusCode::ServiceUnavailable))), + std::string::npos); + EXPECT_NE(std::string(exception.what()).find("retry please :"), std::string::npos); } TEST(RequestFailedException, JSONErrorNoError) @@ -58,7 +65,14 @@ TEST(RequestFailedException, JSONErrorNoError) EXPECT_EQ(exception.RequestId, "1"); EXPECT_EQ(exception.ClientRequestId, "2"); EXPECT_EQ(exception.ReasonPhrase, "retry please :"); - EXPECT_EQ(exception.what(), std::string("Received an HTTP unsuccessful status code.")); + EXPECT_EQ(std::string(exception.what()).find("Received an HTTP unsuccessful status code"), 0); + EXPECT_NE( + std::string(exception.what()) + .find(std::to_string( + static_cast::type>( + Azure::Core::Http::HttpStatusCode::ServiceUnavailable))), + std::string::npos); + EXPECT_NE(std::string(exception.what()).find("retry please :"), std::string::npos); } TEST(RequestFailedException, EmptyValues) @@ -74,5 +88,11 @@ TEST(RequestFailedException, EmptyValues) EXPECT_EQ(exception.RequestId, std::string()); EXPECT_EQ(exception.ClientRequestId, std::string()); EXPECT_EQ(exception.ReasonPhrase, std::string()); - EXPECT_EQ(exception.what(), std::string("Received an HTTP unsuccessful status code.")); + EXPECT_EQ(std::string(exception.what()).find("Received an HTTP unsuccessful status code"), 0); + EXPECT_NE( + std::string(exception.what()) + .find(std::to_string( + static_cast::type>( + Azure::Core::Http::HttpStatusCode::None))), + std::string::npos); }