Repository navigation
fix: log traceback in logger.exception() of Haystack loggers - #13189
hardness1020 wants to merge 2 commits into
Conversation
Stop wrapping `exception` in `getLogger`: the wrapper forwarded `exc_info=None`, so records had no traceback. Stdlib `Logger.exception` already calls the patched `error` with `exc_info=True`, which also fixes the caller line on Python 3.10. Fixes deepset-ai#13188 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@hardness1020 is attempting to deploy a commit to the deepset Team on Vercel. A member of the Team first needs to authorize it. |
| # `exception` stays unpatched: the stdlib version calls the patched `error` with `exc_info=True`. Wrapping it would | ||
| # pass `exc_info=None` and drop the traceback. |
There was a problem hiding this comment.
Could we keep the wrapper here instead of relying on stdlib Logger.exception delegating to self.error? Without it a positional call raises Logger.error() takes 1 positional argument ..., which is confusing from exception(). Lets give patch_log_method_to_kwargs_only an exc_info default instead:
| # `exception` stays unpatched: the stdlib version calls the patched `error` with `exc_info=True`. Wrapping it would | |
| # pass `exc_info=None` and drop the traceback. | |
| logger.exception = patch_log_method_to_kwargs_only(logger.exception, default_exc_info=True) # type: ignore |
with exc_info: Any = default_exc_info in the inner wrapper's signature. WDYT?
There was a problem hiding this comment.
Good catch, done in 40dc64d: patch_log_method_to_kwargs_only takes a keyword-only default_exc_info, and exception passes True. I also pinned the Logger.<method>() name in test_haystack_logger_with_positional_args.
One trade-off: with two wrappers, the caller line is still wrong on Python 3.10 (it points into haystack/logging.py), as on main. #13065 makes that moot. If you want it fixed now, exception can wrap the unpatched error instead, which keeps the exception() name and passes on 3.10:
logger.exception = functools.wraps(logger.exception)(
patch_log_method_to_kwargs_only(unpatched_error, default_exc_info=True)
)Happy to switch.
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Restore the wrapper so positional-arg errors name `Logger.exception()`, and give `patch_log_method_to_kwargs_only` a keyword-only `default_exc_info`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Related Issues
logger.exception()fromhaystack.logging.getLoggerdoes not log the traceback #13188Proposed Changes:
getLoggerwrappedlogger.exceptionwithpatch_log_method_to_kwargs_only, which always forwardsexc_info=None. The stdlib defaultexc_info=Truenever applied, sologger.exception(...)logged no traceback in JSON or console output.patch_log_method_to_kwargs_onlytakes a keyword-onlydefault_exc_info(defaultNone), andexceptionis wrapped withdefault_exc_info=True. Explicitexc_infostill works, and positional-arg errors still nameLogger.exception().PatchedLogger.exceptionnow declaresexc_info: Any = True.How did you test it?
test/test_logging.py:exception()attachesexc_info, andexc_info=Falseis respected. The traceback test fails without the fix.test_haystack_logger_with_positional_argsnow checks that theTypeErrornames the called method.test/test_logging.pypasses on Python 3.14 and 3.10. Tests for the 8 modules that calllogger.exceptionpass, except 2test_type_serializationfailures that also fail onmainunder 3.14 (Unionrepr).exceptionentry, and the console output renders the traceback.hatch run fmtandhatch run test:typespass.Notes for the reviewer
exception()records stays wrong for plainlogging.Loggerinstances (two wrapper frames), as onmain. It goes away once 3.10 is dropped (Drop Python 3.10 support and add Python 3.15 #13065). See the review thread for a single-wrapper variant that fixes it now.exc_valueis not truncated (same as for stdlib loggers today), and console mode withrichshows locals in tracebacks.Checklist
fix:,feat:,build:,chore:,ci:,docs:,style:,refactor:,perf:,test:and added!in case the PR includes breaking changes.🤖 Generated with Claude Code