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

Upgrade http(s)-proxy-agent dependent npm packages #22306

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

cheenamalhotra
Copy link
Member

@cheenamalhotra cheenamalhotra commented Mar 12, 2023

This is an attempt to address #22227 as the old version of http-proxy-agent and https-proxy-agent packages seem to conflict with MSAL and cause issues with HTTPS calls (for both MSAL and ADAL).

MSAL depends on the updated packages and gets conflicted with the older versions of http packages if present in the package list. VSCode-MSSQL extension also gets impacted with these packages, and a similar issue was resolved there too recently.
Repro is available here: https://github.com/cheenamalhotra/azurewithmsal/tree/failing (ignore lint errors) (Even though it doesn't fail with the same 'certificate is expired' error - it indicates the https-proxy-agent old version package interferes with MSAL calls and is capable of causing issues when communicating with Azure endpoints)

The VSCode-MSSQL extension's RC version (using updated packages only) does not seem impacted by this issue (#22227 (comment))

Notable updates in this PR:

P.S. I did not implement our own HttpClient for proxy integration as of now, as that doesn't seem to have impact on resolving the error. The old npm packages are the root cause of conflicts as also reproduced in repro.

@coveralls
Copy link

coveralls commented Mar 12, 2023

Pull Request Test Coverage Report for Build 4407829157

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.004%) to 41.948%

Files with Coverage Reduction New Missed Lines %
extensions/machine-learning/src/common/processService.ts 2 81.48%
Totals Coverage Status
Change from base Build 4389036439: 0.004%
Covered Lines: 28805
Relevant Lines: 63751

💛 - Coveralls

@cheenamalhotra cheenamalhotra merged commit 8539b63 into main Mar 13, 2023
@cheenamalhotra cheenamalhotra deleted the cheena/npm-updates branch March 13, 2023 18:06
@alanrenmsft alanrenmsft mentioned this pull request Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants