Merge pull request #2935 from djspiewak/bug/fatal-error-continuation

Resolved issue with fatal errors being eaten

core/shared/src/main/scala/cats/effect/IOFiber.scala

@@ -1496,7 +1496,7 @@ private final class IOFiber[A] private (
     runtime.shutdown()
 
     // Make sure the shutdown did not interrupt this thread.
-    Thread.interrupted()
+    val interrupted = Thread.interrupted()
 
     var idx = 0
     val tables = runtime.fiberErrorCbs.tables
@@ -1518,8 +1518,11 @@ private final class IOFiber[A] private (
       idx += 1
     }
 
-    Thread.currentThread().interrupt()
-    null
+    if (interrupted) {
+      Thread.currentThread().interrupt()
+    }
+
+    throw t
   }
 
   // overrides the AtomicReference#toString

tests/js/src/main/scala/catseffect/examplesplatform.scala

@@ -32,10 +32,14 @@ package examples {
     val apps = mutable.Map.empty[String, IOApp]
     def register(app: IOApp): Unit = apps(app.getClass.getName.init) = app
 
+    val rawApps = mutable.Map.empty[String, RawApp]
+    def registerRaw(app: RawApp): Unit = rawApps(app.getClass.getName.init) = app
+
     register(HelloWorld)
     register(Arguments)
     register(NonFatalError)
     register(FatalError)
+    registerRaw(FatalErrorRaw)
     register(Canceled)
     register(GlobalRacingInit)
     register(ShutdownHookImmediateTimeout)
@@ -53,7 +57,11 @@ package examples {
         // emulates the situation in browsers
         js.Dynamic.global.process.exit = js.undefined
       args.shift()
-      apps(app).main(Array.empty)
+      apps
+        .get(app)
+        .map(_.main(Array.empty))
+        .orElse(rawApps.get(app).map(_.main(Array.empty)))
+        .get
     }
   }
 

tests/jvm/src/test/scala/cats/effect/IOAppSpec.scala

@@ -27,8 +27,8 @@ import scala.sys.process.ProcessBuilder
 class IOAppSpec extends Specification {
 
   abstract class Platform(val id: String) { outer =>
-    def builder(proto: IOApp, args: List[String]): ProcessBuilder
-    def pid(proto: IOApp): Option[Int]
+    def builder(proto: AnyRef, args: List[String]): ProcessBuilder
+    def pid(proto: AnyRef): Option[Int]
 
     def dumpSignal: String
 
@@ -37,7 +37,7 @@ class IOAppSpec extends Specification {
       ()
     }
 
-    def apply(proto: IOApp, args: List[String]): Handle = {
+    def apply(proto: AnyRef, args: List[String]): Handle = {
       val stdoutBuffer = new StringBuffer()
       val stderrBuffer = new StringBuffer()
       val p = builder(proto, args).run(BasicIO(false, stdoutBuffer, None).withError { in =>
@@ -71,13 +71,13 @@ class IOAppSpec extends Specification {
 
     val dumpSignal = "USR1"
 
-    def builder(proto: IOApp, args: List[String]) = Process(
+    def builder(proto: AnyRef, args: List[String]) = Process(
       s"$JavaHome/bin/java",
       List("-cp", ClassPath, proto.getClass.getName.replaceAll("\\$$", "")) ::: args)
 
     // scala.sys.process.Process and java.lang.Process lack getting PID support. Java 9+ introduced it but
     // whatever because it's very hard to obtain a java.lang.Process from scala.sys.process.Process.
-    def pid(proto: IOApp): Option[Int] = {
+    def pid(proto: AnyRef): Option[Int] = {
       val mainName = proto.getClass.getSimpleName.replace("$", "")
       val jpsStdoutBuffer = new StringBuffer()
       val jpsProcess =
@@ -93,14 +93,14 @@ class IOAppSpec extends Specification {
   object Node extends Platform("node") {
     val dumpSignal = "USR2"
 
-    def builder(proto: IOApp, args: List[String]) =
+    def builder(proto: AnyRef, args: List[String]) =
       Process(
         s"node",
         "--enable-source-maps" :: BuildInfo
           .jsRunner
           .getAbsolutePath :: proto.getClass.getName.init :: args)
 
-    def pid(proto: IOApp): Option[Int] = {
+    def pid(proto: AnyRef): Option[Int] = {
       val mainName = proto.getClass.getName.init
       val stdoutBuffer = new StringBuffer()
       val process =
@@ -210,6 +210,14 @@ class IOAppSpec extends Specification {
           val h = platform(FatalError, List.empty)
           h.awaitStatus() mustEqual 1
           h.stderr() must contain("Boom!")
+          h.stdout() must not(contain("sadness"))
+        }
+
+        "exit on fatal error without IOApp" in {
+          val h = platform(FatalErrorRaw, List.empty)
+          h.awaitStatus()
+          h.stdout() must not(contain("sadness"))
+          h.stderr() must not(contain("Promise already completed"))
         }
 
         "exit on fatal error with other unsafe runs" in {

tests/shared/src/main/scala/catseffect/RawApp.scala

@@ -0,0 +1,21 @@
+/*
+ * Copyright 2020-2022 Typelevel
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package catseffect
+
+trait RawApp {
+  def main(args: Array[String]): Unit
+}

tests/shared/src/main/scala/catseffect/examples.scala

@@ -40,7 +40,20 @@ package examples {
 
   object FatalError extends IOApp {
     def run(args: List[String]): IO[ExitCode] =
-      IO(throw new OutOfMemoryError("Boom!")).as(ExitCode.Success)
+      IO(throw new OutOfMemoryError("Boom!"))
+        .attempt
+        .flatMap(_ => IO.println("sadness"))
+        .as(ExitCode.Success)
+  }
+
+  object FatalErrorRaw extends RawApp {
+    def main(args: Array[String]): Unit = {
+      import cats.effect.unsafe.implicits._
+      val action =
+        IO(throw new OutOfMemoryError("Boom!")).attempt.flatMap(_ => IO.println("sadness"))
+      action.unsafeToFuture()
+      ()
+    }
   }
 
   object Canceled extends IOApp {