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

[Identity] Fixes to spawn a new instance on macOS #22214

Merged

Conversation

KarishmaGhiya
Copy link
Contributor

@KarishmaGhiya KarishmaGhiya commented Jun 10, 2022

Packages impacted by this PR

@azure/identity

Issues associated with this PR

#21726

Describe the problem that is addressed by this PR

On macOS, the default browser held open the connection, and refused to close the associated sockets unless the window was closed. The workaround with the setting newInstance forces a spawn of a new browser instance, not causing the application to hang or the socket to not close.

Note : This issue happens in Linux too and is not fixed by the PR. Filed an issue on the open dependency for linux

Are there test cases added in this PR? (If not, why?)

Provide a list of related PRs (if any)

PR by @mpodwysocki - #22195

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

@ghost ghost added the Azure.Identity label Jun 10, 2022
@KarishmaGhiya KarishmaGhiya changed the title Fixes to spawn a new instance on macOS [Identity] Fixes to spawn a new instance on macOS Jun 10, 2022
@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-identity

Copy link
Contributor

@mpodwysocki mpodwysocki left a comment

Choose a reason for hiding this comment

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

LGTM

@KarishmaGhiya KarishmaGhiya merged commit 1aae1cc into Azure:hotfix/identity_2.0.5 Jun 10, 2022
@@ -244,7 +244,8 @@ export class MsalOpenBrowser extends MsalNode {
const response = await this.publicApp!.getAuthCodeUrl(authCodeUrlParameters);

try {
await interactiveBrowserMockable.open(response, { wait: true });
// A new instance on macOS only which allows it to not hang, does not fix the issue on linux
await interactiveBrowserMockable.open(response, { wait: true, newInstance: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooh! Interesting 🤔 Thank you!

KarishmaGhiya added a commit to KarishmaGhiya/azure-sdk-for-js that referenced this pull request Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants