这是捕获非特定异常（例如 System.Exception）的一种不好的做法吗？为什么？

Question

我目前正在进行代码审查，下面的代码让我大吃一惊。我看到此代码存在多个问题。你是否同意我的观点？如果是这样，我该如何向我的同事解释这是错误的（固执的类型......）？

• 捕获一般异常（Exception ex）
• 使用“if (ex is something)”代替另一个 catch 块
• 我们吃SoapException、HttpException和WebException。但是，如果 Web 服务失败了，就没有什么可做的了。

代码：

try
{
    // Call to a WebService
}
catch (Exception ex)
{
    if (ex is SoapException || ex is HttpException || ex is WebException)
    {
        // Log Error and eat it.
    }
    else
    {
        throw;
    }
}

Answer 1

口头禅是：

• 您应该只在以下情况下捕获异常 你可以妥善处理它们

因此：

• 你不应该抓住将军 例外。

---

在你的情况下，是的，你应该抓住这些异常并做一些有用的事情（可能不仅仅是吃掉它们——你可以在记录它们之后throw）。

您的编码器正在使用throw（不是throw ex），这很好。

这是您可以捕获多个特定异常的方法：

try
{
    // Call to a WebService
}
catch (SoapException ex)
{
    // Log Error and eat it
}
catch (HttpException ex)
{
    // Log Error and eat it
}
catch (WebException ex)
{
    // Log Error and eat it
}

这几乎等同于您的代码所做的。您的开发人员可能这样做是为了避免重复“记录错误并吃掉它”块。

Answer 2

我目前正在进行代码审查，下面的代码让我大吃一惊。我看到此代码存在多个问题。你同意我的观点吗？

不完全是，见下文。

• 捕获一般异常（Exception ex）

一般来说，捕获一个通用异常实际上是可以的，只要你在得出无法处理它的结论时重新抛出它（使用 throw;）。代码就是这样做的，所以这里没有直接的问题。

• 使用“if (ex is something)”代替另一个 catch 块

catch 块的最终效果是实际上只处理了 SoapException、HttpException 等，而所有其他异常都沿调用堆栈向上传播。我想功能方面这是代码应该做的，所以这里也没有问题。

但是，从美学和可读性 POV 来看，我更喜欢多个 catch 块而不是“if (ex is SoapException || ..)”。一旦您将通用处理代码重构为一个方法，多个 catch 块的类型只会稍微多一些，并且对于大多数开发人员来说更容易阅读。此外，最后的投掷很容易被忽视。

• 我们吃SoapException、HttpException和WebException。但是如果 Web 服务失败了，就没有什么可做的了。

这里可能潜伏着代码的最大问题，但如果不了解应用程序的性质，很难给出建议。如果 Web 服务调用正在执行您稍后依赖的操作，那么仅记录并吃掉异常可能是错误的。通常，您让异常传播给调用者（通常在将其包装到例如 XyzWebServiceDownException 之后），甚至可能在重试 Web 服务调用几次之后（在出现虚假网络问题时更加健壮）。

Answer 3

捕获并重新抛出同一个异常的问题在于，尽管 .NET 尽最大努力保持堆栈跟踪完好无损，但它总是最终会被修改，这会使跟踪异常的实际来源变得更加困难（例如，异常行号可能看起来是重新抛出语句的行，而不是最初引发异常的行）。 There's a whole load more information about the difference between catch/rethrow, filtering, and not catching here.

当有这样的重复逻辑时，您真正需要的是一个异常过滤器，以便您只捕获您感兴趣的异常类型。VB.NET 具有此功能，但不幸的是 C# 没有。假设的语法可能如下所示：

try
{
    // Call to a WebService
}
catch (Exception ex) if (ex is SoapException || ex is HttpException || /* etc. */)
{
    // Log Error and eat it
}

虽然你不能这样做，但我倾向于对公共代码使用 lambda 表达式（你可以在 C# 2.0 中使用 delegate），例如

Action<Exception> logAndEat = ex => 
    {  
        // Log Error and eat it
    };

try
{
    // Call to a WebService
}
catch (SoapException ex)
{
    logAndEat(ex);
}
catch (HttpException ex)
{
    logAndEat(ex);
}
catch (WebException ex)
{
    logAndEat(ex);
}

Answer 4

有时这是处理“捕获每个异常”场景的唯一方法，而不是实际捕获每个异常。

我认为有时，例如，低级框架/运行时代码需要确保没有任何异常逃逸。不幸的是，框架代码也无法知道线程执行的代码引发了哪些异常。

在这种情况下，可能的 catch 块可能如下所示：

try
{
   // User code called here
}
catch (Exception ex)
{
   if (ExceptionIsFatal(ex))
     throw;

   Log(ex);
}

然而，这里有三个要点：

1. 这并不适用于所有情况。在代码审查中，我们非常挑剔哪些地方实际上是必要的，因此是允许的。
2. ExceptionIsFatal() 方法确保我们不会吃掉不应被吞下的异常（ExecutionEngineException、OutOfMemoryException、ThreadAbortException 等）
3. 仔细记录吞下的内容（事件日志、log4net、YMMV）

通常，我完全赞成通过终止 CLR 让未捕获的异常简单地“崩溃”应用程序的做法。然而，尤其是在服务器应用程序中，这有时是不明智的。如果一个线程遇到了一个被认为是非致命的问题，那么没有理由将整个进程撕下来，杀死所有其他正在运行的请求（例如，WCF 以这种方式处理某些情况）。

