From b07896639f1cde4c3d914fa1036acc6733e94b24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Koz=C5=82owski?= Date: Tue, 22 Nov 2022 21:48:37 +0100 Subject: [PATCH] Catch metadata errors in operations without an Errorable (#622) --- build.sbt | 2 +- .../SmithyHttp4sServerEndpoint.scala | 12 ++++-- .../smithy4s/http4s/ProtocolBuilderSpec.scala | 16 ++++++++ .../http/json/JsonCodecApiTests.scala | 16 ++++++++ .../tests/PizzaAdminServiceImpl.scala | 5 +++ .../tests/src/smithy4s/tests/PizzaSpec.scala | 38 +++++++++++++++++++ sampleSpecs/pizza.smithy | 35 ++++++++++++++--- 7 files changed, 115 insertions(+), 9 deletions(-) diff --git a/build.sbt b/build.sbt index 0aea72df0..aaf1d7b6f 100644 --- a/build.sbt +++ b/build.sbt @@ -681,7 +681,7 @@ lazy val tests = projectMatrix Compile / smithySpecs := Seq( (ThisBuild / baseDirectory).value / "sampleSpecs" / "pizza.smithy", (ThisBuild / baseDirectory).value / "sampleSpecs" / "weather.smithy", - (ThisBuild / baseDirectory).value / "sampleSpecs" / "recursiveInput.smithy" + (ThisBuild / baseDirectory).value / "sampleSpecs" / "recursiveInput.smithy", ), moduleName := { if (virtualAxes.value.contains(CatsEffect2Axis)) diff --git a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala index 11c5938d2..ed371a046 100644 --- a/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala +++ b/modules/http4s/src/smithy4s/http4s/internals/SmithyHttp4sServerEndpoint.scala @@ -206,7 +206,8 @@ private[smithy4s] class SmithyHttp4sServerEndpointImpl[F[_], Op[_, _, _, _, _], endpoint.errorable match { case Some(errorable) => val processError: E => Response[F] = compileErrorable(errorable) - (_: Throwable) match { + + { case e: HttpContractError => Response[F](Status.BadRequest).withEntity(e).pure[F] case endpoint.Error((_, e)) => @@ -214,8 +215,13 @@ private[smithy4s] class SmithyHttp4sServerEndpointImpl[F[_], Op[_, _, _, _, _], case e: Throwable => F.raiseError(e) } - case None => - F.raiseError(_) + + case None => { + case e: HttpContractError => + Response[F](Status.BadRequest).withEntity(e).pure[F] + case e: Throwable => + F.raiseError(e) + } } } diff --git a/modules/http4s/test/src/smithy4s/http4s/ProtocolBuilderSpec.scala b/modules/http4s/test/src/smithy4s/http4s/ProtocolBuilderSpec.scala index ae30ef8d2..0aab68a0d 100644 --- a/modules/http4s/test/src/smithy4s/http4s/ProtocolBuilderSpec.scala +++ b/modules/http4s/test/src/smithy4s/http4s/ProtocolBuilderSpec.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2021-2022 Disney Streaming + * + * Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://disneystreaming.github.io/TOST-1.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package smithy4s.http4s import cats.effect.IO diff --git a/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala b/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala index 57033ec2b..f6bda84a7 100644 --- a/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala +++ b/modules/json/test/src/smithy4s/http/json/JsonCodecApiTests.scala @@ -1,3 +1,19 @@ +/* + * Copyright 2021-2022 Disney Streaming + * + * Licensed under the Tomorrow Open Source Technology License, Version 1.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://disneystreaming.github.io/TOST-1.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + package smithy4s.http.json import munit.FunSuite diff --git a/modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala b/modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala index af888562a..c1ee5fafb 100644 --- a/modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala +++ b/modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala @@ -109,4 +109,9 @@ class PizzaAdminServiceImpl(ref: Compat.Ref[IO, State]) def customCode(code: Int): IO[CustomCodeOutput] = IO.pure(CustomCodeOutput(if (code != 0) Some(code) else None)) + def echo( + pathParam: String, + body: EchoBody, + queryParam: Option[String] + ): IO[Unit] = IO.unit } diff --git a/modules/tests/src/smithy4s/tests/PizzaSpec.scala b/modules/tests/src/smithy4s/tests/PizzaSpec.scala index 2ba124c99..7916f419f 100644 --- a/modules/tests/src/smithy4s/tests/PizzaSpec.scala +++ b/modules/tests/src/smithy4s/tests/PizzaSpec.scala @@ -432,6 +432,44 @@ abstract class PizzaSpec } } + routerTest("path param failing refinement results in a BadRequest") { + (client, uri, log) => + client + .send[Unit]( + POST(uri = uri / "echo" / "too-short").withEntity(Json.obj()), + log + ) + .map(_._1) + .map(assert.eql(_, 400)) + } + + routerTest("query param failing refinement results in a BadRequest") { + (client, uri, log) => + client + .send[Unit]( + POST( + (uri / "echo" / "long-enough") + .withQueryParam("queryParam", "too-short") + ).withEntity(Json.obj()), + log + ) + .map(_._1) + .map(assert.eql(_, 400)) + } + + routerTest("body failing refinement results in a BadRequest") { + (client, uri, log) => + client + .send[Unit]( + POST( + uri / "echo" / "long-enough" + ).withEntity(Json.obj("data" -> Json.fromString("too-short"))), + log + ) + .map(_._1) + .map(assert.eql(_, 400)) + } + // note: these aren't really part of the pizza suite pureTest("Happy path: httpMatch") { diff --git a/sampleSpecs/pizza.smithy b/sampleSpecs/pizza.smithy index 09558e2d9..0260bc360 100644 --- a/sampleSpecs/pizza.smithy +++ b/sampleSpecs/pizza.smithy @@ -7,14 +7,13 @@ use smithy4s.api#simpleRestJson @simpleRestJson service PizzaAdminService { version: "1.0.0", - errors: [GenericServerError, GenericClientError], - operations: [AddMenuItem, GetMenu, Version, Health, HeaderEndpoint, RoundTrip, GetEnum, GetIntEnum, CustomCode] + operations: [AddMenuItem, GetMenu, Version, Health, HeaderEndpoint, RoundTrip, GetEnum, GetIntEnum, CustomCode, Echo] } @http(method: "POST", uri: "/restaurant/{restaurant}/menu/item", code: 201) operation AddMenuItem { input: AddMenuItemRequest, - errors: [PriceError], + errors: [PriceError, GenericServerError, GenericClientError], output: AddMenuItemResult } @@ -87,7 +86,7 @@ structure PriceError { @http(method: "GET", uri: "/restaurant/{restaurant}/menu", code: 200) operation GetMenu { input: GetMenuRequest, - errors: [NotFoundError, FallbackError], + errors: [NotFoundError, FallbackError, GenericClientError], output: GetMenuResult } @@ -288,4 +287,30 @@ structure CustomCodeInput { structure CustomCodeOutput { @httpResponseCode code: Integer -} \ No newline at end of file +} + + +@http(method: "POST", uri: "/echo/{pathParam}") +operation Echo { + input := { + @required + @httpLabel + @length(min: 10) + pathParam: String, + + @httpQuery("queryParam") + @length(min: 10) + queryParam: String, + + @httpPayload + @required + body: EchoBody + } + // this operation must NOT have any errors + errors: [] +} + +structure EchoBody { + @length(min: 10) + data: String +}