uschindler commented on a change in pull request #1624:
URL: https://github.com/apache/lucene-solr/pull/1624#discussion_r446631519
##########
File path: solr/core/src/java/org/apache/solr/api/AnnotatedApi.java
##########
@@ -306,7 +313,7 @@ void invoke(SolrQueryRequest req, SolrQueryResponse rsp,
CommandOperation cmd) {
} catch (InvocationTargetException ite) {
log.error("Error executing command ", ite);
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
ite.getCause());
- } catch (Exception e) {
+ } catch (Throwable e) {
Review comment:
There are two things to do:
- have the try-catch only around the invoke and nothing else, so we don't
accidentally catch any unwanted exception in the surrounding code
- only checked exceptions should be wrapped, everything else rethrown. This
keeps stack traces clean.
The reason for the declaration of "Throwable" comes from the fact that the
method handle always throws the original Exception and it's now wrapped like
for reflection. As checked exceptions is a compiler thing, normal code using
method handles in bytecode or invokedynamic never needs to catch Throwable.
It's only the stupid Java compiler that enforces us to catch everything,
because it can't know if the handle might possibly throw a checked exception.
When writing scripting languages you just ignore the Throwable when you produce
bytecode. 🤓
Here we should catch all possible exceptions that are commonly thrown by
plugins: SolrException, Runtime exception, Error and rethrown those. The last
catch block would catch Throwable and log it separately as some fatal error.
The code above should check the possible exceptions a method may throw and
forbid e.g. a plugin to throw Interrupted exception. So maybe get all throws
clauses of method and have whitelist. In the catch clause then throw
AssertionError if the "unknown" exception happens.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
