Make AwsRegionProviders throw exceptions like AwsCredentialsProviders

Author: varunnvs92

regions/src/main/java/software/amazon/awssdk/regions/providers/AwsProfileRegionProvider.java

@@ -17,6 +17,7 @@
 
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.core.SdkSystemSetting;
+import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.profiles.ProfileFile;
 import software.amazon.awssdk.profiles.ProfileProperties;
 import software.amazon.awssdk.regions.Region;
@@ -35,7 +36,7 @@ public Region getRegion() {
                           .profile(profileName)
                           .map(p -> p.properties().get(ProfileProperties.REGION))
                           .map(Region::of)
-                          .orElse(null);
+                          .orElseThrow(() -> new SdkClientException("No region provided in profile: " + profileName));
     }
 }
 

regions/src/main/java/software/amazon/awssdk/regions/providers/AwsRegionProvider.java

@@ -15,7 +15,6 @@
 
 package software.amazon.awssdk.regions.providers;
 
-import software.amazon.awssdk.annotations.ReviewBeforeRelease;
 import software.amazon.awssdk.annotations.SdkProtectedApi;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.regions.Region;
@@ -28,10 +27,10 @@
 @FunctionalInterface
 public interface AwsRegionProvider {
     /**
-     * @return Region name to use or null if region information is not available.
+     * Returns the region name to use. If region information is not available, throws an {@link SdkClientException}.
+     *
+     * @return Region name to use.
      */
-    @ReviewBeforeRelease("Should this throw exceptions so that its contract matches that of the credential providers? This is " +
-                         "currently a protected API used in STS, so we should decide before GA.")
-    Region getRegion() throws SdkClientException;
+    Region getRegion();
 
 }

regions/src/main/java/software/amazon/awssdk/regions/providers/AwsRegionProviderChain.java

@@ -26,6 +26,8 @@
 /**
  * Composite {@link AwsRegionProvider} that sequentially delegates to a chain of providers looking
  * for region information.
+ *
+ * Throws an {@link SdkClientException} if region could not be find in any of the providers.
  */
 public class AwsRegionProviderChain implements AwsRegionProvider {
 
@@ -40,6 +42,8 @@ public AwsRegionProviderChain(AwsRegionProvider... providers) {
 
     @Override
     public Region getRegion() throws SdkClientException {
+        List<String> exceptionMessages = null;
+
         for (AwsRegionProvider provider : providers) {
             try {
                 final Region region = provider.getRegion();
@@ -49,9 +53,16 @@ public Region getRegion() throws SdkClientException {
             } catch (Exception e) {
                 // Ignore any exceptions and move onto the next provider
                 log.debug("Unable to load region from {}:{}", provider.toString(), e.getMessage());
+
+                String message = provider.toString() + ": " + e.getMessage();
+                if (exceptionMessages == null) {
+                    exceptionMessages = new ArrayList<>();
+                }
+                exceptionMessages.add(message);
             }
         }
 
-        return null;
+        throw new SdkClientException("Unable to load region from any of the providers in the chain " + this
+                                     + ": " + exceptionMessages);
     }
 }

regions/src/main/java/software/amazon/awssdk/regions/providers/InstanceProfileRegionProvider.java

@@ -15,14 +15,13 @@
 
 package software.amazon.awssdk.regions.providers;
 
-import org.slf4j.LoggerFactory;
 import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.regions.util.EC2MetadataUtils;
 
 /**
  * Attempts to load region information from the EC2 Metadata service. If the application is not
- * running on EC2 this provider will return null.
+ * running on EC2 this provider will thrown an exception.
  */
 public class InstanceProfileRegionProvider implements AwsRegionProvider {
 
@@ -41,16 +40,15 @@ public Region getRegion() throws SdkClientException {
             }
         }
 
-        return region == null ? null : Region.of(region);
+        if (region == null) {
+            throw new SdkClientException("Unable to retrieve region information from EC2 Metadata service. "
+                                         + "Please make sure the application is running on EC2.");
+        }
+
+        return Region.of(region);
     }
 
     private String tryDetectRegion() {
-        try {
-            return EC2MetadataUtils.getEC2InstanceRegion();
-        } catch (SdkClientException sce) {
-            LoggerFactory.getLogger(InstanceProfileRegionProvider.class)
-                      .debug("Ignoring failure to retrieve the region: {}", sce.getMessage());
-            return null;
-        }
+        return EC2MetadataUtils.getEC2InstanceRegion();
     }
 }

regions/src/main/java/software/amazon/awssdk/regions/providers/SystemSettingsRegionProvider.java

