UnknownErrorResponse would be more useful if it added details about the request

[epifab]

When UnknownErrorResponse is raised, I have no way to tell what request was it about. It'd be great if some details were included - the requested uri would be a great start

[yisraelU]

The correlation of all Responses, successful or otherwise to the Request is usually handled at a higher level, usually via logging This Response is provided in the context of errors predefined in Smithy, and the interpreter is unable to use a predefined error

[kubukoz]

I kinda agree but kinda not: errors should be caught and logged at a high level, like a global error handler. With the current shape of UnknownErrorResponse it's not possible, as all the context is lost between the http4s client call and smithy4s wrapping it.

I'd be :+1: on adding more ctx to that error

[yisraelU]

@kubukoz why wouldnt a logging middleware solve that problem

[kubukoz]

I guess you could do it with middleware... you could catch exceptions and enrich them with info from endpointHints. Still, that seems like something we should either document as a pattern, or we could just have some info in the error type.

I wouldn't do logging in such a middleware, unless it was also catching and handling the error somehow.

[kubukoz]

I take that back, I don't think you can do wrap these exceptions in middleware because they're not produced by the http4s client. I was trying something like this:

new ClientEndpointMiddleware.Simple[IO] {
  def prepareWithHints(
      serviceHints: Hints,
      endpointHints: Hints
  ): Client[IO] => Client[IO] =
    c =>
      Client(c.run(_).adaptError { case e =>
        WithContext(
          e,
          endpointHints
            .get(ShapeId)
            .getOrElse(sys.error("no shape id on endpoint"))
            .show
        )
      })

and it never hit that adaptError branch.

[yisraelU]

Thats not whatI was referring to , I am talking about what is used in every http application to correlate error responses to requests , like if you get a http 500 error, which usually just contains exception data , a standard out of the box logging solution is whats used for that

[kubukoz]

sure, you can log it immediately in a http4s client middleware, I'm just saying I consider it a better thing to:

1. let the exception propagate
2. catch, log and handle it on the top level, e.g. in your server error handler.

This mirrors the approach described here.

[yisraelU]

I hear , I guess simplest solution is to simply thread it through from inputToRequest(_)

[yisraelU]

I wish the http4s Client interface could be more flexible with what it allowed you to return

[Baccata]

Improving this will be binary breaking, so it's likely not gonna happen until the next minor version (0.18), which is gonna take a couple months at least.

However, in a few lines of code, you can add more details to the error about the high level operation that was is run. The code may be pretty scary because of the polymorphism but it's reasonably simple, conceptually speaking

  import smithy4s._
  import smithy4s.kinds.FunctorAlgebra
  
  // Instance of the generated service
  val helloWorldClient : HelloWorldService[IO] = ???
  
  case class EnhancedUnkownErrorResponse(service: ShapeId, operation: ShapeId, input: String, unkownError: UnknownErrorResponse) extends Throwable

  def enhanceUnknownError[Alg[_[_, _, _, _, _]], F[_]: MonadThrow](impl: FunctorAlgebra[Alg, F])(implicit service: Service[Alg]) : FunctorAlgebra[Alg, F] = {
    // Turn the existing instance into a generic polymorphic function 
    val original = service.toPolyFunction(impl)
    // Create a new polymorphic function of the same type, in order to intercept the input 
    val transformed = new service.FunctorInterpreter[F] {
      def apply[I, E, O, SI, SO](op: service.Operation[I, E, O, SI, SO]): F[O] = {
        // retrieving the input of the operation, as well as the associated endpoint instance which contains 
        // metadata related to the operation 
        val (input, endpoint) = service.endpoint(op)
        // executing the operation in the original client, and adapt the error with metadata 
        original(op).adaptErr {
          case e: UnknownErrorResponse => EnhancedUnkownErrorResponse(service.id, endpoint.id, input.toString, e)
        }
      }
    }
    // transform the created polymorphic function back into the generated interface 
    service.fromPolyFunction(transformed)
  }

  val enhanced : HelloWorldService[IO] = enhanceUnknownError(helloWorldClient)

Here's the same thing in monomorphic version, in case it helps readers conceptualise what's going on :

  def enhanceMonomorphic(impl: HelloWorldService[IO]): HelloWorldService[IO] = {
    val original : HelloWorldService.FunctorInterpreter[IO] = HelloWorldService.toPolyFunction(impl)
    val transformed = new HelloWorldService.FunctorInterpreter[IO] {
      def apply[I, E, O, SI, SO](op: HelloWorldServiceOperation[I, E, O, SI, SO]): IO[O] = {
        val (input, endpoint) = HelloWorldService.endpoint(op)
        original(op).adaptErr {
          case e: UnknownErrorResponse => EnhancedUnkownErrorResponse(HelloWorldService.id, endpoint.id, input.toString, e)
        }
      }
    }
    HelloWorldService.fromPolyFunction(transformed)
  }