Allow to configure a proxy using environment vars #4233
Conversation
There was a problem hiding this comment.
Thank you for this initiative! There are a few questions, however:
- Which types of requests does this affect? Only the ones made by got, anything that uses the net module, or some other subset?
- Does this affect requests made by Electron (none node code) as well?
- We should probably allow users to configure Proxies in the settings, similar to in other apps ("Use system settings", "use no proxy", "specify proxy")
- Can we disable Proxies altogether if not needed? I.e., call bootstrap() programmatically, maybe even after the config provider has booted up? If this only needs to happen before the first request, we can easily query the config provider which doesn't make a request itself
source/global-agend.d.ts
Outdated
| @@ -0,0 1,3 @@ | |||
| declare module 'global-agent' { | |||
There was a problem hiding this comment.
There's a typo in the file name
source/main.ts
Outdated
| @@ -47,6 48,9 @@ if (!app.requestSingleInstanceLock()) { | |||
|
|
|||
| // If we reach this point, we are now booting the first instance of Zettlr. | |||
|
|
|||
| // Use a proxy if one has been configured | |||
| bootstrap() | |||
There was a problem hiding this comment.
If not necessary to do so before the appReady event, please move this call out of the main file and into the boot cycle. If this needs to be called first thing even before the boot cycle, please add a comment why.
UFOMelkor
force-pushed
the
proxy-support
branch
from
15dc2bd to
3dab2c1
Compare
According to the docs, it affects everything that uses node's
I'll test a later calling point when next time at work. If it is possible to call it after the config provider has booted up, I would guess the way to go would be adding a service provider for applying proxy configuration after the config provider but before the update provider? |
|
Thanks for the update! Regarding Electron requests: I was specifically thinking about images. These are automatically loaded once an image is added to the document. So one thing you could test is see if image loading in Markdown files after proxying are also using the proxy or if they bypass it. Then, I think it would suffice to add a comment explaining what is affected just so that we know (especially since this is a side effect that we may wonder about in the future). Regarding where and how: that's a good question. On the one hand it's environment setup so the most correct place would be the end checker, but then when it's user configurable it can only run after the config provider has booted up. I'm a little bit hesitant regarding another service provider just for one setting. The question is: could there be potentially more settings except the proxy configuration that may fall into this category of needing to run early but still only after the config? One thing I could think about is the language setting, which needs to run super early but is currently instantiated only after the config has loaded. Maybe a dedicated function within the config provider that sets up envs from the config ...? Hmmm... |
UFOMelkor
force-pushed
the
proxy-support
branch
from
3dab2c1 to
07c6fb3
Compare
See the latest comment in #4084.
I do need to test it again at work, but if you have some spare time, you might provide some feedback on whether this implementation would be okay or you would like follow another approach.