From fb6c2e379abc4414cc353d32f55424a1227bb128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Thu, 10 Nov 2022 19:45:46 +0100 Subject: [PATCH] Catch PayloadErrors in complete decoding in CodecAPI (#596) --- .../aws/internals/AwsUnaryEndpoint.scala | 3 +- modules/core/src/smithy4s/http/CodecAPI.scala | 8 +++-- modules/core/src/smithy4s/http/Partial.scala | 29 +++++++++++++++++-- .../SmithyHttp4sClientEndpoint.scala | 6 ++-- .../SmithyHttp4sServerEndpoint.scala | 3 +- .../http/json/JsonCodecApiTests.scala | 20 +++++++++++++ 6 files changed, 60 insertions(+), 9 deletions(-) diff --git a/modules/aws/src/smithy4s/aws/internals/AwsUnaryEndpoint.scala b/modules/aws/src/smithy4s/aws/internals/AwsUnaryEndpoint.scala index 5a859e13b..bf7db0f48 100644 --- a/modules/aws/src/smithy4s/aws/internals/AwsUnaryEndpoint.scala +++ b/modules/aws/src/smithy4s/aws/internals/AwsUnaryEndpoint.scala @@ -115,7 +115,8 @@ private[aws] class AwsUnaryEndpoint[F[_], Op[_, _, _, _, _], I, E, O, SI, SO]( metadataPartial <- metadataDecoder.decode(metadata).liftTo[F] bodyPartial <- codecAPI.decodeFromByteArrayPartial(codec, response.body).liftTo[F] - } yield metadataPartial.combine(bodyPartial) + decoded <- metadataPartial.combineCatch(bodyPartial).liftTo[F] + } yield decoded } } diff --git a/modules/core/src/smithy4s/http/CodecAPI.scala b/modules/core/src/smithy4s/http/CodecAPI.scala index 7a47173ac..8177d42f3 100644 --- a/modules/core/src/smithy4s/http/CodecAPI.scala +++ b/modules/core/src/smithy4s/http/CodecAPI.scala @@ -73,7 +73,9 @@ trait CodecAPI { codec: Codec[A], bytes: Array[Byte] ): Either[PayloadError, A] = - decodeFromByteArrayPartial(codec, bytes).map(_.complete(MMap.empty)) + decodeFromByteArrayPartial(codec, bytes).flatMap( + _.completeCatch(MMap.empty) + ) /** * Decodes partial data from a byte buffer, returning a function that is able @@ -94,7 +96,9 @@ trait CodecAPI { codec: Codec[A], bytes: ByteBuffer ): Either[PayloadError, A] = - decodeFromByteBufferPartial(codec, bytes).map(_.complete(MMap.empty)) + decodeFromByteBufferPartial(codec, bytes).flatMap( + _.completeCatch(MMap.empty) + ) /** * Writes data to a byte array. Field values bound diff --git a/modules/core/src/smithy4s/http/Partial.scala b/modules/core/src/smithy4s/http/Partial.scala index a96de9096..1cfa0f03f 100644 --- a/modules/core/src/smithy4s/http/Partial.scala +++ b/modules/core/src/smithy4s/http/Partial.scala @@ -17,11 +17,19 @@ package smithy4s.http import scala.collection.{Map => MMap} +import scala.annotation.nowarn final class MetadataPartial[A] private[smithy4s] ( private[smithy4s] val decoded: MMap[String, Any] ) { + @deprecated( + "0.16.8", + "This may throw a PayloadError. Use combineCatch instead." + ) final def combine(body: BodyPartial[A]): A = body.complete(decoded) + + final def combineCatch(body: BodyPartial[A]): Either[PayloadError, A] = + body.completeCatch(decoded) } object MetadataPartial { private[smithy4s] def apply[A](decoded: MMap[String, Any]) = @@ -31,13 +39,30 @@ object MetadataPartial { final class BodyPartial[A] private[smithy4s] ( private[smithy4s] val complete: MMap[String, Any] => A ) { - final def combine(metadata: MetadataPartial[A]): A = complete( - metadata.decoded + @deprecated( + "0.16.8", + "This may throw a PayloadError. Use combineCatch instead." ) + @nowarn("msg=method combine in class MetadataPartial is deprecated") + final def combine(metadata: MetadataPartial[A]): A = + metadata.combine(this) + + final def combineCatch( + metadata: MetadataPartial[A] + ): Either[PayloadError, A] = + metadata.combineCatch(this) final def map[B](f: A => B): BodyPartial[B] = new BodyPartial( complete andThen f ) + + @nowarn("msg=method combine in class BodyPartial is deprecated") + private[smithy4s] def completeCatch( + m: MMap[String, Any] + ): Either[PayloadError, A] = try Right(complete(m)) + catch { + case p: PayloadError => Left(p) + } } object BodyPartial { def apply[A](complete: MMap[String, Any] => A): BodyPartial[A] = diff --git a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sClientEndpoint.scala b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sClientEndpoint.scala index 4b83e8cf3..a6a9d5d74 100644 --- a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sClientEndpoint.scala +++ b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sClientEndpoint.scala @@ -183,18 +183,18 @@ private[smithy4s] class SmithyHttp4sClientEndpointImpl[F[_], Op[_, _, _, _, _], metadataDecoder: Metadata.PartialDecoder[T] )(implicit entityDecoder: EntityDecoder[F, BodyPartial[T]] - ): F[Either[MetadataError, T]] = { + ): F[Either[HttpContractError, T]] = { val headers = getHeaders(response) val metadata = Metadata(headers = headers, statusCode = Some(response.status.code)) metadataDecoder.total match { case Some(totalDecoder) => - totalDecoder.decode(metadata).pure[F] + totalDecoder.decode(metadata).pure[F].widen case None => for { metadataPartial <- metadataDecoder.decode(metadata).pure[F] bodyPartial <- response.as[BodyPartial[T]] - } yield metadataPartial.map(_.combine(bodyPartial)) + } yield metadataPartial.flatMap(_.combineCatch(bodyPartial)) } } } diff --git a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala index aa5b20c50..11c5938d2 100644 --- a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala +++ b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala @@ -135,7 +135,8 @@ private[smithy4s] class SmithyHttp4sServerEndpointImpl[F[_], Op[_, _, _, _, _], for { metadataPartial <- inputMetadataDecoder.decode(metadata).liftTo[F] bodyPartial <- request.as[BodyPartial[I]] - } yield metadataPartial.combine(bodyPartial) + decoded <- metadataPartial.combineCatch(bodyPartial).liftTo[F] + } yield decoded } } } diff --git a/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala b/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala index 86c230421..57033ec2b 100644 --- a/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala +++ b/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala @@ -25,4 +25,24 @@ class JsonCodecApiTests extends FunSuite { assertEquals(encodedString, """{"a":"test"}""") } + + test( + "struct codec with a required field should return a Left when the field is missing" + ) { + val schemaWithRequiredField = + Schema + .struct[String] + .apply( + Schema.string + .required[String]("a", identity) + )(identity) + + val capi = codecs(HintMask.empty) + + val codec = capi.compileCodec(schemaWithRequiredField) + + val decoded = capi.decodeFromByteArray(codec, """{}""".getBytes()) + + assert(decoded.isLeft) + } }