Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mrdziuban opened this issue Jun 27, 2023 · 3 comments
Open
Labels

Comments

@mrdziuban
Copy link
Contributor

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 mrdziuban added the bug label Jun 27, 2023
@armanbilge
Copy link
Member

I've given an explanation in #3199 (comment) 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 typelevel/cats-mtl#489.

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

@mrdziuban
Copy link
Contributor Author

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

@armanbilge
Copy link
Member

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 ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants