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

Issue-1438: Fix upstream service error represented as 400 #1570

Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
27eb204
issue-1438- new error type for upstream service error
GaryAghedo Jul 24, 2024
3423781
Issue-1438: handle new error type and return 500
GaryAghedo Jul 24, 2024
cf612f7
issue-1438-formatted files
GaryAghedo Jul 24, 2024
2bc23a9
issue-1438: undo formatting errors
GaryAghedo Jul 26, 2024
9fb8b3b
issue-1438-add missing import
GaryAghedo Jul 26, 2024
0f9e5f1
issue-1438: implemented RawErrorResponse error object
GaryAghedo Aug 2, 2024
4830859
issue-1438: add more test cases
GaryAghedo Aug 6, 2024
5c3858c
issue-1438: update tests and code clean up
GaryAghedo Aug 6, 2024
b8dd7af
issue-1438: code clean up
GaryAghedo Aug 6, 2024
5807072
issue-1438: add missing headers
GaryAghedo Aug 8, 2024
f3ca663
issue-1438: format files
GaryAghedo Aug 8, 2024
66dc50a
refactor FailedDecodeAttempt
GaryAghedo Aug 29, 2024
5cddece
make the RawErrorResponse case class more binary compatibility-friendly
GaryAghedo Sep 5, 2024
d9b64e8
issue-1438: make all field private
GaryAghedo Sep 11, 2024
e2a1bb2
issue-1438: unapply method for RawResponse
GaryAghedo Sep 11, 2024
fdd4c19
issue-1438: make unapply private
GaryAghedo Sep 13, 2024
9ef3171
issue-1438: fixed tests
GaryAghedo Sep 13, 2024
68ecc1f
issue-1438: fixed tests
GaryAghedo Sep 13, 2024
e6ed025
issue-1438: make field public
GaryAghedo Sep 13, 2024
f46d5cc
Add changelog entry
kubukoz Sep 13, 2024
1e338a1
Expose fields directly
kubukoz Sep 13, 2024
f7636c1
Redo the docs a bit, add more private constructors
kubukoz Sep 13, 2024
79f5aca
Merge branch 'series/0.19' into issue-1438-Fix-upstream-service-error…
kubukoz Sep 13, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions modules/core/src/smithy4s/http/HttpContractError.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ package smithy4s
package http

