refactor(libstore): improve aws-auth error handling

Replace nullptr returns with proper exception throwing in AWS credential
provider creation methods. This provides better error diagnostics and
follows C++ best practices for error handling.

Changes:
- Add AwsCrtInitializationError exception type for AWS CRT initialization failures
- Throw exceptions instead of returning nullptr in createDefault() and createProfile()
- Update test files to handle exceptions appropriately
- Preserve graceful handling in filetransfer.cc with existing try-catch block

This addresses reviewer feedback about improving error handling to be more
explicit and informative.

Author: lovesegfault

src/libstore-tests/aws-auth.cc

@@ -3,6 +3,7 @@
 
 #if NIX_WITH_AWS_CRT_SUPPORT
 
+#  include "nix/util/error.hh"
 #  include <gtest/gtest.h>
 #  include <gmock/gmock.h>
 
@@ -23,45 +24,52 @@ class AwsCredentialProviderTest : public ::testing::Test
 
 TEST_F(AwsCredentialProviderTest, createDefault)
 {
-    auto provider = AwsCredentialProvider::createDefault();
-    // Provider may be null in sandboxed environments, which is acceptable
-    if (!provider) {
-        GTEST_SKIP() << "AWS CRT not available in this environment";
+    try {
+        auto provider = AwsCredentialProvider::createDefault();
+        EXPECT_NE(provider, nullptr);
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
     }
-    EXPECT_NE(provider, nullptr);
 }
 
 TEST_F(AwsCredentialProviderTest, createProfile_Empty)
 {
-    auto provider = AwsCredentialProvider::createProfile("");
-    // Provider may be null in sandboxed environments, which is acceptable
-    if (!provider) {
-        GTEST_SKIP() << "AWS CRT not available in this environment";
+    try {
+        auto provider = AwsCredentialProvider::createProfile("");
+        EXPECT_NE(provider, nullptr);
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
     }
-    EXPECT_NE(provider, nullptr);
 }
 
 TEST_F(AwsCredentialProviderTest, createProfile_Named)
 {
-    auto provider = AwsCredentialProvider::createProfile("test-profile");
-    // Profile creation may fail if profile doesn't exist, which is expected
-    // Just verify the call doesn't crash
-    EXPECT_TRUE(true);
+    try {
+        auto provider = AwsCredentialProvider::createProfile("test-profile");
+        // Profile creation may succeed if profile exists
+        EXPECT_NE(provider, nullptr);
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
+    } catch (const Error & e) {
+        // Profile creation may fail if profile doesn't exist, which is expected
+        EXPECT_TRUE(true);
+    }
 }
 
 TEST_F(AwsCredentialProviderTest, getCredentials_NoCredentials)
 {
     // With no environment variables or profile, should return nullopt
-    auto provider = AwsCredentialProvider::createDefault();
-    if (!provider) {
-        GTEST_SKIP() << "AWS CRT not available in this environment";
+    try {
+        auto provider = AwsCredentialProvider::createDefault();
+        ASSERT_NE(provider, nullptr);
+
+        auto creds = provider->getCredentials();
+        // This may or may not be null depending on environment,
+        // so just verify the call doesn't crash
+        EXPECT_TRUE(true); // Basic sanity check
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
     }
-    ASSERT_NE(provider, nullptr);
-
-    auto creds = provider->getCredentials();
-    // This may or may not be null depending on environment,
-    // so just verify the call doesn't crash
-    EXPECT_TRUE(true); // Basic sanity check
 }
 
 TEST_F(AwsCredentialProviderTest, getCredentials_FromEnvironment)
@@ -71,22 +79,19 @@ TEST_F(AwsCredentialProviderTest, getCredentials_FromEnvironment)
     setenv("AWS_SECRET_ACCESS_KEY", "test-secret-key", 1);
     setenv("AWS_SESSION_TOKEN", "test-session-token", 1);
 
-    auto provider = AwsCredentialProvider::createDefault();
-    if (!provider) {
-        // Clean up first
-        unsetenv("AWS_ACCESS_KEY_ID");
-        unsetenv("AWS_SECRET_ACCESS_KEY");
-        unsetenv("AWS_SESSION_TOKEN");
-        GTEST_SKIP() << "AWS CRT not available in this environment";
-    }
-    ASSERT_NE(provider, nullptr);
-
-    auto creds = provider->getCredentials();
-    if (creds.has_value()) {
-        EXPECT_EQ(creds->accessKeyId, "test-access-key");
-        EXPECT_EQ(creds->secretAccessKey, "test-secret-key");
-        EXPECT_TRUE(creds->sessionToken.has_value());
-        EXPECT_EQ(*creds->sessionToken, "test-session-token");
+    try {
+        auto provider = AwsCredentialProvider::createDefault();
+        ASSERT_NE(provider, nullptr);
+
+        auto creds = provider->getCredentials();
+        if (creds.has_value()) {
+            EXPECT_EQ(creds->accessKeyId, "test-access-key");
+            EXPECT_EQ(creds->secretAccessKey, "test-secret-key");
+            EXPECT_TRUE(creds->sessionToken.has_value());
+            EXPECT_EQ(*creds->sessionToken, "test-session-token");
+        }
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
     }
 
     // Clean up
@@ -101,20 +106,18 @@ TEST_F(AwsCredentialProviderTest, getCredentials_WithoutSessionToken)
     setenv("AWS_ACCESS_KEY_ID", "test-access-key-2", 1);
     setenv("AWS_SECRET_ACCESS_KEY", "test-secret-key-2", 1);
 
-    auto provider = AwsCredentialProvider::createDefault();
-    if (!provider) {
-        // Clean up first
-        unsetenv("AWS_ACCESS_KEY_ID");
-        unsetenv("AWS_SECRET_ACCESS_KEY");
-        GTEST_SKIP() << "AWS CRT not available in this environment";
-    }
-    ASSERT_NE(provider, nullptr);
-
-    auto creds = provider->getCredentials();
-    if (creds.has_value()) {
-        EXPECT_EQ(creds->accessKeyId, "test-access-key-2");
-        EXPECT_EQ(creds->secretAccessKey, "test-secret-key-2");
-        EXPECT_FALSE(creds->sessionToken.has_value());
+    try {
+        auto provider = AwsCredentialProvider::createDefault();
+        ASSERT_NE(provider, nullptr);
+
+        auto creds = provider->getCredentials();
+        if (creds.has_value()) {
+            EXPECT_EQ(creds->accessKeyId, "test-access-key-2");
+            EXPECT_EQ(creds->secretAccessKey, "test-secret-key-2");
+            EXPECT_FALSE(creds->sessionToken.has_value());
+        }
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
     }
 
     // Clean up
@@ -125,15 +128,16 @@ TEST_F(AwsCredentialProviderTest, getCredentials_WithoutSessionToken)
 TEST_F(AwsCredentialProviderTest, multipleProviders_Independent)
 {
     // Test that multiple providers can be created independently
-    auto provider1 = AwsCredentialProvider::createDefault();
-    auto provider2 = AwsCredentialProvider::createDefault(); // Use default for both
-
-    if (!provider1 || !provider2) {
-        GTEST_SKIP() << "AWS CRT not available in this environment";
+    try {
+        auto provider1 = AwsCredentialProvider::createDefault();
+        auto provider2 = AwsCredentialProvider::createDefault(); // Use default for both
+
+        EXPECT_NE(provider1, nullptr);
+        EXPECT_NE(provider2, nullptr);
+        EXPECT_NE(provider1.get(), provider2.get());
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
     }
-    EXPECT_NE(provider1, nullptr);
-    EXPECT_NE(provider2, nullptr);
-    EXPECT_NE(provider1.get(), provider2.get());
 }
 
 } // namespace nix

src/libstore-tests/s3-edge-cases.cc

@@ -4,6 +4,7 @@
 
 #if NIX_WITH_AWS_CRT_SUPPORT
 
+#  include "nix/util/error.hh"
 #  include <gtest/gtest.h>
 #  include <gmock/gmock.h>
 
@@ -104,47 +105,55 @@ TEST_F(S3EdgeCasesTest, credentialProvider_Null)
 {
     // Test behavior when credential provider creation fails
     // Profile creation may fail for non-existent profiles, which is expected behavior
-    EXPECT_NO_THROW({
+    try {
         auto provider = AwsCredentialProvider::createProfile("non-existent-profile");
-        // Provider creation might return nullptr for invalid profiles
-        // This is acceptable behavior
-    });
+        // Provider creation might succeed or fail depending on profile existence
+        EXPECT_TRUE(true);
+    } catch (const AwsCrtInitializationError & e) {
+        // AWS CRT not available - this is acceptable
+        EXPECT_TRUE(true);
+    } catch (const Error & e) {
+        // Provider creation failed for invalid profile - this is expected
+        EXPECT_TRUE(true);
+    }
 }
 
 TEST_F(S3EdgeCasesTest, credentialProvider_EmptyProfile)
 {
     // Test that empty profile falls back to default
-    auto provider1 = AwsCredentialProvider::createProfile("");
-    auto provider2 = AwsCredentialProvider::createDefault();
+    try {
+        auto provider1 = AwsCredentialProvider::createProfile("");
+        auto provider2 = AwsCredentialProvider::createDefault();
 
-    if (!provider1 || !provider2) {
-        GTEST_SKIP() << "AWS CRT not available in this environment";
-    }
-    EXPECT_NE(provider1, nullptr);
-    EXPECT_NE(provider2, nullptr);
+        EXPECT_NE(provider1, nullptr);
+        EXPECT_NE(provider2, nullptr);
 
-    // Both should be created successfully
-    EXPECT_TRUE(true);
+        // Both should be created successfully
+        EXPECT_TRUE(true);
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
+    }
 }
 
 TEST_F(S3EdgeCasesTest, concurrentCredentialRequests)
 {
     // Test that multiple concurrent credential requests work
-    auto provider = AwsCredentialProvider::createDefault();
-    if (!provider) {
-        GTEST_SKIP() << "AWS CRT not available in this environment";
-    }
-    ASSERT_NE(provider, nullptr);
+    try {
+        auto provider = AwsCredentialProvider::createDefault();
+        ASSERT_NE(provider, nullptr);
 
-    // Simulate concurrent access (basic test)
-    std::vector<std::optional<AwsCredentials>> results(3);
+        // Simulate concurrent access (basic test)
+        std::vector<std::optional<AwsCredentials>> results(3);
 
-    for (int i = 0; i < 3; ++i) {
-        results[i] = provider->getCredentials();
-    }
+        for (int i = 0; i < 3; ++i) {
+            results[i] = provider->getCredentials();
+        }
 
-    // All calls should complete without crashing
-    EXPECT_EQ(results.size(), 3);
+        // All calls should complete without crashing
+        EXPECT_EQ(results.size(), 3);
+    } catch (const AwsCrtInitializationError & e) {
+        GTEST_SKIP() << "AWS CRT not available in this environment: " << e.what();
+    }
 }
 
 // Replaced with parameterized test S3SpecialCharacterTest above
@@ -178,42 +187,47 @@ TEST_F(S3EdgeCasesTest, multipleParameters)
 TEST_F(S3EdgeCasesTest, credentialTypes_AllScenarios)
 {
     // Test different credential scenarios
-    // Provider creation may return nullptr in sandboxed environments, which is acceptable
+    // Provider creation may throw in sandboxed environments, which is acceptable
 
     // 1. Environment variables with session token
     setenv("AWS_ACCESS_KEY_ID", "ASIATEST", 1);
     setenv("AWS_SECRET_ACCESS_KEY", "secret", 1);
     setenv("AWS_SESSION_TOKEN", "session", 1);
 
-    EXPECT_NO_THROW({
+    try {
         auto provider1 = AwsCredentialProvider::createDefault();
-        // Provider creation might fail in sandboxed build environments
-        if (provider1) {
-            auto creds = provider1->getCredentials();
-            // Just verify the call doesn't crash
-        }
-    });
+        auto creds = provider1->getCredentials();
+        // Just verify the call doesn't crash
+    } catch (const AwsCrtInitializationError &) {
+        // AWS CRT not available - acceptable
+    } catch (const Error &) {
+        // Other errors - acceptable in test environment
+    }
 
     // 2. Environment variables without session token
     unsetenv("AWS_SESSION_TOKEN");
 
-    EXPECT_NO_THROW({
+    try {
         auto provider2 = AwsCredentialProvider::createDefault();
-        if (provider2) {
-            auto creds = provider2->getCredentials();
-        }
-    });
+        auto creds = provider2->getCredentials();
+    } catch (const AwsCrtInitializationError &) {
+        // AWS CRT not available - acceptable
+    } catch (const Error &) {
+        // Other errors - acceptable in test environment
+    }
 
     // 3. Clear environment (should fall back to other providers)
     unsetenv("AWS_ACCESS_KEY_ID");
     unsetenv("AWS_SECRET_ACCESS_KEY");
 
-    EXPECT_NO_THROW({
+    try {
         auto provider3 = AwsCredentialProvider::createDefault();
-        if (provider3) {
-            auto creds = provider3->getCredentials();
-        }
-    });
+        auto creds = provider3->getCredentials();
+    } catch (const AwsCrtInitializationError &) {
+        // AWS CRT not available - acceptable
+    } catch (const Error &) {
+        // Other errors - acceptable in test environment
+    }
 
     // All calls should complete without crashing
     EXPECT_TRUE(true);
@@ -245,14 +259,16 @@ TEST_F(S3EdgeCasesTest, memory_LargeCredentials)
     setenv("AWS_SECRET_ACCESS_KEY", largeSecretKey.c_str(), 1);
     setenv("AWS_SESSION_TOKEN", largeSessionToken.c_str(), 1);
 
-    EXPECT_NO_THROW({
+    try {
         auto provider = AwsCredentialProvider::createDefault();
-        if (provider) {
-            // Should handle large credentials without memory issues
-            auto creds = provider->getCredentials();
-            // Just verify the call completes
-        }
-    });
+        // Should handle large credentials without memory issues
+        auto creds = provider->getCredentials();
+        // Just verify the call completes
+    } catch (const AwsCrtInitializationError &) {
+        // AWS CRT not available - acceptable
+    } catch (const Error &) {
+        // Other errors - acceptable in test environment
+    }
 
     // Clean up
     unsetenv("AWS_ACCESS_KEY_ID");
@@ -263,7 +279,7 @@ TEST_F(S3EdgeCasesTest, memory_LargeCredentials)
 TEST_F(S3EdgeCasesTest, threadSafety_MultipleProviders)
 {
     // Basic thread safety test
-    EXPECT_NO_THROW({
+    try {
         std::vector<std::unique_ptr<AwsCredentialProvider>> providers;
 
         // Create multiple providers
@@ -273,12 +289,14 @@ TEST_F(S3EdgeCasesTest, threadSafety_MultipleProviders)
 
         // Test concurrent credential resolution
         for (const auto & provider : providers) {
-            if (provider) {
-                auto creds = provider->getCredentials();
-                // Just verify calls complete without crashing
-            }
+            auto creds = provider->getCredentials();
+            // Just verify calls complete without crashing
         }
-    });
+    } catch (const AwsCrtInitializationError &) {
+        // AWS CRT not available - acceptable
+    } catch (const Error &) {
+        // Other errors - acceptable in test environment
+    }
 
     EXPECT_TRUE(true);
 }