fs2 icon indicating copy to clipboard operation
fs2 copied to clipboard

`fs2.io.readOutputStream` doesn't preserve errors from `cats.data.EitherT`

Open mrdziuban opened this issue 2 years ago • 3 comments

Using the latest fs2 version 3.7.0 and an effect type that includes cats.data.EitherT, an error in the effect is not preserved when using fs2.io.readOutputStream. Possibly related to #2821

Minimized reproduction:

https://scastie.scala-lang.org/mrdziuban/qLvScG5BSWiLsXlhB5pJYg/2

import cats.data.EitherT
import cats.effect.IO
import cats.effect.unsafe.implicits.global

type Eff[A] = EitherT[IO, String, A]

fs2.io.readOutputStream[Eff](1024)(_ => EitherT.left(IO.pure("error")))
  .compile
  .drain
  .value
  .unsafeRunSync()
// Right(())

mrdziuban avatar Jun 27 '23 20:06 mrdziuban

I've given an explanation in https://github.com/typelevel/fs2/issues/3199#issuecomment-1496592183 why EitherT in general doesn't work well with streams. I know it doesn't help immediately, but the long-term solution is a strategy like proposed in https://github.com/typelevel/cats-mtl/pull/489.

Not to say this very specific issue isn't fixable, perhaps it is. But monad transformer behavior in general is not fixable.

armanbilge avatar Jun 27 '23 23:06 armanbilge

Thanks for the links @armanbilge! I've refactored my code to remove EitherT so at least this isn't blocking me.

This heads towards architectural questions, but given

monad transformer behavior in general is not fixable

should the default behavior of fs2 be to not allow stream usage with monad transformers (or for cats to not provide the fallback Concurrent instance)? I personally would have preferred a compile error to the unexpected runtime behavior

mrdziuban avatar Jun 28 '23 21:06 mrdziuban

or for cats to not provide the fallback Concurrent instance

This is a very good point. I feel inclined to agree, these instances seem more harmful than good, and actually the trouble starts from MonadCancel although I'm not sure if it is observable until Concurrent. This is a discussion for the Cats Effect repository, I can open that ...

armanbilge avatar Jun 28 '23 21:06 armanbilge