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

No way to handle Invalid header value using HttpClientZioBackend #1823

Closed
vitaliihonta opened this issue May 11, 2023 · 1 comment · Fixed by #1856 or softwaremill/sttp-model#270
Closed

Comments

@vitaliihonta
Copy link

Reproducer

Requires "com.softwaremill.sttp.client3" %% "zio" % "3.8.15"
Scastie link to the same code.

import zio._
import sttp.client3._
import sttp.client3.httpclient.zio.HttpClientZioBackend

object Main extends ZIOAppDefault {

  val program = for {
    backend  <- HttpClientZioBackend.scoped()
    response <- basicRequest.get(uri"https://example.com")
       // the problem is here
      .header("X-Api-Key", "Я ЛЮБЛЮ БОРЩ")
      .response(asString)
      .send(backend)
  } yield ()

  val run = program
    .catchAllDefect { defect =>
       // It should have been an error?
       ZIO.consoleWith(_.printLine(s"Captured defect=$defect"))
    }
}

Actual console output:

Captured defect=java.lang.IllegalArgumentException: invalid header value: "Я ЛЮБЛЮ БОРЩ"

Expectation:

While non-latin header values are forbidden, it's confusing that such an error is a ZIO defect.
What is more confusing is that sttp doesn't provide a proper way to validate the header value.
Therefore, there is no way to overcome this error easily.

I'd expect there is a way to either validate the header or handle such errors as ZIO's errors, because the header value may be a user input (therefore, it may be incorrect).

Is there any reason why this invalid header value isn't handled as "normal" error, but as a defect?

@adamw
Copy link
Member

adamw commented Jun 6, 2023

I think there are two problems here:

  1. sttp model contains methods for creating the model classes w/ validation (as described here), however for some reason the header value is not validated (while it should, along the rules specified here). So we should fix this, so that .header(Header.unsafeApply("X-Api-Key", "Я ЛЮБЛЮ БОРЩ")) would behave as expected (throw a validation error)
  2. as for defects/errors, throwing an exception inside a .map in ZIO is translated to a defect (e.g. ZIO.succeed(1).map(_ => throw new IllegalArgumentException(""))). That's just how ZIO works ... but as we want to represent any exceptions that happen during request construction as errors, I think that all ZIO backends should override .send and additionaly do:
    .catchSomeDefect {
      case e: Exception => ZIO.fail(e)
    }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants