English 中文(简体)
捕获非特定异常(例如System.Exception)是否是一种不良实践?为什么?
原标题:
  • 时间:2009-01-08 22:44:24
  •  标签:

I am currently doing a code review and the following code made me jump. I see multiple issues with this code. Do you agree with me? If so, how do I explain to my colleague that this is wrong (stubborn type...)?

  • Catch a generic exception (Exception ex)
  • The use of "if (ex is something)" instead of having another catch block
  • We eat SoapException, HttpException and WebException. But if the Web Service failed, there not much to do.

Code:

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;
    }
}
最佳回答

这个咒语是:

  • You should only catch exceptions if you can properly handle them

因此:

  • You should not catch general exceptions.

在您的情况下,是的,您应该捕获那些异常并做一些有用的事情(可能不仅仅是吃掉它们——在记录日志后,您可以抛出它们)。

你的程式员正在使用 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
}

这基本上等同于您的代码所做的操作。您的开发人员可能是这样做的,以避免重复“记录错误并吞噬它”的块。

问题回答

我目前正在进行代码审查,以下代码让我感到惊讶。我看到这段代码存在多个问题。你同意我的看法吗?

不完全,见下文。

  • Catch a generic exception (Exception ex)

一般来说,捕获通用异常是可以的,只要在确定无法处理异常时重新抛出它(使用throw;)。该代码已经这样做了,所以目前没有问题。

  • The use of "if (ex is something)" instead of having another catch block

捕获块的净效应是,实际上只处理了SoapException、HttpException等异常,而所有其他异常都被沿着调用堆栈传播。我猜功能方面这就是代码应该做的,所以这里也没有问题。

然而,从美学和可读性的角度来看,我更喜欢多个catch块而不是“if(ex is SoapException || ..)”。一旦你将常见的处理代码重构为一个方法,多个catch块只需要稍微多打一点,对于大多数开发人员来说更容易阅读。此外,最后的抛出操作很容易被忽略。

  • We eat SoapException, HttpException and WebException. But if the Web Service failed, there not much to do.

这里可能隐藏着代码最大的问题,但如果不了解应用程序的性质,则难以提供建议。如果Web服务调用在后面执行依赖的操作,则仅记录和忽略异常可能不正确。通常情况下,您应该让异常向呼叫者传播(通常是在将其包装成例如XyzWebServiceDownException之后),甚至在多次重试WebService调用后(以在存在偶发网络问题时更加健壮)。

捕获并重新抛出相同异常的问题在于,尽管.NET尽最大努力保持堆栈跟踪不变,但它总是被修改,这可能会使跟踪异常实际来自哪里更加困难(例如,异常行号可能会显示为重新抛出语句的行,而不是最初引发异常的行)。 在这里可以找到有关catch/rethrow、过滤和不捕捉之间的区别的更多信息。

当出现重复逻辑时,您真正需要的是一种异常过滤器,以便仅捕获您感兴趣的异常类型。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 中可以使用委托)代替常见代码,例如:

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);
}

我想在这里加一句,因为我看到的几乎所有Java/C#代码中的异常处理都是不正确的。例如,你最终会得到非常难以调试的错误信息,因为异常被忽略了。或者,同样糟糕的是,你可能会得到一个模糊的异常信息,它告诉你什么都不知道,因为盲目地遵循“捕获(异常)是不好的”这种想法。结果只会更糟。

首先,要理解异常是一种在代码层间传递错误信息的方式。现在,错误1: 一个层不仅仅是一个堆栈帧,它是具有明确定义职责的代码。如果你只是因为这样而编写接口和实现,那么你应该有更好的事情需要修复。

如果分层设计得当并具有特定职责,那么错误信息的意义随着其浮现而有所不同。 -这就是要做什么的关键,没有通用规则。

所以,这意味着当出现异常时,您有两个选项,但需要了解您所处的层级位置:

A) If you are in the middle of a layer, and you are just an internal, normally private, helper function and something goes bad: DONT WORRY, let the caller receive the exception. Its perfectly OK because you have no business context and 1) You are not ignoring the err或者 (huòzhě)and 2) The caller is part of your layer and should have known this can happen, but you might not now the context to handle it down below.

或者...

B) 你是这个层的顶部边界,是内部的表现。如果你遇到了异常,那么默认情况下应该捕获所有异常并阻止它们穿过到上层,因为这对调用者来说没有意义,甚至更糟的是,你可能会发生改变,调用者会依赖实现细节并且两者都会崩溃。

一个应用程序的强度在于层之间的解耦级别。通常情况下,您将停止所有操作并使用通用异常重新抛出错误,将信息转换为对上层更有意义的错误。

