ddsol / speedtest.net

node.js SpeedTest.net client module
MIT License
611 stars 124 forks source link

move init proxy env to speedTest function #50

Closed devildant closed 6 years ago

devildant commented 6 years ago

i use a node script that periodically runs a speedtest, and I sometimes activate the proxy. at this time the the speedtest returns an error because it has retain the environment parameters of the first launch. this is the reason I moved the initialization of the variables. following this change everything works

ddsol commented 6 years ago

The proxy options should not be a global variable, it should be an option for a particular test and therefore the variable (property) should be associated with the test itself, not with the module.

If you run the script from the command line, it's ok to allow for environment options, but environment options should not be used at all when it's run as a module used by some app.

As such I'm not going to be able to add this code to let the environment variables property be a normal way to pass options to speedtest.

A better solution is to simply pass the new options when creating a new test. This is exactly the reason why the option was added: To allow each test to have its own proxy.

devildant commented 6 years ago

the environment variables can be manipulated within a program, in order to simplify certain actions, in addition, as well, the modification of these variables only affects the context of the program in question, it's not a hack, simply a simple use for a heavy client. 50% of the npm module connected to the network uses the environment variable HTTP_PROXY, HTTPS_PROXY. the best example I can give you is "request". it was a mistake I made to initialize the recovery of these variables at the beginning of the test.

As such I'm not going to be able to add this code to let the environment variables property be a normal way to pass options to speedtest.

in this case remove the management environment variables, because currently they are not managed properly.

Another point is that it is cleaner to handle the proxy in one place within the program, and the environment variables allows that.

Another point, if the speedtest was launched a first time with environment variables it becomes impossible to disable the proxy without fully reloading the module

leave the code in the state requires either restarting the program or cheating and removing the "require" cache.

it's your project you do as you want :)