msgraph-beta-sdk-dotnet icon indicating copy to clipboard operation
msgraph-beta-sdk-dotnet copied to clipboard

Beta and official SDK should share the same ODataError type

Open olivermue opened this issue 1 year ago • 2 comments

In our current solution we mix the usage of the official and beta Graph API, cause we use a few features that are currently only available within the beta. Within v4 this mix was really cumbersome due to the shared namespace (to get this to run you need to use package aliases and extern alias). So switching to an individual namespace for the beta package really improves the usage of it.

While upgrading our solution to the v5 SDK I see that the ODataError type exists in both SDKs, which leads to some more complicated codes at a few places, cause we have to catch both exception types to ensure everything works as expected.

It would be great if the ODataError could be moved to the core library just like it had been done for the PageIterator to make the parallel usage of both libraries even more simple.

olivermue avatar Mar 09 '23 08:03 olivermue

Thanks for raising this @olivermue

The ODataError is a generated type that has a common base of type ApiException. Any chance catching the base type is helpful? Also, is there any chance you could share an example of the current usage to help us understand it better?

andrueastman avatar Mar 09 '23 17:03 andrueastman

The drawback of catching ApiException would be that we can't easily access the error details from the MainError Error property. At several places we are able to handle specific error cases like Request_ResourceNotFound or we search for specific substrings within ex.Error.Message like "originated within an external service" or "Permission being assigned already exists on the object".

As already stated we try to use the v1.0 API as much as possible, but a few places (especially DeviceManagement) still has some endpoints only available via the beta API. While we move from 4.x to 5.x I'm going to check at which places we can move from beta to v1.0 to make life easier, cause a lot of endpoints made it into v1.0 since we implemented them by using beta (e.g. deleted users, user password resets, education assignments).

I think the most part will boil down to be v1.0 and a few places maybe will be beta exclusive. The only really messy part will still be device management a.k.a. Intune, cause here we really mix up v1.0 and beta calls (by the way one reason for that could be that the official Intune admin center under https://endpoint.microsoft.com/ heavenly dependent on beta and mixes v1.0 into it. Just open it, check the network tab of your browser while clicking around. It is really funny that the beta endpoint doesn't get any customer support, but a Microsoft globally available product depends so much on this interface).

While further refactoring our code, my guess would be that really only a handful places do these mixes and we have to do such messy stuff like:

try
{
    myServiceWrapper.DoSomething();
}
catch (Microsoft.Graph.Models.ODataErrors.ODataError) when (ex.Error.Code == "FooBar")
{
    // Handle FooBar error
}
catch (Microsoft.Graph.Beta.Models.ODataErrors.ODataError) when (ex.Error.Code == "FooBar")
{
    // Handle FooBar error
}

The thing that probably could happen more often, if you have added both NuGet Packages to your project (or to any of your child or grand child projects) and then adding the wrong using namespace to your code like this:

public async Task DoSomething()
{
    try
    {
        var user = await serviceClient.Users[userId].GetAsync();
        await ChangeSomething(user);
    }
    catch(ODataError ex) when (ex.Error.Code == "Request_ResourceNotFound")
    {
        // User doesn't exist. We ignore the change.
    }
}

This given code is absolutely okay and looks fine, but if your header of the file looks like this:

using Microsoft.Graph;
using Microsoft.Graph.Models;
using Microsoft.Graph.Beta.Models.ODataErrors;

You are in trouble, because you catch the wrong type of exception. This can easily happen, especially when you add the ODataError the first time to a file within Visual Studio it will show up the available using statements in alphabetically order, which makes the beta namespace first and that is the potentially biggest problem: IntelliSense suggestions to pick using namespace

Hope, these thoughts help you to better understand the problems arised while upgrading to v5 SDK.

olivermue avatar Mar 10 '23 07:03 olivermue