@@ -26,6 +26,15 @@
 public class SystemSettingsRegionProvider implements AwsRegionProvider {
     @Override
     public Region getRegion() throws SdkClientException {
-        return SdkSystemSetting.AWS_REGION.getStringValue().map(Region::of).orElse(null);
+        return SdkSystemSetting.AWS_REGION.getStringValue()
+                                          .map(Region::of)
+                                          .orElseThrow(this::exception);
+    }
+
+    private SdkClientException exception() {
+        return new SdkClientException(String.format("Unable to load region from system settings. Region must be specified either "
+                                                    + "via environment variable (%s) or system property (%s).",
+                                                    SdkSystemSetting.AWS_REGION.environmentVariable(),
+                                                    SdkSystemSetting.AWS_REGION.property()));
     }
 }

regions/src/test/java/software/amazon/awssdk/regions/providers/AwsProfileRegionProviderTest.java

@@ -16,11 +16,13 @@
 package software.amazon.awssdk.regions.providers;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 import java.net.URISyntaxException;
 import java.nio.file.Paths;
 import org.junit.Rule;
 import org.junit.Test;
+import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.testutils.EnvironmentVariableHelper;
 import software.amazon.awssdk.core.SdkSystemSetting;
@@ -31,10 +33,12 @@ public class AwsProfileRegionProviderTest {
     public EnvironmentVariableHelper settingsHelper = new EnvironmentVariableHelper();
 
     @Test
-    public void nonExistentDefaultConfigFile_ReturnsNull() {
+    public void nonExistentDefaultConfigFile_ThrowsException() {
         settingsHelper.set(SdkSystemSetting.AWS_CONFIG_FILE, "/var/tmp/this/is/invalid.txt");
         settingsHelper.set(SdkSystemSetting.AWS_SHARED_CREDENTIALS_FILE, "/var/tmp/this/is/also.invalid.txt");
-        assertThat(new AwsProfileRegionProvider().getRegion()).isNull();
+        assertThatThrownBy(() -> new AwsProfileRegionProvider().getRegion())
+            .isInstanceOf(SdkClientException.class)
+            .hasMessageContaining("No region provided in profile: default");
     }
 
     @Test

regions/src/test/java/software/amazon/awssdk/regions/providers/AwsRegionProviderChainTest.java

@@ -16,7 +16,6 @@
 package software.amazon.awssdk.regions.providers;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
@@ -77,12 +76,12 @@ public void providerThrowsError_DoesNotContinueChain() {
         assertEquals(expectedRegion, chain.getRegion());
     }
 
-    @Test
-    public void noProviderGivesRegion_ReturnsNull() {
+    @Test (expected = SdkClientException.class)
+    public void noProviderGivesRegion_ThrowsException() {
         AwsRegionProviderChain chain = new AwsRegionProviderChain(new NeverAwsRegionProvider(),
                                                                   new NeverAwsRegionProvider(),
                                                                   new NeverAwsRegionProvider());
-        assertNull(chain.getRegion());
+       chain.getRegion();
     }
 
     private static class NeverAwsRegionProvider implements AwsRegionProvider {

regions/src/test/java/software/amazon/awssdk/regions/providers/InstanceProfileRegionProviderTest.java

@@ -16,7 +16,6 @@
 package software.amazon.awssdk.regions.providers;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNull;
 
 import java.io.IOException;
 import org.junit.AfterClass;
@@ -26,6 +25,7 @@
 import org.junit.experimental.runners.Enclosed;
 import org.junit.runner.RunWith;
 import software.amazon.awssdk.core.SdkSystemSetting;
+import software.amazon.awssdk.core.exception.SdkClientException;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.regions.util.EC2MetadataUtilsServer;
 
@@ -73,9 +73,8 @@ public void metadataServiceRunning_ProvidesCorrectRegion() {
     }
 
     /**
-     * If the EC2 metadata service is not present then the provider should just return null instead
-     * of failing. This is to allow the provider to be used in a chain context where another
-     * provider further down the chain may be able to provide the region.
+     * If the EC2 metadata service is not present then the provider will throw an exception. If the provider is used
+     * in a {@link AwsRegionProviderChain}, the chain will catch the exception and go on to the next region provider.
      */
     public static class MetadataServiceNotRunning {
 
@@ -87,9 +86,9 @@ public void setup() {
             regionProvider = new InstanceProfileRegionProvider();
         }
 
-        @Test
-        public void metadataServiceNotRunning_ProvidesCorrectRegion() {
-            assertNull(regionProvider.getRegion());
+        @Test (expected = SdkClientException.class)
+        public void metadataServiceNotRunning_ThrowsException() {
+            regionProvider.getRegion();
         }
 
     }