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

Add correlationId to errors thrown #3930

Merged
merged 16 commits into from Aug 10, 2021
Merged

Add correlationId to errors thrown #3930

merged 16 commits into from Aug 10, 2021

Conversation

tnorling
Copy link
Collaborator

@tnorling tnorling commented Aug 4, 2021

Changes include:

  • Adds setCorrelationId method to AuthError class
  • CorrelationId is added to the error before being thrown from all public APIs
  • CorrelationId is generated upfront when instantiating InteractionClient
  • CorrelationId is stored as a temporary cache item during the redirect flow
  • Logger is cloned with new CorrelationId when instantiating InteractionClient

Follow up PR will address successful responses

@github-actions github-actions bot added msal-browser Related to msal-browser package msal-common Related to msal-common package labels Aug 4, 2021
@tnorling tnorling added this to the @azure/msal-browser@2.16.2 milestone Aug 4, 2021
@github-actions github-actions bot added the msal-node Related to msal-node package label Aug 5, 2021
@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2021

Codecov Report

Merging #3930 (f03f955) into dev (314d4c0) will increase coverage by 2.28%.
The diff coverage is 90.41%.

Flag Coverage Δ
msal-angular 95.85% <ø> (ø)
msal-browser 87.98% <99.31%> (+0.04%) ⬆️
msal-common 85.00% <100.00%> (+0.01%) ⬆️
msal-core ?
msal-node 81.57% <21.05%> (-1.79%) ⬇️
msal-react 94.11% <ø> (ø)
Impacted Files Coverage Δ
lib/msal-node/src/client/ClientApplication.ts 69.52% <0.00%> (-5.74%) ⬇️
...ib/msal-node/src/client/PublicClientApplication.ts 77.77% <0.00%> (-9.73%) ⬇️
...l-node/src/client/ConfidentialClientApplication.ts 64.58% <44.44%> (-7.51%) ⬇️
...owser/src/interaction_client/SilentIframeClient.ts 97.14% <85.71%> (ø)
lib/msal-browser/src/app/ClientApplication.ts 98.34% <100.00%> (+<0.01%) ⬆️
lib/msal-browser/src/cache/BrowserCacheManager.ts 94.27% <100.00%> (+0.01%) ⬆️
...er/src/interaction_client/BaseInteractionClient.ts 100.00% <100.00%> (ø)
...msal-browser/src/interaction_client/PopupClient.ts 100.00% <100.00%> (ø)
...l-browser/src/interaction_client/RedirectClient.ts 100.00% <100.00%> (ø)
...rowser/src/interaction_client/SilentCacheClient.ts 92.00% <100.00%> (-0.60%) ⬇️
... and 48 more

Copy link
Member

@sameerag sameerag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets get the interactionHandler code cleaned up eventually. This looks good. Approved.

@github-actions github-actions bot added the samples Related to the samples apps for the library. label Aug 10, 2021
@github-actions github-actions bot removed the samples Related to the samples apps for the library. label Aug 10, 2021
@tnorling tnorling merged commit 51e8f1b into dev Aug 10, 2021
@tnorling tnorling deleted the error-correlationId branch August 10, 2021 18:05
@tnorling tnorling modified the milestones: @azure/msal-browser@2.16.2, @azure/msal-browser@2.17.0 Aug 18, 2021
@ghost
Copy link

ghost commented Sep 8, 2021

🎉@azure/msal-node@v1.3.1 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented Sep 8, 2021

🎉@azure/msal-browser@v2.17.0 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
msal-browser Related to msal-browser package msal-common Related to msal-common package msal-node Related to msal-node package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants