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

Updated how Graph service exceptions are handling in controls #142

Merged
merged 2 commits into from
Aug 6, 2021

Conversation

shweaver-MSFT
Copy link
Member

@shweaver-MSFT shweaver-MSFT commented Aug 6, 2021

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

When the GraphServiceClient fails to make a request for whatever reason and throws an exception, in most cases we eat the exception and move on. This makes it difficult to understand WHY a request has failed and whether the exception encountered is part of normal logical flow (e.g. 404 - Not Found), or actually unexpected.

What is the new behavior?

In the new behavior, Graph client extension methods no longer eat exceptions, so any GraphServiceClient exceptions will bubble up to the caller.

The controls have also been updated to handle any expected exceptions that may occur when using the GraphServiceClient to make requests, and rethrow any unexpected ones. This increases stability for controls like PersonView, which is frequently used to represent entities that do not support a profile photo, such as conference rooms.

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Sample in sample app has been added / updated (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Header has been added to all new source files (run build/UpdateHeaders.bat)
  • Contains NO breaking changes

Other information

I also tested the WithScopes method to see if issue #8 still exists, and it appears that incremental consent is not working at the moment. Perhaps there is some additional configuration required in MsalProvider to get it going, but I'm not sure what that is. WindowsProvider is based on WAM apis that do not support incremental consent quite yet, so it is not expected to work there.

So the current behavior is that WithScopes seemingly does nothing. It doesn't show a prompt for additional consent, and will throw a 403 when calling for resources without the pre-requisite consent.

Based on this, we should investigate how incremental consent is expected to work with MSAL for .NET and see if we can get that working.

@ghost
Copy link

ghost commented Aug 6, 2021

Thanks shweaver-MSFT for opening a Pull Request! The reviewers will test the PR and highlight if there is any merge conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌

@shweaver-MSFT shweaver-MSFT merged commit ad7efbd into main Aug 6, 2021
@shweaver-MSFT shweaver-MSFT deleted the shweaver/graph-ex-handling branch August 6, 2021 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant