microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
928 stars 291 forks source link

Cannot read properties of undefined (reading 'userPrincipalName') in MGT PersonCard when using the Proxy provider #2694

Closed UmamaheshwariGolla closed 5 months ago

UmamaheshwariGolla commented 10 months ago

Cannot read properties of undefined (reading 'userPrincipalName') in MGT PersonCard We are using the MGT Proxy Provider and when the service is down I am getting the error. We are using the versions of mgt as mentioned below.

@microsoft/mgt-proxy-provider - 3.1.0 @microsoft/mgt-react - 3.1.0

Using the below Code Initialize Provider

 let provider: any = new ProxyProvider(
      `https://localhost:44301/dev/api/GraphToolkit/`,
      async () => {
        return {
          Authorization: `Bearer ${token}`,
        };
      }
    );

    Providers.globalProvider = provider;
    Providers.globalProvider.isIncrementalConsentDisabled = true;
export const Card = () => {
  const personDetails: microsoftgraph.User = {
    displayName: "displayname",
    mail: "mail",
    userPrincipalName: "mail",
  };

  const presence: Presence = {
    activity: "InActive",
    availability: "DoNotDisturb",
  };

  return (
    <Person
      userId={userID}
      personCardInteraction={PersonCardInteraction.hover}
      avatarSize={"auto"}
      personPresence={presence}
      showPresence={true}
    >
    </Person>
  );
};

Complete Error:

mgt-person-card.ts:943  Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'userPrincipalName')
    at MgtPersonCard.renderMessagingSection (mgt-person-card.ts:943:1)
    at MgtPersonCard.renderOverviewSection (mgt-person-card.ts:901:1)
    at MgtPersonCard.renderSectionNavigation (mgt-person-card.ts:849:1)
    at MgtPersonCard.renderExpandedDetails (mgt-person-card.ts:767:1)
    at MgtPersonCard.render (mgt-person-card.ts:554:1)
    at MgtPersonCard.update (lit-element.ts:160:1)
    at MgtPersonCard.update (templatedComponent.ts:99:1)
    at MgtPersonCard.performUpdate (reactive-element.ts:1328:1)
    at MgtPersonCard.scheduleUpdate (reactive-element.ts:1261:1)
    at MgtPersonCard._$Ej (reactive-element.ts:1233:1)

Expected behavior On click of the expand button it should not freeze/Crash the UI.

Screenshots image

We are using the People Picker as well, it does not have seem to have any crash when suing the component. But it keeps loading. image

Environment (please complete the following information):

sebastienlevert commented 10 months ago

When you say "when the service is down", do you mean that when you shutdown your proxy? This is expected as it requires a service on the other side to properly work. The Graph Toolkit is meant to be connected to Graph at all times.

What would be your expectations from an exception management?

UmamaheshwariGolla commented 10 months ago

Hi @sebastienlevert,

We are trying to prevent any crashes when the proxy service is unavailable. Currently, if the Proxy service goes down, only the Person card displays an error, while the People picker remains unaffected.

Is there any way to address this issue and avoid crashes or avoid errors ?

Please advise.

sebastienlevert commented 10 months ago

@gavinbarron, please share your thoughts!

gavinbarron commented 10 months ago

Offline state for the proxy which a ProxyProvider relies on is not a use case that has been well factored into the current state of our codebase. If the need is a resilient application and a Proxy is a must in your environment, then I'd seriously consider a highly available set up with geo-redundant hosting some form of traffic management solution to maximize uptime. Keeping the components rendering and not crashing the page is a good idea too, but without the ability to connect to Microsoft Graph then there are a lot of opportunities for client-side failure.

People picker might not look to be rendering an error state, but that constant load state is an error, just not one that is logging to the console or visually represented well.

This specific error is happening when then person card can't get the current user from the configured provider, in this case the ProxyProvider. At present resolving the current user always goes over the wire and will fail if it cannot connect to graph.

There's a number of ways that this specific issue could be approached.

As always, we're happy to accept contributions to improve our library.

@sebastienlevert we should add some work into the backlog to move the elements and components libraries to use strictNullChecks which would help identify areas like this where there are things that could potentially be nullish as is happening here.

mikewalker74 commented 10 months ago

I would hope there is a managed exception for this scenario it doesn't seem to have an event or ability to intercept the proxy being unreachable, some components manage this but others crash horribly, I would hope to have this be more robust and consistent tbh.

sebastienlevert commented 5 months ago

At the moment, this is not something we're likely to prioritize. That being said, the Graph Toolkit is an open source project and we'll be happy to support and review if you want to contribute to its codebase! Please let us know if you'd be happy to help and implement this capability. If not, it's absolutely fine, we'll go and close the issue and might re-open it later if we reconsider our priorities. Thanks!

microsoft-github-policy-service[bot] commented 5 months ago

This issue has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment.