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.

Sign up for GitHub

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

Jump to bottom

text/plain response body encoding silent failure #3188

Open
lemony312 opened this issue Oct 10, 2024 · 7 comments · May be fixed by #3204
Open

text/plain response body encoding silent failure #3188

lemony312 opened this issue Oct 10, 2024 · 7 comments · May be fixed by #3204
Labels
💎 Bounty bug Something isn't working

Comments

@lemony312
Copy link
Contributor

lemony312 commented Oct 10, 2024 •
edited by 987Nabil
Loading

Describe the bug
I am using the Endpoint definition to accept some body and return a case class.
I defined the schema and some json encoders.
If a user makes a request with Accept: plain/text the route will return a 500 with no body.
(I had to copy the endpoint code and add log statements to see what the actual error was and it showed that is TextBinary encoder failing - see screenshot)

To Reproduce
Steps to reproduce the behaviour:

  case class MyItem(id: String, name: String)
  object MyItem {
    implicit val schema: zio.schema.Schema[MyItem] = zio.schema.DeriveSchema.gen

    implicit val encoder: JsonEncoder[MyItem] = JsonCodec.jsonEncoder(schema)
    implicit val decoder: JsonDecoder[MyItem] = JsonCodec.jsonDecoder(schema)
  }

  val getEndpoint =
    Endpoint(
      RoutePattern.GET / "api" / "item" / PathCodec.string("itemId") ?? Doc.p("Get Item")
    ).out[MyItem]


  val getItemHandler = handler { (itemId: String) =>
    ZIO.succeed(MyItem(itemId, "name"))
  }
  val getItemRoute = getEndpoint.implementHandler(getItemHandler)

  val routes = Routes(getItemRoute)
  Server.serve(routes)

$ curl localhost:8080/api/item/id -H 'accept:text/plain'

This will return a 500 with no body (and NO LOGS) on the server.

if you explicity set text plain

    implicit val codec: zio.json.JsonCodec[MyItem] = JsonCodec.jsonCodec(schema)
    implicit val zioHttpContentCodec: HttpContentCodec[MyItem] = {
      val supportPlainText =
        HttpContentCodec(
          ListMap(
            MediaType.text.plain ->
              BinaryCodecWithSchema(
                zio.schema.codec.JsonCodec.zioJsonBinaryCodec[MyItem],
                schema
              )
          )
        )
      HttpContentCodec.fromSchema ++ supportPlainText
    }

then you can curl this way

as a side note, the client sent a list of accept content including application/json
Yet the server decided to pick the first from the list, fail silently, and not retry another content type.

Another note - if you define the route explicitly (not using endpoint) - it will not fail!
Expected behaviour
better: Either a server log or some information
best: Support text/plain automatically

Screenshots
encoder_failure
@lemony312 lemony312 added the bug Something isn't working label Oct 10, 2024
@tomorkenyi
Copy link

tomorkenyi commented Oct 16, 2024 •
edited
Loading

You are getting no errors because encoding into the body response is turned off, it can leak server details. ("Secure by default")

It can be set it though with ErrorResponseConfig introduced in this commit:
2c90dde#diff-c3252812e3130ed03f83725d8f8fe134295b999de4d4de33374072966250700c

So if you upgrade to at least RC10, you can add this as an aspect to your route, something like:
val getItemRoute = getEndpoint.implementHandler(getItemHandler).toRoutes @@ ErrorResponseConfig.debug

Check the documentation about the attributes here:
https://github.com/zio/zio-http/blob/main/docs/reference/response/response.md#failure-responses-with-details

On the other hand your code is trying to use a MyItem as a response out, which is not supported by the TextBinaryCodec, that is chosen by zio-http. This is based on your only explicit header’s mediatype (it is using the first passed or the default one: https://github.com/zio/zio-http/blob/main/zio-http/shared/src/main/scala/zio/http/codec/HttpContentCodec.scala#L113).

So if you are not sending the text/plain header or application/json is preceding that you are good to go, or if you want to use that text/plain header then specify a suitable output like a String.

@lemony312
Copy link
Contributor Author

lemony312 commented Oct 18, 2024 •
edited
Loading

Are you saying that by default zio-http will only support json encoding but will not support text/plain codec by default?
Why is it that if I do not use the Endpoint but use a direct implementation, that text/plain does not break the server? This seems like an inconsistency but I could misunderstanding.

It was not clear from the links you posted if there is a setting to at least print error logs to the console (and not in the response)

@987Nabil
Copy link
Contributor

@lemony312 Your direct impl returns a json for text/plain which you can ofc, but is not an expected behavior and therefore not the default. The default is returning a simple String which does not support multi field case classes

@jdegoes
Copy link
Member

jdegoes commented Nov 9, 2024

@987Nabil Since this is a codec error, can't we do better reporting without leaking details?

/bounty $250

@algora-pbc Algora PBC
Copy link

algora-pbc bot commented Nov 9, 2024 •
edited
Loading

💎 $250 bounty • ZIO

Steps to solve:

  1. Start working: Comment /attempt #3188 with your implementation plan
  2. Submit work: Create a pull request including /claim #3188 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to zio/zio-http!

Add a bounty • Share on socials

Attempt Started (GMT+0) Solution
🟢 @asr2003 Nov 9, 2024, 9:25:46 PM #3204

@asr2003
Copy link
Contributor

asr2003 commented Nov 9, 2024 •
edited by algora-pbc bot
Loading

/attempt #3188

Algora profile Completed bounties Tech Active attempts Options
@asr2003    3 ZIO bounties
+ 5 bounties from 3 projects
Rust, Scala,
Go & more
﹟350
 Cancel attempt

@asr2003 asr2003 linked a pull request Nov 9, 2024 that will close this issue
Open
@algora-pbc Algora PBC
Copy link

algora-pbc bot commented Nov 9, 2024

💡 @asr2003 submitted a pull request that claims the bounty. You can visit your bounty board to reward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
💎 Bounty bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement codec error handling asr2003/zio-http
5 participants
@jdegoes @tomorkenyi @987Nabil @lemony312 @asr2003