kiota icon indicating copy to clipboard operation
kiota copied to clipboard

Is there any way to get response body when status code is unsuccessful?

Open marcinjahn opened this issue 2 years ago • 7 comments

I generated a client with Kiota, based on an OpenAPI spec, which does not specify error codes, hence errorMapping is empty in Kiota-generated client. The ApiException that Kiota throws contains just the status code and response headers. Is there any way to read the actual response body (as string) without modifying the Kiota-generated client code?

marcinjahn avatar Feb 13 '24 07:02 marcinjahn

Hi @marcinjahn , Thanks for using kiota and for reaching out. This is something we've been talking about a couple of times internally and waiting to see if there was customer demand for it. On one hand we'd prefer the API descriptions to be improved so a model for the error is generated and the API consumer gets a better experience. We're also concerned about the performance aspects, especially if the consumer doesn't dispose of the body. On the other hand, we acknowledge that API producers and consumers are more often than not two different people, and that it's hard for consumers to funnel feedback to the producers. Maybe what we could do is add a content type and content fields to the API exception type. And only populate then if we were not able to deserialize to a generated type. I'd like @sebastienlevert our PM to weight in on the experience aspect. He is on vacations for the next few weeks though. In the meantime, you might be able to work this around by implementing your custom handler.

baywet avatar Feb 13 '24 14:02 baywet

Hi @baywet, exception containing the content would pretty much solve my issue. Do you mean implementing a custom DelegatingHandler, or something else, Kiota-specific?

marcinjahn avatar Feb 13 '24 14:02 marcinjahn

yes that what I meant

baywet avatar Feb 13 '24 15:02 baywet

While I like the suggestion, it would not be an experience that would be seamless across all exceptions. Could we use the work we are doing around UntypedJson to support the body of the exception?

sebastienlevert avatar Feb 27 '24 18:02 sebastienlevert

We could potentially decide to wrap the response into untyped json constructs when we don't have a mapping, and attach that to API error. Of course that assumes the response is JSON

baywet avatar Feb 27 '24 19:02 baywet

Transferring issue as part of https://github.com/microsoft/kiota-abstractions-dotnet/issues/238

andrueastman avatar Jul 09 '24 08:07 andrueastman

Moving this to main Repo to track this accross all languages for now.

andrueastman avatar Aug 15 '24 07:08 andrueastman

Following, this would be an essential feature, not all OpenAPI specs come with proper error mappings unfortunately

ntauth avatar Nov 07 '24 14:11 ntauth

This would solve an important pain point for us. +1.

merphidos avatar Nov 07 '24 15:11 merphidos

I updated the title of this issue to reflect a bit better that this is a request for a new feature

marcinjahn avatar Nov 11 '24 18:11 marcinjahn

Thank you for the additional information.

In that scenario, are you looking to get the response body only at development time? (while writing and testing the code) or also in production? (app has been deployed for a while, and you don't want to have to re-deploy to enable observability of the body)

baywet avatar Nov 11 '24 18:11 baywet

Both development and production. Many OpenAPI specs do not have error mappings, so it doesn’t matter much whether it’s dev or prod from my experience, but I may be missing something.

ntauth avatar Nov 11 '24 18:11 ntauth

From my perspective, I don't really understand what you mean by "only at development time". My idea was to be able to have some API that would allow you to read the body of error responses. Some example would be: let's say I have generated the client (using kiota) for some Web API that has some defined contract for error responses. The response body contains some domain-specific error codes. When I get HTTP error, I want to be able to retrieve the error code from the body of the response in order to properly react to it. Still, it assumes that the OpenAPI spec of the Web API does not have proper types for HTTP errors (let's assume I have no control over that, and that's just what I get).

marcinjahn avatar Nov 11 '24 18:11 marcinjahn

The reason why we haven't enabled observing response (or request) bodies "in production" is because it's really easy for anybody to shoot themselves in the foot and start logging data in production. Logging data in production can be terrible for security reasons, it also will be detrimental to performance, and increase application hosting costs.

By "only at development time" what I meant is that code could be behind conditional compilation, and only be available with debug builds (for languages that support it)

Observing all request/response bodies

That being said, we have received that request multiple times since the release of kiota. And we could design a similar middleware handler to the HeadersInspectionHandler, name it something like "DangerousBodyInspectionHandler". This would work in pair with an option and be used like that.

var observationOption = new DangerousBodyInspectionOption();
var result = await client.Foo.Bar.GetAsync(r => r.Options.Add(observationOption));
observationOption.ResponseBody;
// would be a stream the application would be responsible for disposing

This design allows to only copy/observe bodies when requested, lowering IOs.

Observing only error responses

The alternative only for errors is to add a field that would provide access to the response body stream. That field would only be populated if the response was not deserialized to a specific generated type. Which means the cost of errors is higher (memory, IOs), and poses limitations when wanting to observe successful responses or request bodies.

Let us know what you think about the two designs.

baywet avatar Nov 11 '24 18:11 baywet

  • I like the design of having this as an option, which will potentially give better performance to those who don't need that
  • the naming like DangerousBodyInspectionHandler is a bit exaggerated I think. It's not like people never had a way to read response bodies. I think it's up to the specific application to decide if this is dangerous or not. I see you assume the logging use-case, which probably makes sense, but it's not all of the use cases. For other use cases the response body might just be used in control flow of the app to decide what to do next
  • I think it makes sense to enable the responses "observability" only for errors, since in most cases I guess that would be the only reason to enable that. If it allows for better performance in non-error scenarios, even better.

marcinjahn avatar Nov 11 '24 19:11 marcinjahn

Thank you for the additional information.

@sebastienlevert what do you think of the handler design? What if it didn't have the Dangerous part in the name?

baywet avatar Nov 11 '24 19:11 baywet

Is there any temporary workaround we can use to access the response bodies while a proper fix is worked on?

ntauth avatar Nov 20 '24 16:11 ntauth

The only viable option at this point is to implement your own http client middleware handler.

baywet avatar Nov 25 '24 13:11 baywet

I like the option of the BodyInspectionOptions, the same way we're doing it for Headers. I don't see this as an issue and this would provide the required capability, while still being an opt-in. I don't think the Dangerous is required. Let's just make sure we don't alter the original response.

Having a field on all models with the body content seems overkill and will be more confusing than anything.

sebastienlevert avatar Nov 26 '24 19:11 sebastienlevert

Thanks for the input. Created https://github.com/microsoftgraph/msgraph-sdk-design/pull/116 in our design repo. Once it gets merged, I'll create the individual issues for the http libraries, and close this main issue.

baywet avatar Nov 27 '24 15:11 baywet

The design PR has been merged. I also created a bunch of implementation issues (list below) that I recommend anyone following this issue to go subscribe to. Closing to clear confusion. @marcinjahn is this something you'd like to submit a pull request for provided some guidance? If so please jump on the issue for the language you're interested in.

Here is the list of issues:

  • [ ] dotnet https://github.com/microsoft/kiota-dotnet/issues/482
  • [ ] go https://github.com/microsoft/kiota-http-go/issues/187
  • [ ] java https://github.com/microsoft/kiota-java/issues/1685
  • [ ] php https://github.com/microsoft/kiota-php/issues/33
  • [ ] python https://github.com/microsoft/kiota-python/issues/418
  • [ ] ruby https://github.com/microsoft/kiota-http-ruby/issues/42
  • [ ] swift https://github.com/microsoft/kiota-swift/issues/36
  • [ ] typescript https://github.com/microsoft/kiota-typescript/issues/1513

baywet avatar Dec 02 '24 14:12 baywet