From 15cc84dad4184b21954db1a791e96b6f43ee4de2 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Wed, 2 Oct 2024 09:45:01 +0900 Subject: [PATCH] GH-44269: [C++][FS][Azure] Catch missing exceptions on HNS support check (#44274) ### Rationale for this change `Azure::Storage::Files::DataLake::DataLakeDirectoryClient` may throw `Azure::Core::Http::TransportException` and `std::runtime_error` exceptions but they aren't caught. Arrow C++ uses `arrow::Status` not C++ exception. So we must catch all exceptions from Azure SDK for C++. ### What changes are included in this PR? Add catches `Azure::Core::Http::TransportException` and `std::exception`. ### Are these changes tested? Yes. ### Are there any user-facing changes? Yes. * GitHub Issue: #44269 Authored-by: Sutou Kouhei Signed-off-by: Sutou Kouhei --- cpp/src/arrow/filesystem/azurefs.cc | 9 ++++++++- cpp/src/arrow/filesystem/azurefs_test.cc | 18 ++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/cpp/src/arrow/filesystem/azurefs.cc b/cpp/src/arrow/filesystem/azurefs.cc index 6fe1dd3695986..a9f58c4e00c31 100644 --- a/cpp/src/arrow/filesystem/azurefs.cc +++ b/cpp/src/arrow/filesystem/azurefs.cc @@ -275,7 +275,7 @@ std::string BuildBaseUrl(const std::string& scheme, const std::string& authority } template -Status ExceptionToStatus(const Storage::StorageException& exception, +Status ExceptionToStatus(const Azure::Core::RequestFailedException& exception, PrefixArgs&&... prefix_args) { return Status::IOError(std::forward(prefix_args)..., " Azure Error: [", exception.ErrorCode, "] ", exception.what()); @@ -1381,6 +1381,13 @@ Result CheckIfHierarchicalNamespaceIsEnabled( "Check for Hierarchical Namespace support on '", adlfs_client.GetUrl(), "' failed."); } + } catch (const Azure::Core::Http::TransportException& exception) { + return ExceptionToStatus(exception, "Check for Hierarchical Namespace support on '", + adlfs_client.GetUrl(), "' failed."); + } catch (const std::exception& exception) { + return Status::UnknownError( + "Check for Hierarchical Namespace support on '", adlfs_client.GetUrl(), + "' failed: ", typeid(exception).name(), ": ", exception.what()); } } diff --git a/cpp/src/arrow/filesystem/azurefs_test.cc b/cpp/src/arrow/filesystem/azurefs_test.cc index a8087bfc16d78..494c2f7e0c9e4 100644 --- a/cpp/src/arrow/filesystem/azurefs_test.cc +++ b/cpp/src/arrow/filesystem/azurefs_test.cc @@ -2308,6 +2308,24 @@ TYPED_TEST(TestAzureFileSystemOnAllScenarios, MovePath) { this->TestMovePath(); // Tests using Azurite (the local Azure emulator) +TEST_F(TestAzuriteFileSystem, CheckIfHierarchicalNamespaceIsEnabledRuntimeError) { + ASSERT_OK(options_.ConfigureAccountKeyCredential("not-base64")); + ASSERT_OK_AND_ASSIGN(auto datalake_service_client, + options_.MakeDataLakeServiceClient()); + auto adlfs_client = datalake_service_client->GetFileSystemClient("nonexistent"); + ASSERT_RAISES(UnknownError, + internal::CheckIfHierarchicalNamespaceIsEnabled(adlfs_client, options_)); +} + +TEST_F(TestAzuriteFileSystem, CheckIfHierarchicalNamespaceIsEnabledTransportError) { + options_.dfs_storage_authority = "127.0.0.1:20000"; // Wrong port + ASSERT_OK_AND_ASSIGN(auto datalake_service_client, + options_.MakeDataLakeServiceClient()); + auto adlfs_client = datalake_service_client->GetFileSystemClient("nonexistent"); + ASSERT_RAISES(IOError, + internal::CheckIfHierarchicalNamespaceIsEnabled(adlfs_client, options_)); +} + TEST_F(TestAzuriteFileSystem, GetFileInfoSelector) { SetUpSmallFileSystemTree();