hynek / structlog

Simple, powerful, and fast logging for Python.
https://www.structlog.org/
Other
3.6k stars 225 forks source link

ExceptionRenderer w/ ExceptionDictTransformer unable to handle exception log outside exception handler #634

Open raqbit opened 4 months ago

raqbit commented 4 months ago

When using the exception log method outside an exception handler, the ExceptionDictTransformer raises an AttributeError as it calls extract, which does not handle the case where the given exception type is None.

While according to the Python docs^1 this method should only be used from exception handlers, I ran into this issue as the aio-pika (aiormq) library uses it outside an exception handler (issue). (I have configured structlog as the formatter for all loggers, using this setup)

Note that the default _format_exception formatter handles this case properly, as it handles (None, None, None) exc_info^2.


Reproduction:

import structlog
print(structlog.__version__) # 24.4.0
structlog.configure([structlog.processors.dict_tracebacks])
structlog.get_logger().exception("oh no") # AttributeError: 'NoneType' object has no attribute '__name__'. Did you mean: '__ne__'?
hynek commented 4 months ago

Is this a new issue?

@sscherfke I'm afraid there's work for you. 🤓

raqbit commented 4 months ago

Can reproduce in 24.2.0, which does not include #627

Python 3.11.9 (main, Apr  6 2024, 17:59:24) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import structlog
>>> print(structlog.__version__)
24.2.0
>>> structlog.configure([structlog.processors.dict_tracebacks])
>>> structlog.get_logger().exception("oh no")
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<snip>/venv/lib/python3.11/site-packages/structlog/_native.py", line 45, in exception
    return self.error(event, *args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<snip>/venv/lib/python3.11/site-packages/structlog/_native.py", line 134, in meth
    return self._proxy_to_logger(name, event, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<snip>/venv/lib/python3.11/site-packages/structlog/_base.py", line 214, in _proxy_to_logger
    args, kw = self._process_event(method_name, event, event_kw)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<snip>/venv/lib/python3.11/site-packages/structlog/_base.py", line 165, in _process_event
    event_dict = proc(self._logger, method_name, event_dict)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<snip>/venv/lib/python3.11/site-packages/structlog/processors.py", line 412, in __call__
    event_dict["exception"] = self.format_exception(
                              ^^^^^^^^^^^^^^^^^^^^^^
  File "<snip>/venv/lib/python3.11/site-packages/structlog/tracebacks.py", line 262, in __call__
    trace = extract(
            ^^^^^^^^
  File "<snip>/venv/lib/python3.11/site-packages/structlog/tracebacks.py", line 155, in extract
    exc_type=safe_str(exc_type.__name__),
                      ^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute '__name__'. Did you mean: '__ne__'?
sscherfke commented 4 months ago

I don't think I introduced this in the last release and I think that @raqbit is right in admitting that is was wrong to call exception() outside an exception handler. But I also think that gracefully handling the (None, None, None) would be the right thing. Plus, I like work. 🙃

raqbit commented 4 months ago

Perhaps this should be handled in ExceptionRenderer instead of in each transformer separately (like it is done right now in _format_exception)?

sscherfke commented 4 months ago

The typing of processors._figure_out_exc_info(...) -> ExcInfo is misleading b/c it does not cover the (None, None, None) case. It should rather be processors._figure_out_exc_info(...) -> ExcInfo | tuple[None, None, None].

How the (None, None, None) is handled should be up to the exception formatter, though.

sscherfke commented 4 months ago

_figure_out_exc_info() is currently being called in ExceptionPrettyPrinter and in ExceptionRenderer.

ExceptionPrettyPrinter performs an if exc_info check but I think this check should rather be if exc_info != (None, None, None).

The ExceptionRenderer does not perform such a check yet.

sscherfke commented 4 months ago

Started a PR. Is this going into the reight direction, @raqbit @hynek ?