Fixes #3253 - make `User-Agent` configurable
Adds a configuration option to overwrite the default User-Agent header that is send at least by the calendar and news module. Allows other modules to use the individual user agent as well.
The configuration accepts either a string or a function:
var config =
{
...
userAgent: 'Mozilla/5.0 (My User Agent)',
...
}
or
var config =
{
...
userAgent: () => 'Mozilla/5.0 (My User Agent)',
...
}
Thank you for this PR!
First thing, the test fails are unrelated to your changes, we had problems with one unit test which are now fixed on develop, so can you please merge develop into your branch (or rebase on develop), thanks.
I'm not sure if we need the new getUserAgent() function because we store defaults in js/defaults.js, so AFAIK would be the normal way to add the entry
userAgent: `Mozilla/5.0 (Node.js ${Number(process.version.match(/^v(\d+\.\d+)/)[1])}) MagicMirror/${global.version}`,
in this file and then use this property with config.userAgent in the modules and the tests.
But before refactoring this may @rejas can review and tell us what he prefers ...
I'm not sure if we need the new
getUserAgent()function because we store defaults injs/defaults.js, so AFAIK would be the normal way to add the entryuserAgent: `Mozilla/5.0 (Node.js ${Number(process.version.match(/^v(\d+\.\d+)/)[1])}) MagicMirror/${global.version}`,in this file and then use this property with
config.userAgentin the modules and the tests.
In the beginning I thought about that as well, but it felt a little bit odd to have all those variables in a configuration string by default. Also I wasn't sure when those variables are available and in case someone wants to use other runtime variables in here, that might lead to issues - that's why I added the possibility to use it with a callback as well.
I also thought about, wether it would make sense to add a parameter (e.g. the module name, url, ...) to the method, so that it could be possible to use different user agents based upon those parameters.
Based on those thoughts i decided to wrap it in a utility method to allow easier, non-breaking, changes in the future. But if you prefer to consume the config.userAgent directly, I'm also fine with it 😉
and I forgot something yesterday because config.userAgent doesn't work, the module only gets his own config section, not the global one (so the default userAgent had to be set in every module).
So I'm fine with merging this.
since when does config. not work?
you have to pass it from browser side to node helper,
may I'm wrong but AFAIK you can't get e.g. config.language in the node_helper.js of the module (not without special coding)
yes, but 'special' is tiny..
from my MMM-Config
notificationReceived: function (notification, payload, sender) {
// once everybody is loaded up
if (notification === "ALL_MODULES_STARTED") {
// send our config to our node_helper
// copy global config info to module config
this.config.address = config.address;
this.config.port = config.port;
this.config.whiteList = config.ipWhitelist;
this.sendSocketNotification("CONFIG", this.config); // this sends to node_helper
}
},
so for using config.userAgent instead of the implemented function in this PR we had to do this.config.userAgent= config.userAgent; in the 2 modules.
I'm fine with both solutions.
and then in node_helper to use it, its payload.config.userAgent (or whatever variable payload is stored in) from the sockerReceivedNotifications.
and you can get it ALL with
this.config.global_config = config
we have no cross module protection on the config data