S2737 — "catch" clauses should do more than rethrow

Language
C#
Type
Code smell
Severity
Minor
Tags
error-handling, unused, finding, clumsy

Why is this an issue?

A catch clause that only rethrows the caught exception has the same effect as omitting the catch altogether and letting it bubble up automatically.


string s = "";
try
{
  s = File.ReadAllText(fileName);
}
catch (Exception e)  // Noncompliant
{
  throw;
}

Such clauses should either be removed or populated with the appropriate logic.


string s = File.ReadAllText(fileName);

or


string s = "";
try
{
  s = File.ReadAllText(fileName);
}
catch (Exception e)
{
  logger.LogError(e);
  throw;
}

Exceptions

This rule will not generate issues for catch blocks if they are followed by a catch block for a more general exception type that does more than just rethrowing the exception.


var s = ""
try
{
    s = File.ReadAllText(fileName);
}
catch (IOException) // Compliant by exception: removing it would change the logic
{
    throw;
}
catch (Exception)  // Compliant: does more than just rethrow
{
    logger.LogError(e);
    throw;
}

A bare catch, or an unfiltered catch (Exception), that only rethrows is also allowed when it creates a required unwind boundary for one of these temporary execution or security contexts:

Only these patterns are recognized. Other scopes that restore ambient state, such as a TransactionScope, a saved CultureInfo, or a custom impersonation helper, remain reported. Catch clauses with a filter or an exception type other than Exception also remain reported.

The CLR evaluates exception filters in a first pass, before unwinding intervening finally and using scopes in a second pass. Without a local catch, a filter higher in the call stack can therefore observe the temporary context before it is restored. The local catch makes the inner cleanup run first, then propagates the original exception from the restored context.


try
{
    WindowsIdentity.RunImpersonated(token, () => DoWork());
}
catch // Compliant: impersonation is restored before an upstream exception filter runs
{
    throw;
}

The cleanup must run in a scope entered inside the protected try, including inside a method it calls, as with RunImpersonated above. A finally attached to the same try as the rethrowing catch does not create this boundary because it runs only after the catch rethrows.


try
{
    DoWork();
}
catch // Noncompliant: the attached finally does not run before an upstream exception filter
{
    throw;
}
finally
{
    RestoreContext();
}

↑ Back to top