structlog icon indicating copy to clipboard operation
structlog copied to clipboard

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

Open raqbit opened this issue 1 year ago • 7 comments

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__'?

raqbit avatar Jul 23 '24 11:07 raqbit

Is this a new issue?

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

hynek avatar Jul 24 '24 06:07 hynek

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__'?

raqbit avatar Jul 24 '24 06:07 raqbit

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. 🙃

sscherfke avatar Jul 24 '24 06:07 sscherfke

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

raqbit avatar Jul 24 '24 07:07 raqbit

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 avatar Jul 24 '24 07:07 sscherfke

_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 avatar Jul 24 '24 07:07 sscherfke

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

sscherfke avatar Jul 24 '24 08:07 sscherfke