Skip to content
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

How to handle errors in MS Graph SDK v5 #1558

Closed
LockTar opened this issue Nov 11, 2022 · 5 comments · Fixed by #1747
Closed

How to handle errors in MS Graph SDK v5 #1558

LockTar opened this issue Nov 11, 2022 · 5 comments · Fixed by #1747
Assignees

Comments

@LockTar
Copy link

LockTar commented Nov 11, 2022

Describe the bug
I'm searching for a teams app in the appcatalog. It can be that the app doesn't exist. This would result in an error not found. Following the error handling documentation, this error should be checked with in example: exception.IsMatch(GraphErrorCode.AccessDenied.ToString().
I assume that I would receive a GraphErrorCode.ItemNotFound but I get a NotFound and that is not in the GraphErrorCode list.

TeamsApp teamsApp;

try
{
    teamsApp = await client
        .AppCatalogs
        .TeamsApps[appId]
        .Request()
        .GetAsync();
}
catch (ServiceException ex) when (ex.IsMatch(GraphErrorCode.ItemNotFound.ToString()))
{
    logger.LogWarning($"App {appId} can't be found.");
    return;
}

To Reproduce
Steps to reproduce the behavior:

  1. Run above code with appid that doesn't exist.
  2. See error not coming in catch

Expected behavior
I would expect to catch the exception strongly typed.

@LockTar
Copy link
Author

LockTar commented Mar 16, 2023

@andrueastman Any update on this? How should we handle errors in the new v5 SDK?

There is a difference in documentation between the upgrade to v5 docs and the handling errors doc (same folder in the repository).

I think the handling errors doc is old and we should follow the upgrade error handling docs?

Do you have a simple example that checks if a user exists and if not, you can create a new user? Should that be in the catch?

@LockTar LockTar changed the title Request returns 'NotFound' instead of GraphErrorCode.ItemNotFound How to handle errors in MS Graph SDK v5 Mar 16, 2023
@LockTar
Copy link
Author

LockTar commented Mar 16, 2023

Is this in example the intended way to handle not found?

public static async Task AddGroupMemberAsync(GraphServiceClient graphClient, Group group, string memberToAdd)
{
    // Get user to add
    var user = await graphClient.Users[memberToAdd]
        .GetAsync(requestConfig =>
            requestConfig.QueryParameters.Select = new string[] { "id", "displayName" });

    try
    {
        var memberOfGroup = await graphClient
            .Groups[group.Id]
            .Members
            .GetAsync(requestConfig =>
            {
                requestConfig.QueryParameters.Filter = $"id eq '{memberToAdd}'";
            });

        Console.WriteLine($"User {user.Id} already member of group '{group.DisplayName}'");
    }
    catch (ODataError odataError) when (odataError.Error.Code.Equals("Request_ResourceNotFound"))
    {
        //Console.WriteLine(odataError.Error.Code);
        //Console.WriteLine(odataError.Error.Message);

        Console.WriteLine($"Add user {user.Id} as member to group '{group.DisplayName}'");
        ReferenceCreate referenceCreate = new ReferenceCreate();
        referenceCreate.OdataId = "https://graph.microsoft.com/v1.0/directoryObjects/" + user.Id;

        await graphClient.Groups[group.Id]
            .Members
            .Ref
            .PostAsync(referenceCreate);

        Console.WriteLine($"User {user.Id} added as member to group '{group.DisplayName}'");
    }
}

@andrueastman
Copy link
Member

That would be correct @LockTar

Another alternative would be to match the status code as failing to find a user would result in a 404 response code.

catch (ODataError odataError) when (odataError.ResponseStatusCode.Equals(404))

@LockTar
Copy link
Author

LockTar commented Mar 20, 2023

Hi @andrueastman,

Thank you for updating the docs but I don't think this should be closed already.

I see a few problems with this setup:

  1. Checking the error code is better to change to this:
catch (ODataError odataError) when (odataError.ResponseStatusCode.Equals(HttpStatusCode.NotFound))
{
        // Handle 404 status code
}
  1. For checking the Error and the Code they can be null like described here. Can they really be null? Should it not be changed to non nullable properties otherwise the described setup won't work.
  2. My original message above checking the ItemNotFound (most used I guess) in GraphErrorCode is wrong. You get a NotFound back. That GraphErrorCode doesn't exist. Please update line 54.
  3. The enum GraphErrorCode is defined in the Microsoft.Graph namespace. This gives issues if you have the production and beta package run next to each other (like I have). Changing the namespace of GraphErrorCode would fix this. Now you get this error:
    The type 'GraphErrorCode' exists in both 'Microsoft.Graph.Beta' and 'Microsoft.Graph'.

@andrueastman
Copy link
Member

Thanks for the follow up @LockTar

These should be resolved via
microsoftgraph/msgraph-beta-sdk-dotnet@6b73b83 and
c70662e

Where GraphErrorCode namespace is corrected in the beta libraries and the relevant documentation fixed.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants