Skip to content

Commit

Permalink
Merge pull request typelevel#209 from tpolecat/issue/129
Browse files Browse the repository at this point in the history
add handler for exception thrown in decoder
  • Loading branch information
tpolecat authored Jul 3, 2020
2 parents 3cd9211 + 64eb3e6 commit 2a116f3
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 4 deletions.
2 changes: 1 addition & 1 deletion modules/core/src/main/scala/Decoder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ object Decoder {
* An error indicating that decoding a value starting at column `offset` and spanning `length`
* columns failed with reason `error`.
*/
case class Error(offset: Int, length: Int, message: String)
case class Error(offset: Int, length: Int, message: String, cause: Option[Throwable] = None)

implicit val ApplyDecoder: Apply[Decoder] =
new Apply[Decoder] {
Expand Down
16 changes: 14 additions & 2 deletions modules/core/src/main/scala/exception/DecodeException.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ class DecodeException[F[_], A, B](
argumentsOrigin: Option[Origin],
encoder: Encoder[A],
rowDescription: TypedRowDescription,
cause: Option[Throwable] = None
) extends SkunkException(
sql = Some(sql),
message = "Decoding error.",
Expand Down Expand Up @@ -63,7 +64,7 @@ class DecodeException[F[_], A, B](
Text(t.tpe.name),
plain("->"),
green(op.getOrElse("NULL")),
if (n === error.offset) cyan(s"├── ${error.message}") else empty
if (n === error.offset) cyan(s"├── ${error.message}${cause.foldMap(_ => " (see below)")}") else empty
// TODO - make a fence for errors that span, like
// ├───── foo bar
//
Expand All @@ -74,9 +75,20 @@ class DecodeException[F[_], A, B](
s"""|The row in question returned the following values (truncated to $MaxValue chars).
|
| ${Text.grid(rowDescription.fields.zipWithIndex.zip(dataʹ).map(describe)).intercalate(plain("\n| ")).render}
|
|""".stripMargin

final protected def exception: String =
cause.foldMap { e =>
s"""|The decoder threw the following exception:
|
| ${e.getClass.getName}: ${e.getMessage}
| ${e.getStackTrace.mkString("\n ")}
|
|""".stripMargin
}

override def sections: List[String] =
super.sections :+ row
super.sections :+ row :+ exception

}
14 changes: 13 additions & 1 deletion modules/core/src/main/scala/net/protocol/Unroll.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import skunk.util.Origin
import skunk.data.TypedRowDescription
import natchez.Trace
import skunk.exception.PostgresErrorException
import scala.util.control.NonFatal

/**
* Superclass for `Query` and `Execute` sub-protocols, both of which need a way to accumulate
Expand Down Expand Up @@ -88,7 +89,17 @@ private[protocol] class Unroll[F[_]: MonadError[?[_], Throwable]: MessageSocket:
rows.flatMap { case (rows, bool) =>
Trace[F].span("decode") {
rows.traverse { data =>
decoder.decode(0, data) match {

// https://github.com/tpolecat/skunk/issues/129
// an exception thrown in a decoder will derail things here, so let's handle it.
// kind of lame because we lose the stacktrace
val result =
try decoder.decode(0, data)
catch {
case NonFatal(e) => Left(Decoder.Error(0, 0, e.getClass.getName, Some(e)))
}

result match {
case Right(a) => a.pure[F]
case Left(e) =>
send(Sync).whenA(extended) *> // if it's an extended query we need to resync
Expand All @@ -102,6 +113,7 @@ private[protocol] class Unroll[F[_]: MonadError[?[_], Throwable]: MessageSocket:
argsOrigin,
encoder,
rowDescription,
e.cause,
).raiseError[F, B]
}
} .map(_ ~ bool)
Expand Down
42 changes: 42 additions & 0 deletions modules/tests/src/test/scala/issue/Issue129.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright (c) 2018-2020 by Rob Norris
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

// Copyright (c) 2018-2020 by Rob Norris
// This software is licensed under the MIT License (MIT).
// For more information see LICENSE or https://opensource.org/licenses/MIT

package tests.issue

import skunk._
import skunk.codec.all._
import skunk.implicits._
import tests.SkunkTest
import skunk.exception.DecodeException
import cats.effect.IO

// https://github.com/tpolecat/skunk/issues/129
case object Issue129Test extends SkunkTest {

case class Country(name: String, code: String)
case class City(name: String, district: String)

val q: Query[Void, (Country, Option[City])] =
sql"""
SELECT c.name, c.code, k.name, k.district
FROM country c
LEFT OUTER JOIN city k
ON c.capital = k.id
ORDER BY c.code DESC
""".query(varchar ~ bpchar(3) ~ varchar.opt ~ varchar.opt)
.map {
case a ~ b ~ Some(c) ~ Some(d) => (Country(a, b), Some(City(c, d)))
// the incomplete match results in a `MatchError` which is expected, but it also
// reports a ProtocolError, which shouldn't happen.
}

sessionTest("issue/129") { s =>
s.execute(q).assertFailsWith[DecodeException[IO,_,_]](show = true).as("ok")
}

}

0 comments on commit 2a116f3

Please sign in to comment.