"Catch throwable" warning duplicates scalac built in warning

[RichardBradley]

The "Catch Throwable" warning will complain if you use

  catch e: Throwable => ...

But this code is encouraged by the scalac compiler in order to avoid one of its own warnings:

scala> def foo = {
     |   try {
     |     println("stuff here")
     |   } catch {
     |     case e => e.printStackTrace()
     |   }
     | }
<console>:11: warning: This catches all Throwables. If this is really intended, use `case e : Throwable` to clear this warning.
           case e => e.printStackTrace()
                ^

Therefore, I think that catch e: Throwable is not a code smell, but a deliberate pattern used to catch all throwables where that is appropriate (e.g. a last-ditch log-and-exit handler).

[sksamuel]

I think catching Throwables is a bit of a smell, as Errors are used to indicate something you can't / not supposed to recover from. ClassFormatErrors and OOMEs etc. But if scalac says one thing, the easist thing is to allow you to turn this off.

[RichardBradley]

I think catching Throwables is a bit of a smell, as Errors are used to indicate something you can't / not supposed to recover from.

I agree that Errors are not usually recoverable, but there are a few cases where catching them is appropriate:

• logging or cleanup followed by a rethrow without recovery
• isolating a container system from errors by "plugins" or child systems

This is rarer than "catch NonFatal(e)", but still not uncommon. See e.g. https://github.com/akka/akka/search?utf8=%E2%9C%93&q=catch+throwable

[sksamuel]

Just because some project decides to catch Throwables, doesn't mean it's correct.

I never use catch NonFatal for same reason - someone decided some Errors should be Exceptions instead. But its all personal opinion.

On 27 April 2015 at 10:10, Richard Bradley [email protected] wrote:

I think catching Throwables is a bit of a smell, as Errors are used to indicate something you can't / not supposed to recover from.

I agree that Errors are not usually recoverable, but there are a few cases where catching them is appropriate:

• logging or cleanup followed by a rethrow without recovery
• isolating a container system from errors by "plugins" or child systems

This is rarer than "catch NonFatal(e)", but still not uncommon. See e.g. https://github.com/akka/akka/search?utf8=%E2%9C%93&q=catch+throwable

— Reply to this email directly or view it on GitHub https://github.com/sksamuel/scalac-scapegoat-plugin/issues/103#issuecomment-96577457 .