[refactoring] improve handling exceptions (#10068)

asolntsev · web-flow · commit 04a681b47bde · 2021-12-08T10:19:07.000+01:00
* [refactoring] improve handling exceptions

* throw "new UncheckedIOException(ioe)"" instead of just "new RuntimeException(ioe)"
* avoid catching too broad Exception which causes unneeded re-wrapping of RuntimeExceptions

* revert changes in com.thoughtworks package

.. as it's deprecated and will be removed anyway.
diff --git a/java/src/org/openqa/selenium/net/LinuxEphemeralPortRangeDetector.java b/java/src/org/openqa/selenium/net/LinuxEphemeralPortRangeDetector.java
@@ -22,6 +22,7 @@
 import java.io.IOException;
 import java.io.Reader;
 import java.io.StringReader;
+import java.io.UncheckedIOException;
 import java.nio.charset.Charset;
 import java.nio.file.Files;
 
@@ -36,7 +37,7 @@ public static LinuxEphemeralPortRangeDetector getInstance() {
       try (Reader inputFil = Files.newBufferedReader(file.toPath(), Charset.defaultCharset())) {
         return new LinuxEphemeralPortRangeDetector(inputFil);
       } catch (IOException e) {
-        throw new RuntimeException(e);
+        throw new UncheckedIOException(e);
       }
     }
     return new LinuxEphemeralPortRangeDetector(new StringReader("49152 65535"));
diff --git a/java/src/org/openqa/selenium/net/PortProber.java b/java/src/org/openqa/selenium/net/PortProber.java
@@ -20,6 +20,7 @@
 import org.openqa.selenium.Platform;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.net.ConnectException;
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
@@ -113,7 +114,7 @@ public static void waitForPortUp(int port, int timeout, TimeUnit unit) {
       } catch (ConnectException | SocketTimeoutException e) {
         // Ignore this
       } catch (IOException e) {
-        throw new RuntimeException(e);
+        throw new UncheckedIOException(e);
       }
     }
   }
diff --git a/java/src/org/openqa/selenium/net/UrlChecker.java b/java/src/org/openqa/selenium/net/UrlChecker.java
@@ -17,6 +17,9 @@
 
 package org.openqa.selenium.net;
 
+import static java.util.concurrent.TimeUnit.MILLISECONDS;
+import static java.util.concurrent.TimeUnit.NANOSECONDS;
+
 import java.io.IOException;
 import java.net.HttpURLConnection;
 import java.net.URL;
@@ -29,9 +32,6 @@
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Logger;
 
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
-import static java.util.concurrent.TimeUnit.NANOSECONDS;
-
 /**
  * Polls a URL until a HTTP 200 response is received.
  */
@@ -130,9 +130,7 @@ public void waitUntilUnavailable(long timeout, TimeUnit unit, final URL url)
       throw new TimeoutException(String.format(
           "Timed out waiting for %s to become unavailable after %d ms",
           url, MILLISECONDS.convert(System.nanoTime() - start, NANOSECONDS)), e);
-    } catch (RuntimeException e) {
-      throw e;
-    } catch (Exception e) {
+    } catch (InterruptedException | ExecutionException e) {
       throw new RuntimeException(e);
     }
   }
diff --git a/java/src/org/openqa/selenium/remote/server/log/SessionLogsToFileRepository.java b/java/src/org/openqa/selenium/remote/server/log/SessionLogsToFileRepository.java
@@ -25,6 +25,7 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
+import java.io.UncheckedIOException;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
@@ -122,7 +123,7 @@ public void removeLogFile(SessionId sessionId) {
     try {
       logFile.removeLogFile();
     } catch (IOException e) {
-      throw new RuntimeException(e);
+      throw new UncheckedIOException(e);
     }
   }
 
diff --git a/java/src/org/openqa/selenium/server/htmlrunner/HTMLSuiteResult.java b/java/src/org/openqa/selenium/server/htmlrunner/HTMLSuiteResult.java
@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import java.io.StringReader;
+import java.io.UncheckedIOException;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -48,7 +49,7 @@ public HTMLSuiteResult(String originalSuite) {
       parser.parse(s, p, true);
     } catch (IOException e) {
       // DGF aw, this won't really happen! (will it?)
-      throw new RuntimeException(e);
+      throw new UncheckedIOException(e);
     }
     hrefs = p.hrefList;
     StringBuilder sb = new StringBuilder();
diff --git a/java/test/org/openqa/selenium/environment/GlobalTestEnvironment.java b/java/test/org/openqa/selenium/environment/GlobalTestEnvironment.java
@@ -37,12 +37,8 @@ public static TestEnvironment get() {
   public static synchronized TestEnvironment getOrCreate(
       Supplier<TestEnvironment> startThisIfNothingIsAlreadyRunning) {
     if (environment == null) {
-      try {
-        environment = startThisIfNothingIsAlreadyRunning.get();
-        environment.assertIsValid();
-      } catch (Exception e) {
-        throw new RuntimeException(e);
-      }
+      environment = startThisIfNothingIsAlreadyRunning.get();
+      environment.assertIsValid();
     }
     return environment;
   }
diff --git a/java/test/org/openqa/selenium/net/StubNetworkInterfaceProvider.java b/java/test/org/openqa/selenium/net/StubNetworkInterfaceProvider.java
@@ -16,6 +16,7 @@
 // under the License.
 package org.openqa.selenium.net;
 
+import java.io.UncheckedIOException;
 import java.net.InetAddress;
 import java.net.UnknownHostException;
 import java.util.Arrays;
@@ -47,7 +48,7 @@ private static InetAddress inetAddress(String host, String addressString) {
       InetAddress tmp = InetAddress.getByName(addressString);
       return InetAddress.getByAddress(host, tmp.getAddress());
     } catch (UnknownHostException e) {
-      throw new RuntimeException(e);
+      throw new UncheckedIOException(e);
     }
   }
 
