If one manually edits Sopel's config file, it is possible to trigger an error case where the underlying RawConfigParser pulls in the value (intended) as if it is a key with None as the value.
Trying to process the malformed config file into ConfigSections fails, and the end of the resulting traceback misleads the user: ValueError: Missing required value for core.admin_accounts (core.admin_accounts is not a required setting)
This was discovered by a user who didn't realize that the channel list must be indented, but it can occur anywhere in the config file. Ultimately it's down to any string at the beginning of a line that is not followed by =.
Reproduction steps
Insert a bare key with no = and no value anywhere in Sopel's [core] section, for example:
[core]
nick = MyBot
foo
host = irc.libera.chat
# ...
Expected behavior
I expect one of two behavior options:
Sopel catches the error thrown here in ConfigSection.__init__() when an empty value is None instead of '' and re-raises a friendlier ConfigurationError with the malformed string (so the user can easily Ctrl-F for it in their text editor).
Sopel squelches the error, or coerces the None to '' so processing can continue.
Option 1 is the strictest, but I wouldn't feel too bad about forcing users who have already manually edited the config file to go back and recheck it for syntax errors. 😁
Option 2 would sort of hide the problem, even though #1973 means Sopel will log a warning (sopel.bot WARNING - Config option `core.foo` is not defined by its section and may not be recognized by Sopel.). It's unlikely anyone will look at the logs unless Sopel actually refuses to start.
Relevant logs
Traceback (most recent call last):
File "/workspace/sopel/sopel/config/types.py", line 75, in __init__
getattr(self, value)
File "/workspace/sopel/sopel/config/types.py", line 227, in __get__
section = getattr(settings, instance._section_name)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/sopel/sopel/config/__init__.py", line 316, in __getattr__
section = self.ConfigSection(name, items, self) # Return a section
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/sopel/sopel/config/__init__.py", line 295, in __init__
value = item[1].strip()
^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'strip'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/workspace/.pyenv_mirror/user/3.12.3/bin/sopel", line 8, in <module>
sys.exit(main())
^^^^^^
File "/workspace/sopel/sopel/cli/run.py", line 467, in main
return command(opts)
^^^^^^^^^^^^^
File "/workspace/sopel/sopel/cli/run.py", line 290, in command_start
settings = get_configuration(opts)
^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/sopel/sopel/cli/run.py", line 226, in get_configuration
settings = utils.load_settings(options)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/sopel/sopel/cli/utils.py", line 349, in load_settings
return config.Config(filename)
^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/sopel/sopel/config/__init__.py", line 136, in __init__
self.define_section('core', core_section.CoreSection,
File "/workspace/sopel/sopel/config/__init__.py", line 278, in define_section
setattr(self, name, cls_(self, name, validate=validate))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/workspace/sopel/sopel/config/types.py", line 82, in __init__
raise ValueError(
ValueError: Missing required value for core.admin_accounts
Notes
I'd like to thank @Duckle29 for prompting me to investigate this. The error was intriguing because it makes absolutely no sense until you tease apart the traceback (and even then, I stopped before figuring out "why admin_accounts specifically?")
Thanks to writing up this issue, I'm now also thinking about a little reorganization of the Config class, because why is the ConfigSection class defined inside the Config class? (It's been that way since 3.1; the class definition was added 12 years ago.)
Description
If one manually edits Sopel's config file, it is possible to trigger an error case where the underlying
RawConfigParserpulls in the value (intended) as if it is a key withNoneas the value.Trying to process the malformed config file into
ConfigSections fails, and the end of the resulting traceback misleads the user:ValueError: Missing required value for core.admin_accounts(core.admin_accountsis not a required setting)This was discovered by a user who didn't realize that the channel list must be indented, but it can occur anywhere in the config file. Ultimately it's down to any string at the beginning of a line that is not followed by
=.Reproduction steps
Insert a bare key with no
=and no value anywhere in Sopel's[core]section, for example:Expected behavior
I expect one of two behavior options:
ConfigSection.__init__()when an empty value isNoneinstead of''and re-raises a friendlierConfigurationErrorwith the malformed string (so the user can easily Ctrl-F for it in their text editor).Noneto''so processing can continue.Option 1 is the strictest, but I wouldn't feel too bad about forcing users who have already manually edited the config file to go back and recheck it for syntax errors. 😁
Option 2 would sort of hide the problem, even though #1973 means Sopel will log a warning (
sopel.bot WARNING - Config option `core.foo` is not defined by its section and may not be recognized by Sopel.). It's unlikely anyone will look at the logs unless Sopel actually refuses to start.Relevant logs
Notes
I'd like to thank @Duckle29 for prompting me to investigate this. The error was intriguing because it makes absolutely no sense until you tease apart the traceback (and even then, I stopped before figuring out "why admin_accounts specifically?")
Thanks to writing up this issue, I'm now also thinking about a little reorganization of the
Configclass, because why is theConfigSectionclass defined inside theConfigclass? (It's been that way since 3.1; the class definition was added 12 years ago.)Sopel version
8.0.0
Installation method
pip installPython version
3.12.3
Operating system
Ubuntu 22.04.3 LTS
IRCd
No response
Relevant plugins
No response