fix(stylua-npm-bin): adjust axios config to work with proxy env variables

[antoineauger]

Hello,

First of all, thanks for providing this code formatter on so many distribution channels 🙏🏻

We wanted to pull the wrapper from npm so we added it to our yarn dependencies.

However, since we are behind a corporate proxy, the install just failed. More precisely, axios was not correctly resolving the GitHub redirects in addition to our proxy environment variables:

Some output from our GitLab CI job

$ https_proxy=$PROXY no_proxy=$NO_PROXY yarn --frozen-lockfile
yarn install v1.22.22
[1/4] Resolving packages...
[2/4] Fetching packages...
warning [email protected]: The engine "pnpm" appears to be invalid.
[3/4] Linking dependencies...
warning "@commitlint/cli > @commitlint/load > [email protected]" has unmet peer dependency "@types/node@*".
warning "@commitlint/cli > @commitlint/load > [email protected]" has unmet peer dependency "typescript@>=4".
[4/4] Building fresh packages...
error /builds/<REDACTED>/node_modules/@johnnymorganz/stylua-bin: Command failed.
Exit code: 1
Command: node ./install.js
Arguments: 
Directory: /builds/<REDACTED>/node_modules/@johnnymorganz/stylua-bin
Output:
Downloading release from https://github.com/johnnymorganz/stylua/releases/download/v0.20.0/stylua-linux-x86_64.zip
Error fetching release: Maximum number of redirects exceeded
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

This is kind of a known issue, see axios/axios#2072 for instance.

This MR implements the fix proposed in axios/axios#4531 (comment).

---

If you are open to it, another option would be to get rid of axios. I see it is only used once so I could refactor the code and just switch to a native node fetch. I could use the undici - fetch for instance: we recently contributed upstream to editorconfig-checker to also bring back support for http proxies 😉

/cc @nejch @fgreinacher @bufferoverflow

🛠️ with ❤️ by Siemens

[antoineauger]

@JohnnyMorganz let me know what you think regarding this MR / a refactoring without axios, happy to contribute 🙏🏻

[JohnnyMorganz]

Thanks for your interest and the PR! Fix looks pretty reasonable to me, happy to merge it in. I haven't tested it myself but the general smoke test still works so I'll take your word for it if the proxy setup works too 😁

Also happy to switch away from using axios if there is a benefit in doing so. I don't remember any particular reason it was chosen (other than being able to stream and pipe the result, but maybe other libraries can do that too). I just noticed in your diff that we already have a dependency on node-fetch, but don't seem to use it... If it doesn't really bring any improvements though then it's probably fine to just leave it be.

It might take a bit of time for this fix to make its way to a release unfortunately. I'm hoping to pick up some work and get a v2 release out, but that may take a while. Let me know if it would be beneficial for a patch release.

[antoineauger]

Thanks for the quick merge @JohnnyMorganz!

I haven't tested it myself but the general smoke test still works so I'll take your word for it if the proxy setup works too

TBH, I was not expecting such a quick merge 😁.

I can still add a small test a-la https://github.com/TooTallNate/proxy-agents/blob/main/packages/proxy-agent/test/test.ts with a mock proxy server if you think it is necessary (seems a simpler solution than testing this with GitHub actions but maybe I'm wrong).

but maybe other libraries can do that too

Yes, see https://medium.com/deno-the-complete-reference/download-file-with-fetch-in-node-js-57dd370c973a for instance.

If it doesn't really bring any improvements though then it's probably fine to just leave it be.

After having went through the proxy-related axios issues, I do think switching for a more-adopted solution would only make the maintenance easier (and you already have the dep on node-fetch as you mentioned).

But I don't want to force your decision here, just let me know and I would be happy to refactor.

It might take a bit of time for this fix to make its way to a release unfortunately. I'm hoping to pick up some work and get a v2 release out, but that may take a while. Let me know if it would be beneficial for a patch release.

Alright, no pressure. I will implement a quick wget-based workaround on our end for the time being and I will watch out for any releases.

Thanks again for maintaining this tool 👍🏻