Use our own NonFatal exception classifier

[armanbilge]

In #4247 (comment) I was caught by surprise that InterruptedException is considered fatal by Scala's NonFatal. It's not clear to me or Daniel S why this is the case.

An example where you could reasonably get InterruptedExceptions cropping up on the runtime, is where a Resource creates a threadpool, runs some fibers on it, and cleans that threadpool up by interrupting+joining its threads. Depending on what those fibers are doing at the time the pool is shutdown, they may raise InterruptedExceptions.

Daniel suggested that since we already have so much exception-handling machinery, it is not a huge lift to have our own "NonFatal" that considers InterruptedException to indeed be non-fatal.

[djspiewak]

Searching my memory banks... I think @viktorklang might be the only one with all the context on why InterruptedException was excluded from NonFatal in the first place. If I had to guess, it was a decision made to try to encourage user code to not handle it (since most of the time, catching it is a bad idea). In a sense, InterruptedException falls in a weird middle ground between truly fatal things like OOM and completely benign things like IOException, where it's not safe to treat it like other exceptions, but it's also not really something that burns down the JVM on its way out the door.

[mpilquist]

In case it's useful:

• Removed requirement for ExecutionContext from API fs2#1149
• typelevel/fs2@6bdd49f
• https://github.com/typelevel/fs2/blob/f17c53999f6a6a3ed4f421f4c010e38f92203346/core/shared/src/main/scala/fs2/internal/NonFatal.scala

[kapunga]

Was taking a look at this to maybe open a PR. In addition to InterruptedException, the normal Scala NonFatal accepts VirtualMachineError, ThreadDeath,LinkageError, and ControlThrowable as fatal, while the fs2 NonFatal only accepts VirtualMachineError. Is there any reason why the other three weren't included? As far as I can tell:

• ThreadDeath - occurs when someone calls Thread.stop(), which has been deprecated since Java 1.2 and no one should be doing ever.
• ControlThrowable - base Exception class for exceptions which are thrown for flow control, which at the very least breaks referential transparency and in my opinion no one should be doing ever.
• LinkageError - occurs when there is a compiled incompatibility between a class and one that it depends on (signature change?) which seems highly unlikely and unrecoverable. I'm not even sure how this would happen happen anymore, maybe some library eviction on compilation?

Is there any reason why VirtualMachineError, ThreadDeath,LinkageError, and ControlThrowable shouldn't all be fatal? I don't see one.

[armanbilge]

Thanks @kapunga! I'd start with the current implementation of Scala's NonFatal, minus the InterruptedException. In the future if we encounter other bugs and determine that another one of these exceptions does not need to be considered fatal, it should be easy enough to make the change :)

[kapunga]

I opened a Draft PR here: #4263

Right now, I'd love feedback on the naming, placement, and documentation of UnsafeNonFatal. I'll be going through later to see if I can figure out what usages of scala.util.control.NonFatal should be replaced with UnsafeNonFatal.