-
Notifications
You must be signed in to change notification settings - Fork 229
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
Exposing response headers in APIException #2524
Comments
@darrelmiller do we have a sign-off on that? |
Myself and @darrelmiller are actually discussing Response in a broader scope (available also outside of APIException). Let's wait for this discussion to close before we take on this one. |
@sebastienlevert @baywet @darrelmiller Any chance this will be included in next release, this is blocker for us to upgrade to Graph V5 and use new endpoints to support our clients. I will be happy to help in implementation if needed. |
We do have a design for it @sebastienlevert needs to detail here. Once the design is agreed upon we'll be able to start the work. The scope expanded a little as we're talking about giving access to response headers for both exceptions but also for successful requests. There's a low chance for this to slip from the upcoming to the next release at this point (so not 1.2 but 1.3) |
Based on our discussions, we are going to add all the HTTP response headers to the ApiException object. In the future if there is a need to disable this behaviour for performance reasons we will add a request option that allows developers to disable this feature. |
@andrueastman is this something you'd want to take on across our primary languages? If you're too busy I'll take it. Not suggesting Ronald as he is wrapping up the Go GA. |
No worries @baywet. Will take this on as its not big one. |
Closing this for now for the main lagnuages as below. Dotnet
Java Typescript Go
Issues for PHP, Swift,Ruby and Python created separately |
Thanks a lot for the great work across the board @andrueastman!!! |
Related to microsoftgraph/msgraph-sdk-dotnet#1755
ApiException currently exposes the status code for the failed request(#2216)
We can look into adding a Dictionary to
APIException
to hold the response headers when the error is thrown so as users can access header information that may be useful incase something goes wrong.Implementation wise, the change should be straightforward as the request adapters have direct access to the headers from the native response objects, similar to the status code in the
ThrowIfFailedResponse
method.C# - https://github.com/microsoft/kiota-http-dotnet/blob/2c6d3a411e4925f199f4da8dc10fd5861d69dec0/src/HttpClientRequestAdapter.cs#L344
Java - https://github.com/microsoft/kiota-java/blob/34b716a46ea713e9eff93fc067614c287451e42f/components/http/okHttp/src/main/java/com/microsoft/kiota/http/OkHttpRequestAdapter.java#L583
Typescript - https://github.com/microsoft/kiota-typescript/blob/81e25fa0669afff21af91131a77c275845f6a756/packages/http/fetch/src/fetchRequestAdapter.ts#L308
Go - https://github.com/microsoft/kiota-http-go/blob/main/nethttp_request_adapter.go#L790
The text was updated successfully, but these errors were encountered: