Fix parse errors logged multiple times and document that debug logs can include secrets

CHANGELOG.rst

@@ -102,6 +102,12 @@ Fixed
 - A positional-only parameter that has a default being required, even though
   the call can omit it (`#986
   <https://github.com/mauvilsa/jsonargparse/pull/986>`__).
+- Parse errors logged multiple times when raised from a nested parse, e.g. an
+  invalid value in a config given as argument (`#989
+  <https://github.com/mauvilsa/jsonargparse/pull/989>`__).
+- Subcommand parsers not logging when the logger of the parent parser is set
+  after adding the subcommands (`#989
+  <https://github.com/mauvilsa/jsonargparse/pull/989>`__).
 
 Changed
 ^^^^^^^

DOCUMENTATION.rst

@@ -3662,7 +3662,9 @@ When a parse method fails, by default it prints a short message and exits with a
 non-zero code. During development this is not enough information to find the
 root of the problem. Setting the ``JSONARGPARSE_DEBUG`` environment variable to
 ``true`` changes this, without touching the source code: an
-:class:`.ArgumentError` is raised and the full stack trace is printed.
+:class:`.ArgumentError` is raised, the full stack trace is printed and parsers
+without a logger log at debug level. Debug logs can include the raw input given
+to parsers, secrets included, so only enable them for troubleshooting.
 
 The parsers log some basic events, though this is disabled by default. To enable
 it, set the ``logger`` argument when creating an :class:`.ArgumentParser`. The

jsonargparse/_core.py

@@ -23,6 +23,7 @@
     previous_config,
 )
 from ._common import (
+    LoggerProperty,
     command_line_source,
     config_schema_key,
     debug_mode_active,
@@ -1201,7 +1202,9 @@ def error(self, message: str, ex: Exception | None = None) -> NoReturn:
         error = argument_error(message)
         if source is not None:
             error.value_source_reported = True  # type: ignore[attr-defined]  # so that it is not added again
-        self._logger.error(message)
+        if not getattr(ex, "error_logged", False):  # a nested error was already logged
+            self._logger.error(message)
+        error.error_logged = True  # type: ignore[attr-defined]
         if not self.exit_on_error:
             raise error from ex
         elif debug_mode_active():
@@ -1674,6 +1677,13 @@ def dump_header(self, dump_header: list[str] | None):
             raise ValueError("Expected dump_header to be None or a list of strings.")
         self._dump_header = dump_header
 
+    @LoggerProperty.logger.setter  # type: ignore[attr-defined]
+    def logger(self, logger: bool | str | dict | logging.Logger):
+        LoggerProperty.logger.fset(self, logger)  # type: ignore[attr-defined]
+        if self._subcommands_action:
+            for subparser in self._subcommands_action._name_parser_map.values():
+                subparser.logger = self._logger
+
     # Not supported methods
 
     def parse_known_args(self, *args, **kwargs) -> NoReturn:

jsonargparse_tests/test_core.py

@@ -1150,6 +1150,15 @@ def test_debug_environment_variable(logger):
     assert "Debug enabled, thus raising exception instead of exit" in logs.getvalue()
 
 
+def test_nested_error_logged_once(logger):
+    parser = ArgumentParser(exit_on_error=False, logger=logger)
+    parser.add_argument("--cfg", action="config")
+    parser.add_argument("--int", type=int)
+    with pytest.raises(ArgumentError), capture_logs(logger) as logs:
+        parser.parse_args(["--cfg", '{"int": "invalid"}'])
+    assert 1 == logs.getvalue().count('Parser key "int"')
+
+
 def test_parse_known_args_not_implemented(parser):
     pytest.raises(NotImplementedError, lambda: parser.parse_known_args([]))
 

jsonargparse_tests/test_subcommands.py

@@ -18,6 +18,7 @@
     add_instantiator,
 )
 from jsonargparse_tests.conftest import (
+    capture_logs,
     get_parse_args_stderr,
     get_parse_args_stdout,
     get_parser_help,
@@ -61,7 +62,7 @@
 
 def test_subcommands_get_defaults(subcommands_parser):
     cfg = subcommands_parser.get_defaults().as_dict()
     assert cfg == {"o1": "o1_def", "subcommand": None}
 
 
 def test_subcommands_undefined_subcommand(subcommands_parser):
@@ -313,6 +314,17 @@
     assert cfg == Namespace(subcommand=None)
 
 
+def test_subcommand_logger_set_after_adding(parser, subparser, logger):
+    subparser.add_argument("--int", type=int)
+    subcommands = parser.add_subcommands()
+    subcommands.add_subcommand("foo", subparser)
+    parser.logger = logger
+    assert subparser.logger is logger
+    with pytest.raises(ArgumentError), capture_logs(logger) as logs:
+        parser.parse_args(["foo", "--int=invalid"])
+    assert 1 == logs.getvalue().count('Parser key "int"')
+
+
 def test_subcommand_without_options(parser, subparser):
     subcommands = parser.add_subcommands()
     subcommands.add_subcommand("foo", subparser)