#4505 Not Propagating Fatal Errors

[Frodo2412]

Fixes #4505.

• Adds new test, using example AsyncFatalError
• Adds fatal error checking within IOFiber.runLoop run loop for case IOCont, before continuing run loop
• Adds fatal error checking within IOFiber.asyncContinueFailedR for asynchronous continuation

[Frodo2412]

Made test out of this example, but it seems like its green. Could there be something wrong with how I made the test?

[reardonj]

The original reproducer in #4505 runs the async on a fiber and joins it:

        _ <- pingFiber.join
      } yield ExitCode.Success

join returns an object with the outcome of the fiber. This ignores the actual outcome of the async call. Your implementation doesn't use a fiber, so the failure is still propagated and causes your IO to fail. pingFiber also fails, the issue is it doesn't fatally fail.

The core issue with #4505 is that the code shouldn't even get far enough that there is a join result, as the OOM should kill the app immediately. You can probably reproduce this with just an attempt instead of a start/join

Suggested change

	.flatMap(_ => IO.println("sadness"))
	.attempt
	.flatMap(_ => IO.println("sadness"))

or to more closely replicate the original, something like:

Suggested change

	.flatMap(_ => IO.println("sadness"))
	.start
	.flatMap(_.join)
	.flatMap(_ => IO.println("sadness"))

[Frodo2412]

Applied the second change here, it made the test red: ec8421b

Thanks!

[Frodo2412]

@reardonj I made the test red and added turned them green. Added separate test for the completable future case and it seems to be working as well. Does this look like the right approach?

[reardonj]

This solution makes sense to me, though I'm not especially familiar with Cont implementation details. @durban did note that there are some edge cases:

A tricky thing with cont is that the callback might be called with something which might not be used. (Multiple results, and also cont can complete synchronously.)

I think one of these would be eg. the fiber is canceled then the callback is invoked. I don't know how we would handle that offhand since the fiber might already be complete. Hopefully a maintainer can help a bit there. This looks like it covers the normal flows though!

[reardonj]

Is there a reason to sleep?

[Frodo2412]

I just copied the example verbatim from the original ticket, the wait is there: #4505