zio / interop-cats

ZIO instances for cats-effect type classes
Apache License 2.0
155 stars 66 forks source link

Bad interaction between doobie's unique and ZIO.either #13

Closed grmble closed 5 years ago

grmble commented 5 years ago

If a unique doobie select fails because the row does not exist, and that Task is turned into an UIO via either, the result is not a Left as I would have expected, but an Exception is thrown.

I don't think the problem is with doobie - when using cats-effect instead of zio the behavior is as expected.

import utest._
import doobie._
import doobie.implicits._
import zio._
import zio.interop.catz._

object DoobieUniqueTest extends TestSuite {

  val runtime = new DefaultRuntime {}
  val xa = Transactor.fromDriverManager[Task]("org.sqlite.JDBC", "jdbc:sqlite:memory:demo.db", "", "")

  val tests = Tests {
    test("Find non-existing user") {
      val x = runtime.unsafeRun(createAndFailToFind())
      assert(x.isLeft)
    }
  }

  def createAndFailToFind(): UIO[Either[Throwable, (PasswordHash, String)]] =
    (createTable() *> findUser("nosuchuser")).either

  def createTable(): Task[Int] =
     sql"""create table if not exists passwd (
          |  username varchar primary key,
          |  hash varchar
          |  );
          |  """.stripMargin
       .update.run.transact(xa)

  def findUser(username: String): Task[(PasswordHash, String)] =
    for {
      hash <- sql"""select hash from passwd where username=$username"""
        .query[String]
        .unique
        .transact(xa)
      x <-  Task(upickle.default.read[(PasswordHash, String)](hash))
    } yield x
}

This fails with the following error:

X me.gmeiner.stoic.server.auth.DoobieUniqueTest.Find non-existing user 1827ms zio.FiberFailure: Fiber failed. An unchecked error was produced. doobie.util.invariant$UnexpectedEnd$: ResultSet exhausted; more rows expected. at doobie.util.invariant$UnexpectedEnd$.(invariant.scala) at doobie.hi.resultset$.$anonfun$getUnique$1(resultset.scala:195) at doobie.hi.resultset$.$anonfun$getUnique$1$adapted(resultset.scala:192) at cats.SemigroupalArityFunctions.$anonfun$map2$1(SemigroupalArityFunctions.scala:30) at cats.Monad.$anonfun$map$1(Monad.scala:16) at cats.free.Free.$anonfun$step$1(Free.scala:52) at cats.free.Free.step(Free.scala:53) at cats.free.Free.$anonfun$foldMap$1(Free.scala:153) at cats.data.KleisliFlatMap.$anonfun$tailRecM$2(Kleisli.scala:534) at zio.interop.CatsMonad.$anonfun$tailRecM$1(catsjvm.scala:252) at zio.ZIOFunctions.$anonfun$suspend$1(ZIO.scala:1444) at zio.internal.FiberContext.evaluateNow(FiberContext.scala:485) at zio.Runtime.unsafeRunAsync(Runtime.scala:94) at zio.Runtime.unsafeRunAsync$(Runtime.scala:80) at me.gmeiner.stoic.server.auth.DoobieUniqueTest$$anon$1.unsafeRunAsync(DoobieUniqueTest.scala:12) at zio.Runtime.unsafeRunSync(Runtime.scala:69) at zio.Runtime.unsafeRunSync$(Runtime.scala:66) at me.gmeiner.stoic.server.auth.DoobieUniqueTest$$anon$1.unsafeRunSync(DoobieUniqueTest.scala:12) at zio.Runtime.unsafeRun(Runtime.scala:58) at zio.Runtime.unsafeRun$(Runtime.scala:57) at me.gmeiner.stoic.server.auth.DoobieUniqueTest$$anon$1.unsafeRun(DoobieUniqueTest.scala:12) at me.gmeiner.stoic.server.auth.DoobieUniqueTest$.$anonfun$tests$2(DoobieUniqueTest.scala:17)

neko-kai commented 5 years ago

The problem is/was doobie AFAIK, before doobie used naked throw instead of raiseError in many places, see: https://github.com/tpolecat/doobie/pull/837 https://github.com/tpolecat/doobie/issues/836 https://github.com/tpolecat/doobie/issues/800

We sandbox all doobie calls in production codebase for that reason. Are you sure you're not affected simply by naked throws? Are you on a recent enough version of doobie, after the PR above was merged?

grmble commented 5 years ago

You are correct, the doobie version was too old. 0.5.x has the problem, it goes away with 0.7.0 or newer.

Sorry for the noise.

hollinwilkins commented 3 years ago

I think this issue has popped back up with doobie version 0.10.0. Is there a better workaround than using sandbox to extract the exception and move it into the error channel?

neko-kai commented 3 years ago

@hollinwilkins You can use .absorb method on ZIO to convert defect into a typed error