规则:所有进入层的入口点都应该通过CATCH ALL进行保护,并且所有的错误都应该被翻译或处理。现在仅有1%的情况会进行处理,大多数情况下,你仅需要(或只能)返回正确的抽象错误信息。 Translated: 规则:所有进入层的入口点都应该通过CATCH ALL进行保护,并且所有的错误都应该被翻译或处理。现在仅有1%的情况会进行处理,大多数情况下,你仅需要(或只能)返回正确的抽象错误信息。

不,我相信这很难理解。真实例子->

我有一个运行一些模拟的软件包。这些模拟是以文本脚本的形式存在的。有一个软件包编译这些脚本,还有一个通用的工具软件包只是读取文本文件,当然,还有基础的Java运行时库。UML依赖是->

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

如果私有文件中的utils载入器出现故障并出现文件未找到、权限或其他错误,那么只需让其继续。没有其他的事情可以做。

2)在边界上,在初始调用utilsTextLoader函数时,您将遵循以上规则和CATCH_ALL。编译器并不在乎发生了什么,它只需要知道文件是否已加载。因此,在catch中,重新抛出新的异常并将FileNotFound或其他任何内容翻译为“无法读取文件XXXX”。

3) 编译器现在将知道源代码未被加载。这就是它需要知道的全部。因此,如果我稍后将utilsTestLoader更改为从网络加载,则编译器不会更改。如果你先放弃FileNotFound然后再更改,你将无意义地影响编译器。

4)循环重复:调用文件的较低层的实际函数在出现异常时不会进行任何操作。因此,它会向上传递异常。

由于异常在仿真器和编译器之间的层被捕获,编译器再次CATCHES_ALL,隐藏任何细节,只抛出一个更具体的错误:“无法编译脚本XXX”。

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

最终边界是用户。用户是一个层,所有内容都应用。主要的try语句捕获所有异常,最后创建一个漂亮的对话框或页面,并将已翻译的错误传递给用户。

所以用户可以看见。


模拟器:致命错误,无法启动模拟器。

编译器:无法编译脚本FOO1。

TextLoader:无法读取文件foo1.scp

文件未找到


相比之下:

编译器:NullPointer异常<-常见情况和一个失去的夜晚调试文件名打字错误

装载器:文件未找到 <- 我有提到过装载器要装载数百个脚本吗?

或者 (huòzhě)

c)什么也没发生,因为一切都被忽略了!!!

当然,这前提是每次重新抛出异常时,你都没有忘记设置原因异常。

好的,这是我的意见。这些简单的规则已经多次挽救了我的生命...

啤酒 (pí jiǔ)

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

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

在这种情况下,可能的捕获块可能如下:

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

   Log(ex);
}

然而,这里有三个重要点:

  1. This isn t something for every situation. In code reviews we are very picky about places where this is actually neccessary and thus allowed.
  2. The ExceptionIsFatal() method assures that we don t eat exceptions which should never be swallowed (ExecutionEngineException, OutOfMemoryException, ThreadAbortException, etc.)
  3. What is swallowed is carefully logged (event-log, log4net, YMMV)

通常情况下,我支持让未捕获的异常通过终止CLR来“崩溃”应用程序的做法。然而,特别是在服务器应用程序中,这种做法有时是不明智的。如果一个线程遇到被认为不是致命的问题,就没有理由将整个进程解体,杀死所有其他正在运行的请求(例如,WCF处理某些情况的方式就是这样)。

通常情况下,你应该在全局处理程序中捕获一般异常(这是记录意外异常的理想位置),但除此以外,正如之前所说,只有计划对它们进行操作时,才应在其他地方捕获特定的异常类型。catch块应明确查找这些异常类型,而不是像你发布的代码那样。

我认为在这种情况下这并不是什么坏事,但我在我的代码中也会使用类似的方式处理能够安全忽略的异常,而将其他异常重新抛出。正如迈克尔的回答所指出的那样,每个catch作为单独的块可能会导致一些可读性问题,这可以通过采用这种方式来避免。

关于指出这一点给你的同事,我认为你很难说服他们这种方法是错误的,尤其是如果他们很固执,因为采用其他方式存在可读性问题的潜在风险。由于这个版本仍然抛出无法处理的通用异常,它符合这种实践的精神。然而,如果代码符合以下要求:

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;
  }
}

那么我认为你应该更加担心,因为在一般异常块中检查特定异常并没有进行特定例外的工作是毫无意义的。

the princeple is only to catch the exception you can handle. such as, if you know how to deal with findnotfound, you catch the filenotfoundexception, otherwiese, do NOT catch it and let it be thrown to the upper layer.





相关问题
热门标签