kevinrr888 commented on code in PR #5715:
URL: https://github.com/apache/accumulo/pull/5715#discussion_r2208576577
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/TableOperationsImpl.java:
##########
@@ -1123,7 +1123,7 @@ private Map<String,String>
modifyPropertiesUnwrapped(String tableName,
} catch (AccumuloException ae) {
Throwable cause = ae.getCause();
if (cause instanceof TableNotFoundException) {
- throw new TableNotFoundException(null, tableName, null, ae);
+ throw (TableNotFoundException) cause;
}
Review Comment:
> Each exception has stack trace associated with it. Omitting exceptions and
throwing a cause omits these stack traces from anyone who may be looking at
them to debug a problem. This can make tracking down causes of problems harder,
like you may not even know that a thread took a certain path in the code.
Good point... We do lose the exact tracing in this current impl
> What was the problem you were seeing?
I think the problem I was seeing was related to this:
```
} catch (AccumuloException ae) {
Throwable cause = ae.getCause();
if (cause instanceof TableNotFoundException) {
throw new TableNotFoundException(null, tableName, null, ae);
}
throw ae;
}
```
I was seeing `AccumuloException`s unexpectedly and wasn't sure why, now
looking back I realize its because we have the cause of the
`TableNotFoundException` as the `AccumuloException` which isn't quite right.
The cause of the `TableNotFoundException` is `cause.getCause()`. The
`TableNotFoundException` is just wrapped in an `AccumuloException`
I think probably either:
`throw new TableNotFoundException(null, tableName, null, cause.getCause());`
or
`throw (TableNotFoundException) cause`
Would work here, but I lean towards the latter since the first case is
essentially the second case in a more roundabout way.
As for the lost tracing, `addSuppressed(ae)` seems good to me and I can make
these changes. It unfortunately doesn't seem like there is a nicer way to
preserve the info in `ae` while still throwing the proper exception.
--
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: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
