when config.ini file is incorrect, no meaningful error is given to the user #37

Closed
opened 2021-01-08 18:59:03 +00:00 by prandnum · 8 comments
prandnum commented 2021-01-08 18:59:03 +00:00 (Migrated from gitlab.com)

when config.ini file is incorrect, no meaningful error is given to the user it just says "Error parsing logging config from logging config file /peerplays/witness_node_data_dir/config.ini, using default config" and tries to use the default config file which is wrong. See below

2517150ms th_a       config_util.cpp:239           load_logging_config_ ] Error parsing logging config from logging config file /peerplays/witness_node_data_dir/config.ini, using default config
2517150ms th_a       accounts_list_plugin.cpp:128  list_accounts        ] accounts list plugin:  list_accounts()
2517150ms th_a       bookie_plugin.cpp:468         plugin_initialize    ] bookie plugin: plugin_startup() begin
2517150ms th_a       db_management.cpp:290         force_slow_replays   ] enabling slow replays
2517150ms th_a       bookie_plugin.cpp:501         plugin_initialize    ] bookie plugin: plugin_startup() end
2517151ms th_a       witness.cpp:91                plugin_initialize    ] witness plugin:  plugin_initialize() begin
2517151ms th_a       witness.cpp:103               plugin_initialize    ] Public Key: PPY6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV
2517151ms th_a       witness.cpp:121               plugin_initialize    ] witness plugin:  plugin_initialize() end
2517152ms th_a       object_database.cpp:106       open                 ] Opening object database from /peerplays/witness_node_data_dir/blockchain ...
2518333ms th_a       object_database.cpp:111       open                 ] Done opening object database.

The user needs to be able to run a customized config.ini file and if it is not correct we should fail the startup and provide a proper error reason with which the user can take proper corrective action and then try restarting the process.

ex: "seed-nodes value "" is invalid" --> "seed-nodes value "1.2.3.4:3434" is invalid"
"plugin value "" is invalid" --> "plugin value "peerplays" is invalid"

when config.ini file is incorrect, no meaningful error is given to the user it just says "Error parsing logging config from logging config file /peerplays/witness_node_data_dir/config.ini, using default config" and tries to use the default config file which is wrong. See below ``` 2517150ms th_a config_util.cpp:239 load_logging_config_ ] Error parsing logging config from logging config file /peerplays/witness_node_data_dir/config.ini, using default config 2517150ms th_a accounts_list_plugin.cpp:128 list_accounts ] accounts list plugin: list_accounts() 2517150ms th_a bookie_plugin.cpp:468 plugin_initialize ] bookie plugin: plugin_startup() begin 2517150ms th_a db_management.cpp:290 force_slow_replays ] enabling slow replays 2517150ms th_a bookie_plugin.cpp:501 plugin_initialize ] bookie plugin: plugin_startup() end 2517151ms th_a witness.cpp:91 plugin_initialize ] witness plugin: plugin_initialize() begin 2517151ms th_a witness.cpp:103 plugin_initialize ] Public Key: PPY6MRyAjQq8ud7hVNYcfnVPJqcVpscN5So8BhtHuGYqET5GDW5CV 2517151ms th_a witness.cpp:121 plugin_initialize ] witness plugin: plugin_initialize() end 2517152ms th_a object_database.cpp:106 open ] Opening object database from /peerplays/witness_node_data_dir/blockchain ... 2518333ms th_a object_database.cpp:111 open ] Done opening object database. ``` The user needs to be able to run a customized config.ini file and if it is not correct we should fail the startup and provide a proper error reason with which the user can take proper corrective action and then try restarting the process. ex: "seed-nodes value "<value of the seed node>" is invalid" --> "seed-nodes value "1.2.3.4:3434" is invalid" "plugin value "<plugin name>" is invalid" --> "plugin value "peerplays" is invalid"
prandnum commented 2021-01-08 18:59:27 +00:00 (Migrated from gitlab.com)

@bobinson please provide your view.

@bobinson please provide your view.
serkixenos commented 2021-02-17 12:36:05 +00:00 (Migrated from gitlab.com)

In general, config error explanations are printed only if code for reporting them is explicitly added (eg invalid SON configuration). When software is not able to read config options properly (e.g. it expects an array, but integer is provided), execution will stop. Many config options have default values, and the code is able to fallback to them. From the software's point of view, missing config file, or missing options in config file are valid configurations, as software can apply default values.

In order to resolve this, we need to define what exactly is the meaning of "config.ini is not correct", and then we can plan this improvement further.

In general, config error explanations are printed only if code for reporting them is explicitly added (eg invalid SON configuration). When software is not able to read config options properly (e.g. it expects an array, but integer is provided), execution will stop. Many config options have default values, and the code is able to fallback to them. From the software's point of view, missing config file, or missing options in config file are valid configurations, as software can apply default values. In order to resolve this, we need to define what exactly is the meaning of "config.ini is not correct", and then we can plan this improvement further.
serkixenos commented 2021-02-17 12:36:20 +00:00 (Migrated from gitlab.com)

assigned to @prandnum

assigned to @prandnum
prandnum commented 2021-02-17 14:25:41 +00:00 (Migrated from gitlab.com)

There are two cases:

  1. User does not provide a config.ini
  2. User provides a config.ini with wrong values/syntax

It is ok if the system starts for case 1, using the default values but not OK for case 2. In case 2 the startup should fail because starting the application with the default values is definitely not the expected result for the user as he wants a customized blockchain.

Also, there is no special handling required for individual errors, it just needs to mention which line is incorrect. for example
ERROR: Failed to parse the config.ini file. line #1 is not valid

This will really help in troubleshooting startup issues for peerplays newbies.

@bobinson please provide your view.

There are two cases: 1) User does not provide a config.ini 2) User provides a config.ini with wrong values/syntax It is ok if the system starts for case 1, using the default values but not OK for case 2. In case 2 the startup should fail because starting the application with the default values is definitely not the expected result for the user as he wants a customized blockchain. Also, there is no special handling required for individual errors, it just needs to mention which line is incorrect. for example ERROR: Failed to parse the config.ini file. line #1 is not valid This will really help in troubleshooting startup issues for peerplays newbies. @bobinson please provide your view.
serkixenos commented 2021-02-18 12:14:57 +00:00 (Migrated from gitlab.com)

@prandnum Please provide me example of 2. (multiple, if possible) and I will see what we can do about it.

@prandnum Please provide me example of 2. (multiple, if possible) and I will see what we can do about it.
serkixenos commented 2022-03-28 14:20:55 +00:00 (Migrated from gitlab.com)

unassigned @prandnum

unassigned @prandnum
serkixenos commented 2022-04-12 04:44:02 +00:00 (Migrated from gitlab.com)

Due to limitation imposed by Boost program_options, we cant provide generic handling of config file errors.
Create new ticket with specific requirement on specific option.

Due to limitation imposed by Boost program_options, we cant provide generic handling of config file errors. Create new ticket with specific requirement on specific option.
serkixenos commented 2022-04-12 04:44:12 +00:00 (Migrated from gitlab.com)

assigned to @serkixenos

assigned to @serkixenos
serkixenos (Migrated from gitlab.com) closed this issue 2022-04-12 04:44:14 +00:00
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Peerplays_Blockchain/peerplays_migrated#37
No description provided.