API Exception should have a ResponseStatusCode field and it should be populated by the request adapter

[CoolDadTx]

Currently the error mapping logic, from what I can tell, only supports throwing exceptions. In the core code, if an error mapping is found, then it is called. But irrelevant of what it does then an exception is still going to be thrown. This makes it impossible to handle errors that are expected. As an example almost every REST API returns 404 for resources that cannot be found. In the C# generator this can be easily handled by return null for the reference type that is generally used. This eliminates the need for throwing an exception and allows for client code to just check for null. Currently client code would need to capture the exception and then determine that it is a 404. This is both expensive and boilerplate code.

Suggestion: Make it possible to handle responses (error or otherwise) and return values based upon the status code instead of forcing an exception to be thrown in all cases.

[baywet]

This is a design decision we're unlikely to change at this point, I'll try to detail the reasons why here.
First according to the http spec the 4xx and 5xx status code classes are defined as error classes. I know there's been a long debate on whether 404 is an actual error or expected behavior but it makes more sense to have a consistent behavior across the error class. You wouldn't expect any other 5xx or 4xx not to throw? So why should this one be special?

Then if we follow the logic of returning null instead of an exception, how can a consumer tell the difference between successful with no body (204, etc..) and not found?

Lastly, the error body can contain additional information useful to the consumer that needs to be deserialized and returned somehow. Returning null wouldn't allow that.

The better approach to make easy to maintain code is to define a schema specific to 404 so the consumer can catch that type specifically.

I hope that shines a light on this design decision.

[CoolDadTx]

@baywet, Thank you for responding. I understand your decisions and I'm not actually asking that you change how Kiota handles this by default. What I'm asking for is an extensibility point, or more correctly the existing point, to allow for us to customize this behavior when we don't agree with it.

As for allowing a consumer to know the difference between a 204 and a 404 I would respond by asking - why would they care? If they ask for a resource and don't get anything back then I would personally consider it a mute point what the status code actually was. But, again, I'm not asking Kiota to change how it works, I'm just asking for an extensibility point to allow the author of the API to make customization changes to the generated code because the API dev knows it best.

Finally, in regards to whether I would expect any other 4xx to not throw, yes I actually do. 401 is special and I think Kiota tries to handle that directly. If I'm calling an API that is OAuth-based, for example, then I would expect the client code to handle negotiating the original token. If the server supports refresh tokens and my token expires then I would expect the client to silently reauthenticate without me having to do anything. I, the API user, already set up my credentials at app startup. The client is responsible for the behavior after that.

[baywet]

Thanks for the additional context.

Is this something you're looking to override on a per path basis? or something you'd like to override for a whole client regardless of the path? (trying to understand whether your scenario is cross cutting or request specific)

Difference between 204 and 404: the subtility matters as one is expected behavior, the other is not and mandates special handling.

401 is a bit of a special case, 429 that exceeded retry delay/attempts would be a better example in that regards.

token renewal: Kiota expects the authentication library to provide non-expired tokens and renew expired ones itself to avoid unnecessary roundtrips to the service. Which is something that Azure Identity does by default. You can still get 401s after that (continuous access evaluation policy being triggered, security event like password reset, etc...). Right now the support for that is partial mostly because Azure Identity doesn't support all scenarios, we're working with them to close the loop here.

[CoolDadTx]

@baywet I would see the error handling mostly at a per path level but I could see a client using the same rules for all paths. But if you support it at a path level then it is easy to "apply" it to a whole client I think. So as a minimum I'd say per path. I would almost see it as a post-request transform that could be run to adjust the results and so it could be something that is more general purpose then just error handling. But I'm thinking out loud.

[baywet]

Would you mind giving a try at implementing a middleware handler that'd rewrite the response (switch it from 404 to 204, wipe the body) based on paths requirements and see whether that meets your scenario?

[CoolDadTx]

@baywet yes a middleware handler would work here I believe. But that assumes that a call to Get<T> that gets a 204 doesn't try to deserialize an empty body anyway.

[baywet]

Correct. I thought the goal was to return null when running into an error?
Related: would having the status code as a field on the API Exception (the base class published in abstractions) be helpful at all? (assuming throwing an exception is ok)

[CoolDadTx]

Including the status code in the API exception would make it easier to handle this case. The only downside is the performance hit taken to throw the exception, then catch it to determine that nothing is wrong. But that is probably not a big deal in most cases.

[baywet]

Thanks for getting to the bottom of this discussion. I've heard back from the PM and architect and we're in agreements here.

This change would allow consumers to write that kind of code.

try { 
   var user = await client.Users["jane@contoso.com"].GetAsync()
} catch(ApiException ex) when ex.ResponseStatusCode == 404 {
    // do something for 404s
}

Which is much easier than having to inspect the message string.
Scheduling for implementation.

[baywet]

update: implemented, waiting for review and release here microsoft/kiota-http-dotnet#82

[darrelmiller]

@baywet What happened to being able to provide a custom response handler per request? We had that capability in past SDKs. I think the scenario raised is perfectly valid.

Did we decide that you could use ToRequestInformation and then pass that to SendAsync with a custom ErrorMapping?

[baywet]

No, we still maintain that the request adapter interface is meant to support the generated code, not for humans to use.

In case of cross cutting behavior, middleware handlers are the way to go.
In case of ad hoc behavior, the response handler parameter was moved to a request option to reduce the amount of code being generated.