import smithy4s.capability.MonadThrowLike
import smithy4s.codecs.PayloadError
import smithy4s.codecs.PayloadPath
import smithy4s.codecs.{PayloadError, PayloadPath}
import smithy4s.kinds.PolyFunction
import smithy4s.schema.Schema._
import smithy4s.schema._
Expand Down
35 changes: 27 additions & 8 deletions modules/core/src/smithy4s/http/HttpResponse.scala
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,9 @@ object HttpResponse {
* Creates a response decoder that dispatches the response
* to a given decoder, based on some discriminator.
*/
private def discriminating[F[_], Body, Discriminator, E](
discriminate: HttpResponse[Body] => F[Discriminator],
select: Discriminator => Option[Decoder[F, Body, E]],
private def discriminating[F[_], Body, E](
discriminate: HttpResponse[Body] => F[HttpDiscriminator],
select: HttpDiscriminator => Option[Decoder[F, Body, E]],
toStrict: Body => F[(Body, Blob)]
)(implicit F: MonadThrowLike[F]): Decoder[F, Body, E] =
new Decoder[F, Body, E] {
Expand All @@ -239,13 +239,32 @@ object HttpResponse {
val strictResponse = response.copy(body = strictBody)
F.flatMap(discriminate(strictResponse)) { discriminator =>
select(discriminator) match {
case Some(decoder) => decoder.decode(strictResponse)
case Some(decoder) =>
F.handleErrorWith(decoder.decode(strictResponse)) {
case error: HttpContractError =>
F.raiseError(
RawErrorResponse(
response.statusCode,
response.headers,
bodyBlob.toUTF8String,
FailedDecodeAttempt.DecodingFailure(
discriminator = discriminator,
contractError = error
)
)
)
case otherError => F.raiseError(otherError)
}
case None =>
F.raiseError(
smithy4s.http.UnknownErrorResponse(
response.statusCode,
response.headers,
bodyBlob.toUTF8String
smithy4s.http.RawErrorResponse(
code = response.statusCode,
headers = response.headers,
body = bodyBlob.toUTF8String,
failedDecodeAttempt =
FailedDecodeAttempt.UnrecognisedDiscriminator(
discriminator
)
)
)
}
Expand Down
1 change: 1 addition & 0 deletions modules/core/src/smithy4s/http/HttpUnaryServerCodecs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ object HttpUnaryServerCodecs {
def encodeError(e: E) = F.map(base)(errorW.write(_, e))
def httpContractErrorEncoder(e: HttpContractError) =
F.map(base)(httpContractErrorWriters.write(_, e).withStatusCode(400))

def throwableEncoders(throwable: Throwable): F[HttpResponse[Blob]] =
throwable match {
case e: HttpContractError => httpContractErrorEncoder(e)
Expand Down
121 changes: 121 additions & 0 deletions modules/core/src/smithy4s/http/RawErrorResponse.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
/*
* Copyright 2021-2024 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

case class RawErrorResponse(
GaryAghedo marked this conversation as resolved.
Show resolved Hide resolved
GaryAghedo marked this conversation as resolved.
Show resolved Hide resolved
private val codeField: Int,
private val headersField: Map[CaseInsensitive, Seq[String]],
private val bodyField: String,
private val failedDecodeAttemptField: FailedDecodeAttempt
GaryAghedo marked this conversation as resolved.
Show resolved Hide resolved
) extends Throwable {

def code: Int = codeField
def headers: Map[CaseInsensitive, Seq[String]] = headersField
def body: String = bodyField
def failedDecodeAttempt: FailedDecodeAttempt = failedDecodeAttemptField
GaryAghedo marked this conversation as resolved.
Show resolved Hide resolved

def withCode(newCode: Int): RawErrorResponse =
new RawErrorResponse(
newCode,
headersField,
bodyField,
failedDecodeAttemptField
)

def withHeaders(
newHeaders: Map[CaseInsensitive, Seq[String]]
): RawErrorResponse =
new RawErrorResponse(
codeField,
newHeaders,
bodyField,
failedDecodeAttemptField
)

def withBody(newBody: String): RawErrorResponse =
new RawErrorResponse(
codeField,
headersField,
newBody,
failedDecodeAttemptField
)

def withFailedDecodeAttempt(
newFailedDecodeAttempt: FailedDecodeAttempt
): RawErrorResponse =
new RawErrorResponse(
codeField,
headersField,
bodyField,
newFailedDecodeAttempt
)

override def getMessage(): String = {
val baseMessage = s"status $code, headers: $headers, body:\n$body"
baseMessage + s"""
|FailedDecodeAttempt:
| ${failedDecodeAttempt.getMessage}
""".stripMargin
}

override def getCause: Throwable = failedDecodeAttempt
}

object RawErrorResponse {
def apply(
code: Int,
headers: Map[CaseInsensitive, Seq[String]],
body: String,
failedDecodeAttempt: FailedDecodeAttempt
): RawErrorResponse =
new RawErrorResponse(code, headers, body, failedDecodeAttempt)

def unapply(response: RawErrorResponse): Option[
GaryAghedo marked this conversation as resolved.
Show resolved Hide resolved
(Int, Map[CaseInsensitive, Seq[String]], String, FailedDecodeAttempt)
] =
Some(
(
response.code,
response.headers,
response.body,
response.failedDecodeAttempt
)
)

}

sealed trait FailedDecodeAttempt extends Throwable {
def discriminator: HttpDiscriminator
def getMessage: String
}

object FailedDecodeAttempt {

case class UnrecognisedDiscriminator(discriminator: HttpDiscriminator)
extends FailedDecodeAttempt {
override def getMessage: String =
s"Unrecognised discriminator: $discriminator"
}

case class DecodingFailure(
discriminator: HttpDiscriminator,
contractError: HttpContractError
) extends FailedDecodeAttempt {
override def getMessage: String =
s"Decoding failed for discriminator: $discriminator with error: ${contractError.getMessage}"
}
}
23 changes: 22 additions & 1 deletion modules/http4s/test/src/smithy4s/http4s/Http4sPizzaSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ package smithy4s.http4s
import cats.effect.IO
import cats.effect.Resource
import org.http4s.Uri
import org.http4s.client.Client
import org.http4s.Response
import org.http4s.implicits._
import org.http4s.client.Client
import smithy4s.example.PizzaAdminService

object Http4sPizzaSpec extends smithy4s.tests.PizzaSpec {
Expand All @@ -40,4 +41,24 @@ object Http4sPizzaSpec extends smithy4s.tests.PizzaSpec {

}

def runServerWithClient(
pizzaService: PizzaAdminService[IO],
clientResponse: Response[IO]
): Resource[IO, Res] = {
SimpleRestJsonBuilder
.routes(
SimpleRestJsonBuilder(PizzaAdminService)
.client[IO](Client(_ => Resource.pure(clientResponse)))
.make
.toTry
.get
)
.resource
.map { httpRoutes =>
val client = Client.fromHttpApp(httpRoutes.orNotFound)
val uri = Uri.unsafeFromString("http://localhost")
(client, uri)
}
}

}
5 changes: 2 additions & 3 deletions modules/tests/src/smithy4s/tests/PizzaAdminServiceImpl.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,14 @@
package smithy4s.tests

import cats.effect._
import cats.effect.std.UUIDGen
import cats.implicits._
import smithy4s.Timestamp
import smithy4s.example._
import smithy4s.tests.PizzaAdminServiceImpl._

import java.util.UUID

import PizzaAdminServiceImpl._
import cats.effect.std.UUIDGen

object PizzaAdminServiceImpl {
case class Item(food: Food, price: Float, addedAt: Timestamp)
case class State(restaurants: Map[String, Restaurant])
Expand Down
109 changes: 87 additions & 22 deletions modules/tests/src/smithy4s/tests/PizzaClientSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ import smithy4s.example._
import smithy4s.Timestamp
import weaver._
import smithy4s.http.CaseInsensitive
import smithy4s.http.UnknownErrorResponse
import smithy4s.http.RawErrorResponse
import smithy4s.http.FailedDecodeAttempt
import smithy4s.http.HttpPayloadError
import smithy4s.http.HttpDiscriminator

abstract class PizzaClientSpec extends IOSuite {

Expand Down Expand Up @@ -111,29 +114,67 @@ abstract class PizzaClientSpec extends IOSuite {
GenericClientError("generic error message for 418")
)

clientTestForError(
"Handle error w/o a discriminator header nor a unique status code",
Response(status = Status.ProxyAuthenticationRequired)
.withEntity(
Json.obj("message" -> Json.fromString("generic client error message"))
),
unknownResponse(
407,
Map("Content-Length" -> "42", "Content-Type" -> "application/json"),
"""{"message":"generic client error message"}"""
)
clientAssertError[RawErrorResponse](
"Handle error with a discriminator but can't be decoded",
Response[IO](status = Status.NotFound)
.withEntity("malformed body")
.withHeaders(Header.Raw(CIString("X-Error-Type"), "NotFoundError")),
error => {
error match {
case RawErrorResponse(
code,
headers,
body,
FailedDecodeAttempt.DecodingFailure(
discriminator,
HttpPayloadError(path, expected, _)
)
) =>
expect(code == 404) &&
expect(
headers == Map(
CaseInsensitive("X-Error-Type") -> List("NotFoundError")
)
) &&
expect(body == "malformed body") &&
expect(
discriminator == HttpDiscriminator.NameOnly("NotFoundError")
) &&
expect(path == smithy4s.codecs.PayloadPath(List())) &&
expect(expected == "object")
case _ => failure("Unexpected error type or values")
}
}
)

private def unknownResponse(
code: Int,
headers: Map[String, String],
body: String
): UnknownErrorResponse =
UnknownErrorResponse(
kubukoz marked this conversation as resolved.
Show resolved Hide resolved
code,
headers.map { case (k, v) => CaseInsensitive(k) -> List(v) },
body
)
clientAssertError[RawErrorResponse](
"Handle malformed error response with no discriminator",
Response(status = Status.InternalServerError)
.withEntity("goodbye world"),
error => {
error match {
case RawErrorResponse(
code,
headers,
body,
FailedDecodeAttempt.UnrecognisedDiscriminator(discriminator)
) =>
expect(code == 500) &&
expect(
headers == Map(
CaseInsensitive("Content-Length") -> List("13"),
CaseInsensitive("Content-Type") -> List(
"text/plain; charset=UTF-8"
)
)
) &&
expect(body == "goodbye world") &&
expect(discriminator == HttpDiscriminator.StatusCode(500))
case _ => failure("Unexpected error type or values")
}

}
)

clientTest("Headers are case insensitive") { (client, backend, log) =>
for {
Expand Down Expand Up @@ -208,6 +249,30 @@ abstract class PizzaClientSpec extends IOSuite {
}
}

def clientAssertError[E](
name: String,
response: Response[IO],
assert: E => Expectations
)(implicit
loc: SourceLocation,
ct: scala.reflect.ClassTag[E]
) = {
clientTest(name) { (client, backend, log) =>
for {
_ <- backend.prepResponse(name, response)
maybeResult <- client.getMenu(name).attempt

} yield maybeResult match {
case Right(_) => failure("expected failure")
case Left(error: E) =>
assert(error)
case Left(error) =>
failure(s"Error of unexpected type: $error")
}
}

}

def clientTest(name: TestName)(
f: (
PizzaAdminService[IO],
Expand Down
Loading