Skip to content

Commit c45e208

Browse files
committed
Cleanup exception handling in IOUtils (#29069)
When we copied IOUtils into the Elasticsearch codebase from Lucene, we brought with it its handling of throwables which are out of whack with how we handle throwables in our codebase. This commit modifies our copy of IOUtils to be consistent with how we handle throwables today: do not catch them. We take advantage of this cleanup to simplify IOUtils.
1 parent 56ee723 commit c45e208

File tree

1 file changed

+15
-62
lines changed
  • libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io

1 file changed

+15
-62
lines changed

libs/elasticsearch-core/src/main/java/org/elasticsearch/core/internal/io/IOUtils.java

Lines changed: 15 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -58,23 +58,29 @@ public static void close(final Closeable... objects) throws IOException {
5858
* @see #close(Closeable...)
5959
*/
6060
public static void close(final Iterable<? extends Closeable> objects) throws IOException {
61-
Throwable th = null;
61+
Exception ex = null;
6262

6363
for (final Closeable object : objects) {
6464
try {
6565
if (object != null) {
6666
object.close();
6767
}
68-
} catch (final Throwable t) {
69-
addSuppressed(th, t);
70-
if (th == null) {
71-
th = t;
68+
} catch (final IOException | RuntimeException e) {
69+
if (ex == null) {
70+
ex = e;
71+
} else {
72+
ex.addSuppressed(e);
7273
}
7374
}
7475
}
7576

76-
if (th != null) {
77-
throw rethrowAlways(th);
77+
if (ex != null) {
78+
if (ex instanceof IOException) {
79+
throw (IOException) ex;
80+
} else {
81+
// since we only assigned an IOException or a RuntimeException to ex above, in this case ex must be a RuntimeException
82+
throw (RuntimeException) ex;
83+
}
7884
}
7985
}
8086

@@ -101,65 +107,12 @@ public static void closeWhileHandlingException(final Iterable<? extends Closeabl
101107
if (object != null) {
102108
object.close();
103109
}
104-
} catch (final Throwable t) {
110+
} catch (final IOException | RuntimeException e) {
105111

106112
}
107113
}
108114
}
109115

110-
/**
111-
* Adds a {@link Throwable} to the list of suppressed {@link Exception}s of the first {@link Throwable}.
112-
*
113-
* @param exception the exception to add a suppression to, if non-null
114-
* @param suppressed the exception to suppress
115-
*/
116-
private static void addSuppressed(final Throwable exception, final Throwable suppressed) {
117-
if (exception != null && suppressed != null) {
118-
exception.addSuppressed(suppressed);
119-
}
120-
}
121-
122-
/**
123-
* This utility method takes a previously caught (non-null) {@link Throwable} and rethrows either the original argument if it was a
124-
* subclass of the {@link IOException} or an {@link RuntimeException} with the cause set to the argument.
125-
* <p>
126-
* This method <strong>never returns any value</strong>, even though it declares a return value of type {@link Error}. The return
127-
* value declaration is very useful to let the compiler know that the code path following the invocation of this method is unreachable.
128-
* So in most cases the invocation of this method will be guarded by an {@code if} and used together with a {@code throw} statement, as
129-
* in:
130-
* <p>
131-
* <pre>{@code
132-
* if (t != null) throw IOUtils.rethrowAlways(t)
133-
* }
134-
* </pre>
135-
*
136-
* @param th the throwable to rethrow; <strong>must not be null</strong>
137-
* @return this method always results in an exception, it never returns any value; see method documentation for details and usage
138-
* example
139-
* @throws IOException if the argument was an instance of {@link IOException}
140-
* @throws RuntimeException with the {@link RuntimeException#getCause()} set to the argument, if it was not an instance of
141-
* {@link IOException}
142-
*/
143-
private static Error rethrowAlways(final Throwable th) throws IOException, RuntimeException {
144-
if (th == null) {
145-
throw new AssertionError("rethrow argument must not be null.");
146-
}
147-
148-
if (th instanceof IOException) {
149-
throw (IOException) th;
150-
}
151-
152-
if (th instanceof RuntimeException) {
153-
throw (RuntimeException) th;
154-
}
155-
156-
if (th instanceof Error) {
157-
throw (Error) th;
158-
}
159-
160-
throw new RuntimeException(th);
161-
}
162-
163116
/**
164117
* Deletes all given files, suppressing all thrown {@link IOException}s. Some of the files may be null, if so they are ignored.
165118
*
@@ -180,7 +133,7 @@ public static void deleteFilesIgnoringExceptions(final Collection<? extends Path
180133
// noinspection EmptyCatchBlock
181134
try {
182135
Files.delete(name);
183-
} catch (final Throwable ignored) {
136+
} catch (final IOException ignored) {
184137

185138
}
186139
}

0 commit comments

Comments
 (0)