Answer 5

我想在这里补充一下，因为我看到的几乎所有java / C#代码中的异常处理都是不正确的。 IE。对于被忽略的异常，您最终会遇到非常难以调试的错误，或者同样糟糕的是，您会得到一个不告诉您任何信息的晦涩异常，因为盲目地遵循“捕获（异常）是不好的”，事情会变得更糟。

首先，了解异常是一种便于跨代码层返回错误信息的方式。现在，错误 1：一个层不仅仅是一个堆栈帧，一个层是具有明确职责的代码。如果您只是编写接口和实现代码只是因为，那么您有更好的东西要修复。

如果层设计得很好并且有特定的职责，那么错误信息在冒泡时具有不同的含义。

因此，这意味着当发生异常时，您有 2 个选项，但您需要了解您在层中的哪个位置：

A) 如果你在一个层的中间，并且你只是一个内部的、通常是私有的、帮助函数并且出现了问题：不要担心，让调用者接收异常。完全可以，因为您没有业务背景，并且 1）您没有忽略错误并且 2）调用者是你层的一部分，应该知道这可能发生，但你现在可能没有上下文来处理它。

或者...

B) 你是层的顶部边界，内部的立面。然后，如果你得到一个异常，默认应该是 CATCH ALL 并阻止任何特定的异常跨越到上层，这对调用者来说没有意义，或者更糟糕的是，你可能会改变并且调用者将依赖于一个实现细节，两者都会打破。

应用程序的优势在于层与层之间的解耦程度。在这里，您将作为一般规则停止一切并使用一般异常重新抛出错误，将信息转换为对上层更有意义的错误。

规则：层的所有入口点都应使用 CATCH ALL 保护，并翻译或处理所有错误。现在这种“已处理”仅发生 1% 的时间，大多数情况下您只需要（或可以）以正确的抽象返回错误。

不，我确信这很难理解。真实示例 ->

我有一个运行一些模拟的包。这些模拟在文本脚本中。有一个包可以编译这些脚本，还有一个通用的 utils 包，它只读取文本文件，当然还有基本的 java RTL。 UML 依赖是->

模拟器->编译器->utilsTextLoader->Java文件

1) 如果某个私有内部的 utils 加载器出现问题，并且我得到 FileNotFound、Permissions 或其他任何内容，那就让它过去吧。你无能为力。

2) 在边界处，在最初调用的 utilsTextLoader 函数中，您将遵循上述规则和 CATCH_ALL。编译器不关心发生了什么，它只需要现在是否加载了文件。因此，在捕获中，重新抛出一个新异常并将 FileNotFound 或其他任何内容转换为“无法读取文件 XXXX”。

3) 编译器现在将知道源没有加载。这就是它需要知道的一切。因此，如果我稍后将 utilsTestLoader 更改为从网络加载，编译器将不会更改。如果您放开 FileNotFound 并在以后进行更改，您将一无所获。

4) 循环重复：为文件调用底层的实际函数在收到异常时将不执行任何操作。所以它让它上升。

5) 当异常到达模拟器和编译器之间的层时，编译器再次 CATCHES_ALL，隐藏任何细节并抛出更具体的错误：“无法编译脚本 XXX”

6) 最后再循环一次，调用编译器的模拟器函数就放手了。

7) 最后的边界是用户。用户是一个 LAYER 并且都适用。 main 尝试 catches_ALL 并最终创建一个漂亮的对话框或页面并向用户“抛出”翻译错误。

所以用户看到了。

---

模拟器：致命错误无法启动模拟器

-编译器：无法编译脚本 FOO1

--TextLoader: 无法读取文件 foo1.scp

---trl: FileNotFound

---

比较：

a) 编译器：NullPointer 异常

b) 加载器：找不到文件

或

c) 什么都没有发生，因为一切都被忽略了！！！

当然，这假设在每次重新抛出时您都没有忘记设置原因异常。

好吧，我的 2cts。这个简单的规则多次救了我的命……

-啤酒

Answer 6

您通常仍应在全局处理程序中捕获通用异常（这是记录意外异常的理想场所），但除此之外，如前所述，您应该只在其他地方捕获特定异常类型，如果您打算对它们做某事. catch 块应该显式查找这些异常类型，而不是像您发布的代码那样。

Answer 7

我不认为在这种情况下这是一件坏事，但我也在我的代码中做了类似的事情，可以安全地忽略异常被捕获而其余的被重新抛出。正如Michael's response 所指出的，将每个捕获作为一个单独的块可能会导致一些可读性问题，这些问题可以通过走这条路线来防止。

关于向你的同事指出这一点，我认为你很难说服他们这是错误的做事方式——如果他们很固执，更是如此——因为做事可能存在可读性问题另一种方法。由于此版本仍然抛出无法处理的通用异常，因此符合实践的精神。但是，如果代码符合以下：

try
{
  // Do some work
}
catch (Exception ex)
{
  if (ex is SoapException)
  {
    // SoapException specific recovery actions
  }
  else if (ex is HttpException)
  {
    // HttpException specific recovery actions
  }
  else if (ex is WebException)
  {
    // WebException specific recoery actions
  }
  else
  {
    throw;
  }
}

那么我认为你会有更多的理由担心，因为在一般异常块中检查特定异常是没有意义的。

Answer 8

princeple 只是为了捕获您可以处理的异常。 比如，如果你知道如何处理findnotfound，你就捕获filenotfoundexception，否则，不要捕获它，让它扔到上层。