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

[Bug] Provide better HttpClient factory on .NET fwk (use the factory from .NET Core) #3546

Closed
bgavrilMS opened this issue Aug 2, 2022 · 4 comments · Fixed by #3626
Closed

[Bug] Provide better HttpClient factory on .NET fwk (use the factory from .NET Core) #3546

bgavrilMS opened this issue Aug 2, 2022 · 4 comments · Fixed by #3626
Assignees
@pmaytak
Labels
bug ICM This issue has a corresponding ICM, either for our team or another. P2
Milestone
4.46.2

Comments

@bgavrilMS
Copy link
Member

Looks like the .NET fwk factory is throwing a lot of timeout errors:

image

Suggested fix: use .NET Core's factory on .net
@bgavrilMS bgavrilMS added bug availability P2 ICM This issue has a corresponding ICM, either for our team or another. labels Aug 2, 2022
@jmprieur
Copy link
Contributor

jmprieur commented Aug 2, 2022

amazing!
Do we know if the .NET Core would throw less?

@bgavrilMS
Copy link
Member Author

bgavrilMS commented Aug 2, 2022 •
edited
Loading

yes, I think @pmaytak figure that one out - it's because on .NET Core we use a simple HTTP Factory (a static HttpClient) + Id.Web uses fancy scalable one. But on .NET Fwk we use a more complex HTTP Factory where we try to be smart about DNS refreshes etc.

@bgavrilMS
Copy link
Member Author

bgavrilMS commented Aug 2, 2022 •
edited
Loading

Btw, I updated the query to also print out occurrences of this error in Id.Web. And there aren't any. Well done @jennyf19 and @jmprieur for using a scalable solution in Id.Web

@bgavrilMS bgavrilMS moved this from Triage to Estimated/Committed in MSAL Customer Trust / QM Aug 17, 2022
@bgavrilMS bgavrilMS added this to the 4.47.0 milestone Aug 17, 2022
@bgavrilMS bgavrilMS changed the title [Bug] Replace HttpClient factory on .NET fwk with the one on .NET Core [Bug] Provide better HttpClient factory on .NET fwk (use the factory from .NET Core) Aug 18, 2022
@pmaytak pmaytak self-assigned this Aug 24, 2022
@pmaytak pmaytak moved this from Estimated/Committed to In Progress in MSAL Customer Trust / QM Aug 25, 2022
@pmaytak pmaytak moved this from In Progress to Waiting for Code Review in MSAL Customer Trust / QM Aug 25, 2022
@pmaytak pmaytak mentioned this issue Aug 25, 2022
Merged
Repository owner moved this from Waiting for Code Review to Fixed in MSAL Customer Trust / QM Aug 26, 2022
@pmaytak pmaytak modified the milestones: 4.47.0, 4.46.2 Aug 29, 2022
@pmaytak pmaytak mentioned this issue Nov 10, 2022
Closed
@pmaytak
Copy link
Contributor

pmaytak commented Nov 10, 2022

More details sent via email, but in general: there seems to be 2.2x fewer errors with the updated Http client on Msal.Desktop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pmaytak pmaytak

Labels
bug ICM This issue has a corresponding ICM, either for our team or another. P2
Projects
MSAL Customer Trust / QM
Archived in project
Milestone
4.46.2
Development

Successfully merging a pull request may close this issue.

Use common HTTP Client on .NET Fx. AzureAD/microsoft-authentication-library-for-dotnet
3 participants
@bgavrilMS @jmprieur @pmaytak