fix(release): allow downloading binary via proxy again#407
fix(release): allow downloading binary via proxy again#407theoludwig merged 3 commits intoeditorconfig-checker:masterfrom
Conversation
33a3acd to
9d06ed6
Compare
9d06ed6 to
9c5c2c3
Compare
|
@theoludwig @mstruebing let me know what you think! I tested this already in our corporate enviornment and should work fine. |
9c5c2c3 to
f818f96
Compare
theoludwig
left a comment
There was a problem hiding this comment.
Thank you for the PR. 🚀
Mostly looks good to me.
I will try to test it locally, this afternoon.
We might find a way to refactor to avoid the any and maybe we can directly use the built-in Node.js fetch (which uses undici under the hood), but that's not a blocker to merge and release this if we can't do the refactor. 😄
|
Thanks for the quick review @theoludwig! Ah ok, I'm just not sure if the bundled undici exposes ProxyAgent, so I just imported fetch explicitly along with it. I still need to dive a bit more into the ecosystem though :) Also apologies if I'm mixing styles a bit here. just noticed and pushed, maybe needs more linting for people like me 😅 |
|
@nejch I refactored the code, to avoid the About using Node.js built-in fetch for
No worries, I formatted the code with Prettier, and now, it is also verified with CI. |
|
🎉 This PR is included in version 5.1.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Co-authored-by: Théo LUDWIG <contact@theoludwig.fr>
What changes this PR introduce?
I managed to find a way to test this, plus using the new undici methods and agents. This coincidentally solves #402, since the built javascript is back to 1MB.
The new undici/octokit has absolutely no support for standard environment variables for proxies, so I had to add a rather old
proxy-from-envlibrary, but in the end I realized that's what the old proxy-agent library uses anyway - and in fact it's a solved problem so I don't see it being from 2020 a real issue.I reused octokit's approach to proxy testing with a real little proxy server, this should help if people refactor code to still tes something close to a real scenario.
Edit: since I added some real tests, this refactors the naming of the script to be a bit more accurate.
I also had to bump tsx due to CI failures (see e.g. redwoodjs/graphql#9653)
List any relevant issue numbers
Closes #405
Closes #402 (node-fetch and proxy-agent replaced with undici's implementations)
Is there anything you'd like reviewers to focus on?
Not sure if we should add a note to not delete those tests to avoid any further regressions 😅