Это плохая практика, чтобы поймать неспецифическое исключение, такое как System.Exception? Зачем?

Question

Я сейчас делаю обзор кода, и следующий код заставил меня прыгнуть. Я вижу несколько проблем с этим кодом. Согласен ли ты со мной? Если да, то как мне объяснить моему коллеге, что это неправильно (упрямый тип ...)?

• Поймать универсальное исключение (Exception ex)
• Использование «if (ex - что-то)» вместо использования другого блока catch
• Мы едим SoapException, HttpException и WebException. Но если веб-служба потерпела неудачу, делать особо нечего.

Код:

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 - что-то)» вместо использования другого блока catch

Общий эффект блока catch заключается в том, что фактически обрабатываются только SoapException, HttpException и т. Д., А все остальные исключения распространяются вверх по стеку вызовов. Я предполагаю, что с точки зрения функциональности это то, что должен делать код, поэтому здесь тоже нет проблем.

Однако, исходя из эстетики и удобочитаемости POV, я бы предпочел несколько блоков catch «if (ex is SoapException || ..)». Как только вы реорганизуете общий код обработки в метод, несколько блоков catch становятся лишь немного более типичными и их легче читать большинству разработчиков. Кроме того, последний бросок легко пропускается.

• Мы едим SoapException, HttpException и WebException. Но если веб-службы не удалось, там не так много, чтобы сделать.

Здесь, возможно, таится самая большая проблема кода, но трудно дать совет, не зная больше о природе приложения. Если вызов веб-службы делает что-то, от чего вы зависите позже, то, вероятно, неправильно просто регистрировать и есть исключения. Как правило, вы позволяете исключению распространяться на вызывающую сторону (обычно после помещения его, например, в исключение XyzWebServiceDownException), возможно, даже после нескольких повторных попыток вызова веб-службы (чтобы быть более надежным при наличии ложных проблем в сети).

Answer 3

Проблема с перехватом и повторным вызовом одного и того же исключения заключается в том, что, хотя .NET делает все возможное, чтобы сохранить трассировку стека нетронутой, она всегда заканчивается изменением, которое может усложнить отслеживание того, откуда на самом деле возникло исключение ( например, номер строки исключения, скорее всего, будет выглядеть как строка оператора re-throw, а не строка, в которой изначально было создано исключение). Здесь вы найдете больше информации о разнице между уловом / повторным броском, фильтрацией и не отловом.

Когда есть подобная повторяющаяся логика, вам действительно нужен фильтр исключений, так что вы перехватываете только те типы исключений, которые вам интересны. В VB.NET есть эта функциональность, но, к сожалению, в C # нет. Гипотетический синтаксис может выглядеть так:

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

Поскольку вы не можете сделать это, я вместо этого использую лямбда-выражение для общего кода (вы можете использовать delegate в 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);
}

Answer 4

Я хотел бы добавить сюда, потому что обработка исключений почти во всех кодах java / C #, которые я видел, просто некорректна. То есть в результате вы получаете очень сложную отладочную ошибку для игнорируемых исключений, или, что не менее плохо, вы получаете неясное исключение, которое ничего вам не говорит, потому что слепое следование «отлову (исключение) - это плохо», а все просто хуже.

Во-первых, следует понимать, что исключение - это способ облегчения возврата информации об ошибках между уровнями кода. Теперь ошибка 1: слой - это не просто стековый фрейм, а слой - это код с четко определенной ответственностью. Если вы просто закодировали интерфейсы и подразумеваете это просто потому, что у вас есть более важные вещи, которые нужно исправить.

Если слои хорошо спроектированы и имеют определенные обязанности, то информация об ошибке имеет различное значение, поскольку она всплывает. <-это ключ к тому, что делать, универсального правила не существует. </p>

Итак, это означает, что когда возникает исключение, у вас есть 2 варианта, но вам нужно понять, где в слое вы находитесь:

A) Если вы находитесь в середине уровня, и вы просто внутренняя, обычно частная, вспомогательная функция и что-то идет не так: НЕ БЕСПОКОЙСТВУЙТЕ, пусть вызывающая сторона получит исключение. Это совершенно нормально, потому что у вас нет бизнес-контекста и 1) Вы не игнорируете ошибку и 2) Вызывающий объект является частью вашего слоя и должен был знать, что это может произойти, но теперь вы можете не узнать контекст для его обработки ниже.

или ...

Б) Вы верхняя граница слоя, фасад к внутренностям. Тогда, если вы получите исключение, по умолчанию должно быть CATCH ALL и остановка любых определенных исключений от перехода на верхний уровень, который не будет иметь смысла для вызывающей стороны, или, что еще хуже, вы можете измениться, и вызывающая сторона будет зависеть от реализации деталь и то и другое сломается.

Сила приложения - это уровень развязки между слоями. Здесь вы остановите все как общее правило и сбросите ошибку с общим исключением, переводя информацию в более значимую ошибку для верхнего уровня.

ПРАВИЛО: Все точки входа в слой должны быть защищены CATCH ALL, а все ошибки переведены или обработаны. Теперь это происходит только в 1% случаев, в основном вам просто нужно (или можно) вернуть ошибку в правильной абстракции.

Нет, я уверен, что это очень трудно понять. реальный пример ->

У меня есть пакет, который запускает некоторые симуляции. Эти симуляции в текстовых скриптах. есть пакет, который компилирует эти сценарии, и есть универсальный пакет утилит, который просто читает текстовые файлы и, конечно, базовый java RTL. UML-зависимость ->

