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.

Sign up for GitHub

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

Jump to bottom

Improve interaction_in_progress reliability #4460

Merged
merged 8 commits into from Feb 3, 2022
Merged

Improve interaction_in_progress reliability #4460

merged 8 commits into from Feb 3, 2022

Conversation

tnorling
Copy link
Collaborator

  • Throws interaction_in_progress error at time of attempting to set the temporary cache value to minimize the gap between checking/setting cache and reduce occurence of race conditions
  • Moves this step to the top of interactive APIs so that events are not emitted when this is thrown
  • Fixes a recent regression that made the synchronous part of acquireTokenPopup asynchronous

@github-actions github-actions bot added the msal-browser Related to msal-browser package label Jan 29, 2022
@tnorling tnorling added this to the @azure/msal-browser@2.20.1 milestone Jan 29, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2022 •
edited

Codecov Report

Merging #4460 (ef0562d) into dev (76473e9) will decrease coverage by 0.02%.
The diff coverage is 95.65%.

Flag Coverage Δ *Carryforward flag
msal-angular 96.39% <ø> (ø)
msal-browser 87.42% <95.65%> (-0.10%) ⬇️
msal-common 85.32% <ø> (ø) Carriedforward from 76473e9
msal-core 82.65% <ø> (ø) Carriedforward from 76473e9
msal-node 82.97% <ø> (ø) Carriedforward from 76473e9
msal-node-extensions 76.03% <ø> (ø) Carriedforward from 76473e9
msal-react 92.70% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...rc/interaction_client/StandardInteractionClient.ts 97.97% <ø> (+0.78%) ⬆️
...al-browser/src/interaction_handler/PopupHandler.ts 84.00% <ø> (-0.62%) ⬇️
...browser/src/interaction_handler/RedirectHandler.ts 89.83% <ø> (-0.17%) ⬇️
...msal-browser/src/interaction_client/PopupClient.ts 96.73% <85.71%> (-3.27%) ⬇️
lib/msal-browser/src/app/ClientApplication.ts 96.39% <100.00%> (+0.10%) ⬆️
lib/msal-browser/src/cache/BrowserCacheManager.ts 93.05% <100.00%> (+0.01%) ⬆️
...l-browser/src/interaction_client/RedirectClient.ts 100.00% <100.00%> (ø)
lib/msal-browser/src/utils/PopupUtils.ts 92.50% <100.00%> (ø)

@musale musale mentioned this pull request Jan 31, 2022
Closed
@tnorling tnorling marked this pull request as ready for review January 31, 2022 22:46
pkanher617
pkanher617 approved these changes Feb 3, 2022
View reviewed changes
Copy link
Contributor

@pkanher617 pkanher617 left a comment

Choose a reason for hiding this comment

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

looks good, no other comments

@tnorling tnorling merged commit 729c4a1 into dev Feb 3, 2022
@tnorling tnorling deleted the improve_interaction_in_progress_reliability branch February 3, 2022 18:04
azure-pipelines bot pushed a commit that referenced this pull request Feb 7, 2022
@tnorling
Improve interaction_in_progress reliability (#4460)
4f75ff8 
* improve reliability of interaction_in_progress

* move preflight check

* fix conditional

* Update tests

* Change files
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkanher617 pkanher617 pkanher617 approved these changes

@peterzenz peterzenz peterzenz approved these changes

@jasonnutter jasonnutter Awaiting requested review from jasonnutter

Assignees
No one assigned
Labels
msal-browser Related to msal-browser package
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tnorling @codecov-commenter @pkanher617 @peterzenz