softwaremill / tapir

Rapid development of self-documenting APIs
https://tapir.softwaremill.com
Apache License 2.0
1.36k stars 419 forks source link

Add possibility to access raw body when executing client call #2114

Open matwojcik opened 2 years ago

matwojcik commented 2 years ago

I'd like to access raw response body when my client call fails, so e.g. I can log it.

Right now if I have an endpoint: Endpoint[Unit,Unit,Nothing,Unit,Any] and do SttpClientInterpreter(SttpClientOptions.default).toRequest(endpoint, uri)(wsToPipe)(params).send(backend) I receive Response[DecodeResult[Either[E, O]]]. If this is unmapped error like 500, then we get Error(,java.lang.IllegalArgumentException: Cannot decode from: , request: GET http://localhost) which doesn't bring much value.

Usually I'd like to have the raw body in hand in such cases and log it.

I can see that in sttp there was similar issue https://github.com/softwaremill/sttp/issues/190 which was implemented with asBoth, however in tapir we don't have control over it (at least I can't see such option).

At the same time I wouldn't like to change my original endpoint to have raw String in response.

adamw commented 2 years ago

And if you do sth like:

val req = SttpClientInterpreter(SttpClientOptions.default).toRequest(endpoint, uri)(wsToPipe)(params)

req.response(asBoth(req.responseAs, asString)) 

?

matwojcik commented 2 years ago

It might be a good lead, thanks.

I can see that it cannot be applied to stream or websockets - which probably is justified with the way it is implemented - but in fact the string could be gathered first, and then interpreted as JSON without double consuming from Stream?

However I have a feeling it returns None if the output of Endpoint is Unit, because it fails when using asBoth and returns none if asBothOption:

(Error(,java.lang.IllegalArgumentException: Cannot decode from: , request: GET http://localhost),None)

Oh and it works well if params are swapped:

req.response(asBoth(asString, req.response))

Do you know why it matters?

adamw commented 2 years ago

I can see that it cannot be applied to stream or websockets

Yes, streams are inherently non-replayable, and for websockets it would be hard to define a body :)

in fact the string could be gathered first, and then interpreted as JSON without double consuming from Stream?

I'm not sure what you mean?

Do you know why it matters?

Interesting! No idea, would need debugging :). In theory should work both ways ...

matwojcik commented 2 years ago

@adamw BTW this behaviour is kind of misleading - Endpoint which has Unit as an output - when e.g. status is 500 then the cause of exception java.lang.IllegalArgumentException: Cannot decode from: is the following:

[E] Caused by: java.lang.IllegalArgumentException: Cannot convert a void output to a value!
[E]     at sttp.tapir.client.ClientOutputParams.apply(ClientOutputParams.scala:56)
[E]     at sttp.tapir.client.sttp.EndpointToSttpClient.$anonfun$toSttpRequest$6(EndpointToSttpClient.scala:50)

because it fails into:

      case EndpointOutput.Void() => DecodeResult.Error("", new IllegalArgumentException("Cannot convert a void output to a value!"))

This is completely misleading and makes debugging harder. I would expect rather something that states that status code is not supported by the endpoint.

Unfortunately I don't have much time to deeply look into it, but I though I'll mention it anyway as tapir is just before 1.0.0, which would make some breaking changes (if needed for fixing this) impossible for a time being ;)

adamw commented 2 years ago

@matwojcik Thanks for mentioning that although 1.0 only stabilised core, with the client interpreters we'll have some flexibility still (though the user-facing API shouldn't change much).

Anyway ... not sure yet what we could do here. If you have an endpoint where the body is ignored (as here, I think you've got an infallibleEndpoint as your basis) after translation to sttp-client request, the specification will say to ignore the body in case of a request - and that's what is happening. And the excepiton is because you are trying to decode the response, even though you specified that errors are not possible.

Maybe we could make the interpereter more intelligent ... to always add a fallback mapping for the "other" status codes (not defined in the endpoint), which would read the body as a string & return an appropriate decode result.