maedhroz commented on a change in pull request #1301:
URL: https://github.com/apache/cassandra/pull/1301#discussion_r796265733
##########
File path: src/java/org/apache/cassandra/db/streaming/CassandraStreamReader.java
##########
@@ -136,10 +143,10 @@ public SSTableMultiWriter read(DataInputPlus inputPlus)
throws IOException
logger.warn("[Stream {}] Error while reading partition {} from
stream on ks='{}' and table='{}'.",
session.planId(), partitionKey,
cfs.keyspace.getName(), cfs.getTableName(), e);
if (writer != null)
- {
- writer.abort(e);
- }
- throw Throwables.propagate(e);
+ e = writer.abort(e);
+ Throwables.throwIfUnchecked(e);
+ Throwables.throwIfInstanceOf(e, Exception.class);
+ throw new RuntimeException(e); // not possible; just here to
compile
Review comment:
At this point, what if we just do the following:
1.) Leave all the method signatures alone in the `IStreamReader` hierarchy.
(i.e. Leave the `throws IOException`.)
2.) Change the exception we throw for dropped tables from `IOException` to
something a bit more appropriate, like maybe `IllegalStateException`.
3.) Change the bits above to just:
```
Throwables.throwIfUnchecked(e);
if (e instanceof IOException) throw (IOException) e;
throw new RuntimeException(e);
```
My guess is we wanted to avoid wrapping `IOException` in a
`RuntimeException`, but as things stand, the `throws Exception` doesn't really
help explain the API (which is probably the only point of checked exceptions in
the first place).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
