Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
raqbit opened this issue Jul 23, 2024 · 7 comments

Comments

@raqbit
Copy link

raqbit commented Jul 23, 2024

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 docs1 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_info2.


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

Footnotes

  1. https://docs.python.org/3/library/logging.html#logging.Logger.exception

  2. https://github.com/hynek/structlog/blob/e48a553979b00ab580d8ab0e55b5cf96e752a324/src/structlog/_frames.py#L25

@hynek
Copy link
Owner

hynek commented Jul 24, 2024

Is this a new issue?

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

@raqbit
Copy link
Author

raqbit commented Jul 24, 2024

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
Copy link
Contributor

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
Copy link
Author

raqbit commented Jul 24, 2024

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

@sscherfke
Copy link
Contributor

sscherfke commented Jul 24, 2024

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
Copy link
Contributor

_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
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants