weixin_39660408
2020-12-27 11:22 阅读 2

Always unwrap InvocationTargetException

InvocationTargetException adds nothing to the exception message stacktrace. Lots of issues are reported like InvocationTargetException when ... which is sad :-/

The only extra information it adds is it designates that the exception happened in the reflectively called method rather than in the reflective engine itself. In practice, the distinction is very subtle, and I doubt it is worth keeping InvocationTargetException.

For instance, the exception in https://github.com/typetools/checker-framework/issues/3700 starts with InvocationTargetException and it is useless.

I would suggest to always unwrap InvocationTargetException.

WDYT?

See https://github.com/typetools/checker-framework/blob/0d1de3b05a4ac9601f321b027416ea8641dbfc4c/framework/src/main/java/org/checkerframework/common/basetype/BaseTypeChecker.java#L303-L312

该提问来源于开源项目:typetools/checker-framework

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享

6条回答 默认 最新

  • weixin_39540934 weixin_39540934 2020-12-27 11:22

    Thanks for the suggestion.

    The only extra information it adds is it designates that the exception happened in the reflectively called method rather than in the reflective engine itself. In practice, the distinction is very subtle, and I doubt it is worth keeping InvocationTargetException.

    I agree that distinction is subtle, but stack traces are only intended for programmers working on the Checker Framework, when the Checker Framework crashes. Therefore, I don't think that exposing the subtlety is necessarily a problem, if that information might be useful.

    The error message provides one more useful piece of information beyond what you mentioned. It gives the arguments to the call (this is the Arrays.toString(args) part of the message). In this particular example, this part of the information is not very informative (it is [org.checkerframework.checker.nullness.KeyForSubchecker]), but in other cases we have found it helpful, and that's why it is in the message.

    By the way, I completely agree with issue #3700: The Checker Framework should produce a useful error message rather than crashing.

    点赞 评论 复制链接分享
  • weixin_39660408 weixin_39660408 2020-12-27 11:22

    but stack traces are only intended for programmers working on the Checker Framework

    I took the stacktrace in #3700 right from Gradle output, so InvocationTargetException is printed to the console.

    I'm afraid it looks like you miss what I suggest here.

    I suggest the following (see the very last line and the message):

    java
     Throwable err = t.getCause(); 
     if (err instanceof UserError || err instanceof TypeSystemError) { 
         // Don't add another stack frame, just show the message. 
         throw (RuntimeException) err; 
     } 
     throw new BugInCF( 
             String.format( 
                     "Error when invoking constructor %s on args %s: %s", // 
    点赞 评论 复制链接分享
  • weixin_39660408 weixin_39660408 2020-12-27 11:22

    Another InvocationTargetException is caught in https://github.com/typetools/checker-framework/blob/d2b9b3141a0b0de7d201389b2c04f2feee67cac9/framework/src/main/java/org/checkerframework/common/reflection/DefaultReflectionResolver.java#L601-L603

    点赞 评论 复制链接分享
  • weixin_39540934 weixin_39540934 2020-12-27 11:22

    but stack traces are only intended for programmers working on the Checker Framework

    I took the stacktrace in #3700 right from Gradle output, so InvocationTargetException is printed to the console.

    Yes, but it is a bug in the Checker Framework that you saw it. :-/

    3700 is not just a usability problem: the Checker Framework crashed.

    I'm afraid it looks like you miss what I suggest here.

    Yes, I did. Thanks for being more explicit; now I understand. I will open a pull request to make the change. (You can always open a pull request which is fully explicit. Your bug reports are useful even without a pull request, though.)

    点赞 评论 复制链接分享
  • weixin_39660408 weixin_39660408 2020-12-27 11:22

    I will open a pull request to make the change. (You can always open a pull request which is fully explicit. Your bug reports are useful even without a pull request, though.)

    Thanks. Frankly speaking, I have ~1500 checkerframework violations left in Calcite codebase, so it consumes time :-/ If you watch long enough you might see me creating PRs, however, I would need to finish with Calcite first :-)

    点赞 评论 复制链接分享
  • weixin_39540934 weixin_39540934 2020-12-27 11:22

    I understand. Thanks again for all your bug reports and feature suggestions. They are useful!

    点赞 评论 复制链接分享

相关推荐