From 680c7db875977e0ffa53e60eb762ab38647f6d2c Mon Sep 17 00:00:00 2001 From: Mike Zhang Date: Sat, 1 Oct 2022 17:31:23 +1000 Subject: [PATCH 1/2] JAVA-3033 - Consider handling IOException in a manner consistent with other exceptions in CloudConfigFactory.fetchProxyMetadata() --- .../core/config/cloud/CloudConfigFactory.java | 21 ++++++++++++------- .../config/cloud/CloudConfigFactoryTest.java | 17 ++++++++++++--- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java b/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java index f7386dcc390..5aea1392d35 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java @@ -24,12 +24,7 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import edu.umd.cs.findbugs.annotations.NonNull; -import java.io.BufferedReader; -import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; +import java.io.*; import java.net.ConnectException; import java.net.InetSocketAddress; import java.net.MalformedURLException; @@ -228,8 +223,14 @@ protected BufferedReader fetchProxyMetadata( connection.setSSLSocketFactory(sslContext.getSocketFactory()); connection.setRequestMethod("GET"); connection.setRequestProperty("host", "localhost"); - return new BufferedReader( - new InputStreamReader(connection.getInputStream(), StandardCharsets.UTF_8)); + try { + return new BufferedReader( + new InputStreamReader(connection.getInputStream(), StandardCharsets.UTF_8)); + } catch (IOException ioe) { + throw new IllegalStateException( + "Unable to read data from cloud metadata service. Please make sure your cluster is not parked or terminated", + ioe); + } } catch (ConnectException e) { throw new IllegalStateException( "Unable to connect to cloud metadata service. Please make sure your cluster is not parked or terminated", @@ -238,6 +239,10 @@ protected BufferedReader fetchProxyMetadata( throw new IllegalStateException( "Unable to resolve host for cloud metadata service. Please make sure your cluster is not terminated", e); + } catch (FileNotFoundException e) { + throw new IllegalStateException( + "Metadata service request path not found. Please make sure the metadata service url is correct", + e); } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java index 6f54413b601..7f1da159941 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java @@ -35,8 +35,7 @@ import com.github.tomakehurst.wiremock.jetty9.JettyHttpServer; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.google.common.base.Joiner; -import java.io.FileNotFoundException; -import java.io.IOException; +import java.io.*; import java.net.InetSocketAddress; import java.net.URISyntaxException; import java.net.URL; @@ -134,7 +133,19 @@ public void should_throw_when_metadata_not_found() throws Exception { URL configFile = new URL("http", "localhost", wireMockRule.port(), BUNDLE_PATH); CloudConfigFactory cloudConfigFactory = new CloudConfigFactory(); Throwable t = catchThrowable(() -> cloudConfigFactory.createCloudConfig(configFile)); - assertThat(t).isInstanceOf(FileNotFoundException.class).hasMessageContaining("metadata"); + assertThat(t).isInstanceOf(IllegalStateException.class).hasMessageContaining("metadata"); + } + + @Test + public void should_throw_when_stream_io_error_fetch_metadata() throws Exception { + // given + mockHttpSecureBundle(secureBundle()); + stubFor(any(urlPathEqualTo("/metadata")).willReturn(aResponse().withStatus(401))); + // when + URL configFile = new URL("http", "localhost", wireMockRule.port(), BUNDLE_PATH); + CloudConfigFactory cloudConfigFactory = new CloudConfigFactory(); + Throwable t = catchThrowable(() -> cloudConfigFactory.createCloudConfig(configFile)); + assertThat(t).isInstanceOf(IllegalStateException.class).hasMessageContaining("metadata"); } @Test From f292c60f1394cdb61e1f526e4f4bce71401761ad Mon Sep 17 00:00:00 2001 From: Henry Hughes Date: Thu, 27 Jul 2023 15:21:09 -0700 Subject: [PATCH 2/2] Small fixes to initial commit - Move FNF handling to before IOE (FNF is subclass of IOE) - Update tests to assert exact message strings - add test for bad gateway (502) from /metadata - remove wildcard exceptions --- .../core/config/cloud/CloudConfigFactory.java | 16 +++++++--- .../config/cloud/CloudConfigFactoryTest.java | 32 ++++++++++++++++--- 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java b/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java index 5aea1392d35..077321ac9d5 100644 --- a/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java +++ b/core/src/main/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactory.java @@ -24,7 +24,13 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import edu.umd.cs.findbugs.annotations.NonNull; -import java.io.*; +import java.io.BufferedReader; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.FileNotFoundException; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; import java.net.ConnectException; import java.net.InetSocketAddress; import java.net.MalformedURLException; @@ -226,6 +232,10 @@ protected BufferedReader fetchProxyMetadata( try { return new BufferedReader( new InputStreamReader(connection.getInputStream(), StandardCharsets.UTF_8)); + } catch (FileNotFoundException e) { + throw new IllegalStateException( + "Metadata service request path not found. Please make sure the metadata service url is correct", + e); } catch (IOException ioe) { throw new IllegalStateException( "Unable to read data from cloud metadata service. Please make sure your cluster is not parked or terminated", @@ -239,10 +249,6 @@ protected BufferedReader fetchProxyMetadata( throw new IllegalStateException( "Unable to resolve host for cloud metadata service. Please make sure your cluster is not terminated", e); - } catch (FileNotFoundException e) { - throw new IllegalStateException( - "Metadata service request path not found. Please make sure the metadata service url is correct", - e); } } diff --git a/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java b/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java index 7f1da159941..fcdb69e9161 100644 --- a/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java +++ b/core/src/test/java/com/datastax/oss/driver/internal/core/config/cloud/CloudConfigFactoryTest.java @@ -35,7 +35,8 @@ import com.github.tomakehurst.wiremock.jetty9.JettyHttpServer; import com.github.tomakehurst.wiremock.junit.WireMockRule; import com.google.common.base.Joiner; -import java.io.*; +import java.io.FileNotFoundException; +import java.io.IOException; import java.net.InetSocketAddress; import java.net.URISyntaxException; import java.net.URL; @@ -121,7 +122,7 @@ public void should_throw_when_bundle_not_readable() throws Exception { Throwable t = catchThrowable(() -> cloudConfigFactory.createCloudConfig(configFile)); assertThat(t) .isInstanceOf(IllegalStateException.class) - .hasMessageContaining("Invalid bundle: missing file config.json"); + .hasMessage("Invalid bundle: missing file config.json"); } @Test @@ -133,11 +134,14 @@ public void should_throw_when_metadata_not_found() throws Exception { URL configFile = new URL("http", "localhost", wireMockRule.port(), BUNDLE_PATH); CloudConfigFactory cloudConfigFactory = new CloudConfigFactory(); Throwable t = catchThrowable(() -> cloudConfigFactory.createCloudConfig(configFile)); - assertThat(t).isInstanceOf(IllegalStateException.class).hasMessageContaining("metadata"); + assertThat(t) + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Metadata service request path not found. Please make sure the metadata service url is correct"); } @Test - public void should_throw_when_stream_io_error_fetch_metadata() throws Exception { + public void should_throw_when_metadata_unauthorized() throws Exception { // given mockHttpSecureBundle(secureBundle()); stubFor(any(urlPathEqualTo("/metadata")).willReturn(aResponse().withStatus(401))); @@ -145,7 +149,25 @@ public void should_throw_when_stream_io_error_fetch_metadata() throws Exception URL configFile = new URL("http", "localhost", wireMockRule.port(), BUNDLE_PATH); CloudConfigFactory cloudConfigFactory = new CloudConfigFactory(); Throwable t = catchThrowable(() -> cloudConfigFactory.createCloudConfig(configFile)); - assertThat(t).isInstanceOf(IllegalStateException.class).hasMessageContaining("metadata"); + assertThat(t) + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Unable to read data from cloud metadata service. Please make sure your cluster is not parked or terminated"); + } + + @Test + public void should_throw_when_metadata_bad_gateway() throws Exception { + // given + mockHttpSecureBundle(secureBundle()); + stubFor(any(urlPathEqualTo("/metadata")).willReturn(aResponse().withStatus(502))); + // when + URL configFile = new URL("http", "localhost", wireMockRule.port(), BUNDLE_PATH); + CloudConfigFactory cloudConfigFactory = new CloudConfigFactory(); + Throwable t = catchThrowable(() -> cloudConfigFactory.createCloudConfig(configFile)); + assertThat(t) + .isInstanceOf(IllegalStateException.class) + .hasMessage( + "Unable to read data from cloud metadata service. Please make sure your cluster is not parked or terminated"); } @Test