Симулятор-> Компилятор-> utilsTextLoader-> Файл Java

1) Если что-то ломается в загрузчике утилит в одном приватном файле, и я получаю FileNotFound, Permissions или что-то еще, просто позвольте этому пройти. Больше ничего не поделаешь.

2) На границе, в первоначально вызванной функции utilsTextLoader вы будете следовать приведенному выше правилу и CATCH_ALL. Компилятору все равно, что произойдет, просто нужно сейчас загрузить файл или нет. Таким образом, в подвохе повторно выведите новое исключение и переведите FileNotFound или что-либо еще в «Не удалось прочитать файл XXXX».

3) Компилятор теперь будет знать, что источник не был загружен. Это ВСЕ, что нужно знать. Поэтому, если я позже изменю utilsTestLoader для загрузки из сети, компилятор не изменится. Если вы отпустите FileNotFound и позже измените его, вы окажете влияние на компилятор даром.

4) Цикл повторяется: фактическая функция, которая вызвала нижний уровень для файла, ничего не сделает при получении исключения. Так что это позволяет ему подняться.

5) Как исключение попадает в слой между симулятором и компилятором, компилятор снова CATCHES_ALL, скрывая любые детали и просто выдает более конкретную ошибку: «Не удалось скомпилировать скрипт XXX»

6) Наконец, повторите цикл еще раз, функция симулятора, которая вызывает компилятор, просто отпускает.

7) Наконец, граница для пользователя. Пользователь это СЛОЙ и все относится. У основного есть попытка, которая делает catches_ALL и, наконец, просто создает красивое диалоговое окно или страницу и «выдает» переведенную ошибку пользователю.

Так видит пользователь.

---

Симулятор: фатальная ошибка не может запустить симулятор

-Compiler: не удалось скомпилировать скрипт FOO1

- TextLoader: не удалось прочитать файл foo1.scp

--- trl: FileNotFound

---

Сравнить с:

a) Компилятор: исключение NullPointer <- общий случай и потерянная ночь отладки опечатки имени файла </p>

b) Загрузчик: файл не найден <- Я упоминал, что загрузчик загружает сотни сценариев ?? </p>

или

в) Ничего не происходит, потому что все было проигнорировано !!!

Конечно, это предполагает, что при каждом перебрасывании вы не забыли установить причину исключения.

Ну, мои 2ct. Эти простые правила много раз спасали мою жизнь ...

-Ale

Answer 5

Иногда это единственный способ справиться со сценариями «поймать каждое исключение», фактически не перехватывая каждое исключение.

Я думаю, что иногда, скажем, низкоуровневый фреймворк / код времени выполнения должен убедиться, что ни одно исключение не ускользнет. К сожалению, код фреймворка также не может знать, какие исключения вызываются кодом, выполняемым потоком.

В этом случае возможный блок catch может выглядеть следующим образом:

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

   Log(ex);
}

Здесь есть три важных момента:

1. Это не что-то для каждой ситуации. В обзорах кода мы очень требовательны к местам, где это на самом деле необходимо и, следовательно, разрешено.
2. Метод ExceptionIsFatal () гарантирует, что мы не будем использовать исключения, которые никогда не следует проглатывать (ExecutionEngineException, OutOfMemoryException, ThreadAbortException и т. Д.)
3. То, что глотается, тщательно регистрируется (журнал событий, log4net, YMMV) * ​​1015 *

Как правило, я все за то, чтобы позволить неперехваченным исключениям просто "вылетать" из приложения, завершая работу CLR. Однако, особенно в серверных приложениях, это иногда не имеет смысла. Если один поток сталкивается с проблемой, которая считается нефатальной, нет причин прерывать весь процесс, убивая все остальные выполняющиеся запросы (например, WCF обрабатывает некоторые случаи таким образом).

Answer 6

Обычно вы все равно должны ловить универсальные исключения в глобальном обработчике (который является идеальным местом для записи неожиданных исключений), но в противном случае, как было сказано выше, вы должны ловить определенные типы исключений только в других местах, если вы планируете что-то с ними делать , Блоки catch должны явно искать эти типы исключений, а не код, который вы опубликовали.

Answer 7

принцип только для того, чтобы поймать исключение, с которым вы можете справиться. например, если вы знаете, как обращаться с findnotfound, вы ловите исключение файла notfoundexception, в противном случае НЕ перехватываете его и позволяете выбросить на верхний уровень.

Answer 8

Я не думаю, что в этом случае это так уж плохо, но я также делаю что-то похожее в моем коде с исключениями, которые можно безопасно проигнорировать, поймав, а остальные перебросив. Как отмечается в ответе Майкла , наличие каждого улова в виде отдельного блока может вызвать некоторые проблемы с читабельностью, которые могут быть предотвращены при прохождении этого маршрута.

Что касается указания на это вашему коллеге, я думаю, вам будет трудно убедить их в том, что это неправильный способ ведения дел - даже в большей степени, если они упрямы - из-за потенциальных проблем с читаемостью при выполнении действий. другой способ. Поскольку эта версия по-прежнему создает общие исключения, которые не могут быть обработаны, она соответствует духу практики. Однако если код соответствует следующему:

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

Тогда, я думаю, у вас будет немного больше поводов для беспокойства, поскольку нет смысла выполнять работу для конкретного исключения, проверяя его в блоке общего исключения.