bee icon indicating copy to clipboard operation
bee copied to clipboard

Disallow/warn about unknown parameters in config

Open AuHau opened this issue 2 years ago • 2 comments

Currently, if you put some unknown parameter in the config, Bee ignores it and does not "yell" or exit. While this saves us some work on Desktop for the upcoming 1.8 release (we don't have to write a migration to remove transaction and block-hash for existing configs), I would argue that this might not be the best behavior.

Mainly, because it creates space for user errors when the configuration of Bee, especially for typos. For example, a user enters ful-node: true and wonders why Bee does not launch in Full mode even no errors seems to appear, but he has a typo ful-node instead of full-node.

I understand that this is a bit philosophical question about what should happen. You can go either with being strict and exit upon unknown parameters (honestly, my preference - fail fast), but if you would be against that, I would at least suggest putting a warning message to logs about the unknown parameters.

AuHau avatar Sep 09 '22 07:09 AuHau

Having fallen victim to just such an error in the past with a bee node, I would go with this suggestion, even a warning would help. Most command line oriented programs won't run if you have an unsupported argument, so there is certainly much precedent. I even learned just last night that nethermind will warn you if you have an unrecognizable NETHERMIND_* environment variable defined which saved me much consternation when my customization options weren't working.

ldeffenb avatar Sep 09 '22 10:09 ldeffenb

I'm working on a PR that would warn about unrecognized params like so:

DISCLAIMER:
This software is provided to you "as is", use at your own risk and without warranties of any kind.
It is your responsibility to read and understand how Swarm works and the implications of running this software.
The usage of Bee involves various risks, including, but not limited to:
damage to hardware or loss of funds associated with the Ethereum account connected to your node.
No developers or entity involved will be liable for any claims and damages associated with your use,
inability to use, or your interaction with other nodes or the software.

version: 1.8.0-dev - planned to be supported until 11 December 2022, please follow https://ethswarm.org/

WARN: unrecongnized configuration parameters:  db-capacity

"time"="2022-09-12 14:25:07.534721" "level"="warning" "logger"="node" "msg"="clef is not enabled; portability and security of your keys is sub optimal"

notanatol avatar Sep 12 '22 11:09 notanatol

This looks quite alright to me, although I would still prefer to directly error out and exit, instead of do warnings 😅 @notanatol have you consider that? I mean the more I think about this, the fact that you are running the Bee with some unknown flag means that the user expect the flag to do something, which it won't as it is not known to Bee logic, so in this POV it makes more sense to error out and let the user fix it.

AuHau avatar Oct 24 '22 12:10 AuHau

But if you error out and exit, some service and docker implementations will go into an infinite restart loop. Not down-voting this suggestions as I totally agree with @AuHau reasoning, just another data point to consider.

And having typed this, anyone that notices such restart looping will likely think to go look for a reason and will hopefully then notice the configuration warning...

ldeffenb avatar Oct 24 '22 14:10 ldeffenb

Agree, although I expect that this will be a problem only when it is released as it is breaking change and will most probably break some instances (for example in Swarm Desktop we also did not clean up some config properties changes in the latest release), but then it should be communicated as breaking change and people should find out.

AuHau avatar Oct 26 '22 06:10 AuHau