From d2f29afb1dc9a998cd13b0af75e519cb065d54f1 Mon Sep 17 00:00:00 2001 From: JinmingHu Date: Mon, 25 Jan 2021 22:22:27 +0800 Subject: [PATCH] Added additional information in `StorageException`, fixed a bug where `ClientRequestId` wasn't filled in `StorageException`. (#1455) * additional info in storage exception * changelog * clang-format --- sdk/storage/azure-storage-common/CHANGELOG.md | 8 ++++ .../storage/common/storage_exception.hpp | 2 + .../src/storage_exception.cpp | 20 +++++++++ .../test/share_file_client_test.cpp | 42 +++++++++++++++++++ 4 files changed, 72 insertions(+) diff --git a/sdk/storage/azure-storage-common/CHANGELOG.md b/sdk/storage/azure-storage-common/CHANGELOG.md index 12b703caa..d4823e75f 100644 --- a/sdk/storage/azure-storage-common/CHANGELOG.md +++ b/sdk/storage/azure-storage-common/CHANGELOG.md @@ -2,10 +2,18 @@ ## 12.0.0-beta.7 (Unreleased) +### New Features + +- Added additional information in `StorageException`. + ### Breaking Changes - `AccountSasResource::BlobContainer` was renamed to `AccountSasResource::Container`. +### Bug Fixes + +- Fixed `ClientRequestId` wasn't filled in `StorageException`. + ## 12.0.0-beta.6 (2020-01-14) ### New Features diff --git a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_exception.hpp b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_exception.hpp index e2a3126d9..31edf0b14 100644 --- a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_exception.hpp +++ b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_exception.hpp @@ -3,6 +3,7 @@ #pragma once +#include #include #include #include @@ -21,6 +22,7 @@ namespace Azure { namespace Storage { std::string RequestId; std::string ErrorCode; std::string Message; + std::map AdditionalInformation; std::unique_ptr RawResponse; static StorageException CreateFromResponse( diff --git a/sdk/storage/azure-storage-common/src/storage_exception.cpp b/sdk/storage/azure-storage-common/src/storage_exception.cpp index fc3862415..c75fb4f2b 100644 --- a/sdk/storage/azure-storage-common/src/storage_exception.cpp +++ b/sdk/storage/azure-storage-common/src/storage_exception.cpp @@ -34,6 +34,7 @@ namespace Azure { namespace Storage { std::string errorCode; std::string message; + std::map additionalInformation; if (response->GetHeaders().find(Details::HttpHeaderContentType) != response->GetHeaders().end()) { @@ -48,8 +49,10 @@ namespace Azure { namespace Storage { XmlTagError, XmlTagCode, XmlTagMessage, + XmlTagUnknown, }; std::vector path; + std::string startTagName; while (true) { @@ -60,6 +63,7 @@ namespace Azure { namespace Storage { } else if (node.Type == Details::XmlNodeType::EndTag) { + startTagName.clear(); if (path.size() > 0) { path.pop_back(); @@ -71,6 +75,7 @@ namespace Azure { namespace Storage { } else if (node.Type == Details::XmlNodeType::StartTag) { + startTagName = node.Name; if (std::strcmp(node.Name, "Error") == 0) { path.emplace_back(XmlTagName::XmlTagError); @@ -83,6 +88,10 @@ namespace Azure { namespace Storage { { path.emplace_back(XmlTagName::XmlTagMessage); } + else + { + path.emplace_back(XmlTagName::XmlTagUnknown); + } } else if (node.Type == Details::XmlNodeType::Text) { @@ -97,6 +106,15 @@ namespace Azure { namespace Storage { { message = node.Value; } + else if ( + path.size() == 2 && path[0] == XmlTagName::XmlTagError + && path[1] == XmlTagName::XmlTagUnknown) + { + if (!startTagName.empty()) + { + additionalInformation.emplace(std::move(startTagName), node.Value); + } + } } } } @@ -129,9 +147,11 @@ namespace Azure { namespace Storage { result.StatusCode = httpStatusCode; result.ReasonPhrase = std::move(reasonPhrase); result.RequestId = std::move(requestId); + result.ClientRequestId = std::move(clientRequestId); result.ErrorCode = std::move(errorCode); result.Message = std::move(message); result.RawResponse = std::move(response); + result.AdditionalInformation = std::move(additionalInformation); return result; } }} // namespace Azure::Storage diff --git a/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp b/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp index 1e339d689..426833533 100644 --- a/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp +++ b/sdk/storage/azure-storage-files-shares/test/share_file_client_test.cpp @@ -747,4 +747,46 @@ namespace Azure { namespace Storage { namespace Test { EXPECT_EQ(1536, result.ClearRanges[1].Length.GetValue()); } + TEST_F(FileShareFileClientTest, StorageExceptionAdditionalInfo) + { + Azure::Storage::Files::Shares::ShareClientOptions options; + class InvalidQueryParameterPolicy : public Azure::Core::Http::HttpPolicy { + public: + ~InvalidQueryParameterPolicy() override {} + + std::unique_ptr Clone() const override + { + return std::make_unique(*this); + } + + std::unique_ptr Send( + Core::Context const& ctx, + Core::Http::Request& request, + Core::Http::NextHttpPolicy nextHttpPolicy) const override + { + request.GetUrl().AppendQueryParameter("comp", "lease1"); + return nextHttpPolicy.Send(ctx, request); + } + }; + options.PerOperationPolicies.emplace_back(std::make_unique()); + auto fileClient = Azure::Storage::Files::Shares::ShareFileClient::CreateFromConnectionString( + StandardStorageConnectionString(), m_shareName, RandomString(), options); + try + { + fileClient.Create(1024); + } + catch (StorageException& e) + { + EXPECT_NE(e.StatusCode, Azure::Core::Http::HttpStatusCode::None); + EXPECT_FALSE(e.ReasonPhrase.empty()); + EXPECT_FALSE(e.ClientRequestId.empty()); + EXPECT_FALSE(e.RequestId.empty()); + EXPECT_FALSE(e.ErrorCode.empty()); + EXPECT_FALSE(e.Message.empty()); + EXPECT_FALSE(e.AdditionalInformation.empty()); + return; + } + FAIL(); + } + }}} // namespace Azure::Storage::Test