@@ -58,7 +59,7 @@ private static InetAddress inetAddress(String addressString) {
       InetAddress tmp = InetAddress.getByName(addressString);
       return InetAddress.getByAddress(addressString, tmp.getAddress());
     } catch (UnknownHostException e) {
-      throw new RuntimeException(e);
+      throw new UncheckedIOException(e);
     }
   }
 
diff --git a/java/test/org/openqa/selenium/remote/tracing/opentelemetry/TracerTest.java b/java/test/org/openqa/selenium/remote/tracing/opentelemetry/TracerTest.java
@@ -570,7 +570,7 @@ public void cleverShenanigansRepresentingWhatWeSeeInTheRouter() {
           }
         });
         toReturn.get();
-      } catch (Exception e) {
+      } catch (InterruptedException | ExecutionException e) {
         throw new RuntimeException(e);
       }
       return new HttpResponse();
diff --git a/java/test/org/openqa/selenium/testing/drivers/GridSupplier.java b/java/test/org/openqa/selenium/testing/drivers/GridSupplier.java
@@ -17,6 +17,8 @@
 
 package org.openqa.selenium.testing.drivers;
 
+import static org.openqa.selenium.remote.http.Contents.string;
+
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.firefox.FirefoxOptions;
@@ -31,13 +33,12 @@
 import org.openqa.selenium.support.ui.Wait;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.net.URL;
 import java.time.Duration;
 import java.util.Map;
 import java.util.function.Supplier;
 
-import static org.openqa.selenium.remote.http.Contents.string;
-
 public class GridSupplier implements Supplier<WebDriver> {
 
   private static OutOfProcessSeleniumServer hub;
@@ -80,7 +81,7 @@ private synchronized void startServers() {
           "-role", "node",
           "-hub", String.valueOf(hubUrl));
     } catch (IOException e) {
-      throw new RuntimeException(e);
+      throw new UncheckedIOException(e);
     }
 
     // Keep polling the status page of the hub until it claims to be ready

Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,7 @@`
`20`	`20`	`import org.openqa.selenium.Platform;`
`21`	`21`
`22`	`22`	`import java.io.IOException;`
	`23`	`+import java.io.UncheckedIOException;`
`23`	`24`	`import java.net.ConnectException;`
`24`	`25`	`import java.net.InetSocketAddress;`
`25`	`26`	`import java.net.ServerSocket;`
`@@ -113,7 +114,7 @@ public static void waitForPortUp(int port, int timeout, TimeUnit unit) {`
`113`	`114`	`} catch (ConnectException \| SocketTimeoutException e) {`
`114`	`115`	`// Ignore this`
`115`	`116`	`} catch (IOException e) {`
`116`		`- throw new RuntimeException(e);`
	`117`	`+ throw new UncheckedIOException(e);`
`117`	`118`	`}`
`118`	`119`	`}`
`119`	`120`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@`
`16`	`16`	`// under the License.`
`17`	`17`	`package org.openqa.selenium.net;`
`18`	`18`
	`19`	`+import java.io.UncheckedIOException;`
`19`	`20`	`import java.net.InetAddress;`
`20`	`21`	`import java.net.UnknownHostException;`
`21`	`22`	`import java.util.Arrays;`
`@@ -47,7 +48,7 @@ private static InetAddress inetAddress(String host, String addressString) {`
`47`	`48`	`InetAddress tmp = InetAddress.getByName(addressString);`
`48`	`49`	`return InetAddress.getByAddress(host, tmp.getAddress());`
`49`	`50`	`} catch (UnknownHostException e) {`
`50`		`- throw new RuntimeException(e);`
	`51`	`+ throw new UncheckedIOException(e);`
`51`	`52`	`}`
`52`	`53`	`}`
`53`	`54`
`@@ -58,7 +59,7 @@ private static InetAddress inetAddress(String addressString) {`
`58`	`59`	`InetAddress tmp = InetAddress.getByName(addressString);`
`59`	`60`	`return InetAddress.getByAddress(addressString, tmp.getAddress());`
`60`	`61`	`} catch (UnknownHostException e) {`
`61`		`- throw new RuntimeException(e);`
	`62`	`+ throw new UncheckedIOException(e);`
`62`	`63`	`}`
`63`	`64`	`}`
`64`	`65`
Original file line number	Diff line number	Diff line change
`@@ -570,7 +570,7 @@ public void cleverShenanigansRepresentingWhatWeSeeInTheRouter() {`
`570`	`570`	`}`
`571`	`571`	`});`
`572`	`572`	`toReturn.get();`
`573`		`- } catch (Exception e) {`
	`573`	`+ } catch (InterruptedException \| ExecutionException e) {`
`574`	`574`	`throw new RuntimeException(e);`
`575`	`575`	`}`
`576`	`576`	`return new HttpResponse();`