Java：错误处理不佳，最终抛出答案

【问题标题】：Java: Poor error handling, Throw inside FinallyJava：错误处理不佳，最终抛出
【发布时间】：2019-08-27 11:26:24
【问题描述】：

我有以下代码，我正在通过 fortify 运行。为什么它被标记为糟糕的错误处理，最后扔进去？

private String getResourceContent(String fileName) throws IOException {

    try (InputStream resource = ErrorResource.classLoader.getResourceAsStream(fileName)) {
        return new String(resource.readAllBytes(), StandardCharsets.UTF_8);
    } catch (NullPointerException n) {
        throw new ErrorDescriptorException(
                String.format("Error loading Error description data from Resource file [%s].", fileName), n);
    }
}

【问题讨论】：

你为什么要使用 NPE 而不是基本的 if-else 检查null？很少有正当的理由抓住 NPE。只需if (resource == null) { ... }。
见Throwing an exception inside finally 来自Software Engineering。

标签： java fortify

【解决方案1】：

说明

这在官方文档中有很好的解释（见Poor Error Handling: Throw Inside Finally）。让我快速引用重要的部分：

在finally 块内使用throw 语句会中断通过try-catch-finally 的逻辑进程。

在 Java 中，finally 块总是在其相应的 try-catch 块之后执行，并且通常用于释放分配的资源，例如文件句柄或数据库游标。在 finally 块中抛出异常可以绕过关键的清理代码，因为正常的程序执行将被中断。

因此，您可以轻松绕过清理代码，从而导致资源泄漏。

虽然在您的代码中不直接可见，但您实际上有一个 hidden finally 块，因为您使用的是 try-with-resources，它会自动关闭资源最后阻塞。

另请参阅Throwing an exception inside finally，其中已对此进行了讨论。

示例

以下是官方文档中的示例：

public void processTransaction(Connection conn) throws FileNotFoundException {
    FileInputStream fis = null;
    Statement stmt = null;
    try {
        stmt = conn.createStatement();
        fis = new FileInputStream("badFile.txt");
        ...
    } catch (FileNotFoundException fe) {
        log("File not found.");
    } catch (SQLException se) {
        // handle error
    } finally {
        if (fis == null) {
            // This bypasses cleanup code
            throw new FileNotFoundException();
        }

        if (stmt != null) {
            try {
                // Not executed if the exception is thrown
                stmt.close();
            }
            catch (SQLException e) {
                log(e);
            }
        }
    }
}

当FileNotFoundException 被抛出时，对stmt.close() 的调用被绕过。

注意

您为什么使用NullPointerException 而不是基本的if-else 检查null？捕获NullPointerException 的正当理由很少。做吧：

try (InputStream resource = ErrorResource.classLoader.getResourceAsStream(fileName)) {
    if (resource == null) {
        // TODO Throw your exception here
    }
    return new String(resource.readAllBytes(), StandardCharsets.UTF_8);
}

通过说明找不到资源的确切原因，这也可能有助于改进错误消息。

【讨论】：

NPE 也可以在 ErrorResource.classLoader.getResourceAsStream(fileName) 中出现。如果由引导类加载器加载，ErrorResource.classLoader 可以为空？
但是catch块中抛出的异常是如何中断finally块的呢？ finally 块中抛出的异常会转义整个语句，而不是被 catch 块捕获。
@KaranKhanna 然后在那里检查它。不要对常规控制流使用异常，尽可能使用 if-else。而对于 NPE，这几乎总是可能的。
@Zabuza 我把它改成了：try (InputStream resource = classLoader.getResourceAsStream(fileName)) { if (resource == null) { throw new ErrorDescriptorException( String.format("错误加载错误描述数据来自资源文件 [%s].", fileName)); } return new String(resource.readAllBytes(), StandardCharsets.UTF_8); } 还是一样的问题。
如果您暂时从方法中删除throw new Error... all，它会消失吗？您能否编辑您的问题并包含更多信息？喜欢完整的课程和完整的警告信息？也许它指向不同的位置。

【解决方案2】：

除了来自您的工具的误导消息之外，您的代码中实际上存在糟糕的错误处理，原因有很多：

捕捉 NPE 确实是 不好的 做法。要么是一个错误（不应该为 null 的东西），要么您的代码缺少检查 if (whatever == null) 以及处理该预期情况的相应代码
假设此 NPE 与您在新异常中表达的含义完全相同，只是猜测

换句话说：没有更多信息，不清楚究竟你的工具抱怨了什么。但是：不需要工具就能理解：这是糟糕的错误处理。

除此之外，此类工具通常会提供有关其警告的某种信息。含义：该警告可能带有“错误 ID”，您应该能够在工具的文档中查找该“错误 ID”以获得进一步的解释。

【讨论】：

您的 cmets 通常可能是正确的，但不要回答问题。如果它是任何其他异常类型，则问题不会发生重大变化。
@MichaelPiefel 我不同意。如果该代码会抛出一些“内部”运行时异常，并且 try/catch 的全部目的是将预期的运行时异常转换为某个检查版本怎么办？
我的评论中没有什么可以同意的。 OP 询问“为什么 Fortify 会在最后抛出错误时给出错误”，而您的回答没有说明该主题。这不是我的观点，而是事实。
@MichaelPiefel 当你转向源代码时......那里没有 finally 语句。我的回答涉及更广泛的背景。比如：为什么该代码会“质量差”，以及如何处理此类问题（转到您的工具的文档）。 OP 已经收到了更具体的答案，但请始终记住，还有其他未来的读者可能会带着稍微不同的问题来到这里。

【解决方案3】：

考虑以下代码，它大致基于您的代码：

String throwing(InputStream inputStream) throws IOException {
    try (InputStream resource = inputStream) {
        return "good";
    } catch (NullPointerException n) {
        return "bad";
    }
}

你看，这里没有抛出异常。不过，您不能删除 throws IOException 位 - 怎么样？好吧，InputStream#close() 可以抛出它，它将在 try-with-resources 语句创建的隐式 finally 块中。我想您对此无能为力，它看起来像是 Fortify 误报。

【讨论】：