Fix for Corporate Proxies #874
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Author
|
@microsoft-github-policy-service agree |
millerds
requested changes
Dec 8, 2025
Contributor
millerds
left a comment
millerds
left a comment
There was a problem hiding this comment.
Thanks for doing this!
It's probably fine that it's all together. Some formatting things that should be corrected and someone else will have to take a look at it too.
src/app/helpers/helperMethods.ts
Outdated
| import { AttemptAwareConfig, hasProxy, addProxy, addLogger, addInterceptor } from "./requestHelpers.js"; | ||
|
|
||
| const zipFile = 'project.zip'; | ||
| const log = debug("genOffice").extend("helper") |
Contributor
There was a problem hiding this comment.
nit: Please add semicolon to end of the statement. This should be done in other places as well.
src/app/helpers/helperMethods.ts
Outdated
| const useProxyFirst = process.env.GENERATOR_OFFICE_USE_PROXY === "true"; | ||
| const projectTemplateZipFile = `${projectRepo}/archive/${projectBranch}.zip`; | ||
| return axios({ | ||
| log("Setting up config for %s",projectTemplateZipFile) |
Contributor
There was a problem hiding this comment.
nit: please a space between the ',' and next argument. Should be done in other statements as well.
Contributor
|
/azurepipelines run |
|
No pipelines are associated with this pull request. |
Contributor
|
/azurepipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Contributor
|
You will also need to pull a change I just added to master in order for the tests to pass. |
Merging updates to master
Nit updates for spacing
Contributor
Author
|
/azurepipelines run |
|
Commenter does not have sufficient privileges for PR 874 in repo OfficeDev/generator-office |
Contributor
|
/azurepipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
millerds
approved these changes
Dec 10, 2025
Contributor
|
Thanks again for the submission and work you did! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Do these changes impact User Experience? (e.g., how the user interacts with Yo Office and/or the files and folders the user sees in the project that Yo Office creates)
If Yes, briefly describe what is impacted.
Do these changes impact documentation? (e.g., a tutorial on https://learn.microsoft.com/en-us/office/dev/add-ins/overview/office-add-ins)
If Yes, briefly describe what is impacted.
Validation/testing performed:
Ran all tests in convert-to-single-host on both a computer requiring a proxy and a comnputer without a proxy. I also added tests to validate all my code with a fake proxy.
After test cases passed, I validated it worked with yo directly on both the proxied and non-proxied computers.
Platforms tested:
Overview
With the use of the requestHelpers.ts, I give the project the ability to identify and switch to a proxy server in the event of download failure. This update should resolve issue #872.
Why changes were made?
While axios could attempt to resolve the error, they have provided acceptable solutions for others to resolve on their own, as seen in the addInterceptor function in requestHelpers.ts. Axios updates alone were not enough because I was able to get all the tests to pass, but when run through the yo command line, it still failed because the default http agent was being overwritten as a BoundHttpsProxyAgent instead of a regular HttpsProxyAgent. I believe a yo dependency is modifying the Agent somehow, which might be the Microsoft usage tracker. Adding global-proxy allows you to bypass the BoundHttpsProxyAgent and switch to the proxy when necessary.
Changes breakdown
I updated the npm run scripts:
Updated the helperMethods.ts with
In index.ts, I ran into multiple issues where the files would download, but failed to properly filter and copy files (randomly in tests), so I added more logging and changed the order of operations. This may cause issues if run on machines with limited disk space.
In convert-to-single-host tests, I added a specific test for my proxy changes, replaced deprecated withPrompts with withAnswers, and added logging to all of the tests so you can see the performance changes that conveniently come with debug.
My apologies for not making these changes bite-sized. I am open to any suggestions and am willing to break this into multiple branches/pull requests.