Skip to content

feat: support logging of exceptions#277

Open
IzaakGough wants to merge 13 commits into
mainfrom
@invertase/feat-support-exception-logging
Open

feat: support logging of exceptions#277
IzaakGough wants to merge 13 commits into
mainfrom
@invertase/feat-support-exception-logging

Conversation

@IzaakGough
Copy link
Copy Markdown
Contributor

@IzaakGough IzaakGough commented Jun 1, 2026

Summary

Fix firebase_functions.logger so exception values can be logged safely without failing JSON serialization, including the documented sys.exc_info()[0] pattern from issue #172. Logged exceptions now include structured type, message, and stack trace details.

Problem/Root Cause

Issue #172 reports that the current logging API does not handle exceptions reliably. In particular, the documented example:

except:
    e = sys.exc_info()[0]
    logger.error("...", error=e)

passes an exception class such as TypeError, which is not JSON serializable. The logger attempted to pass that value through the normal JSON serialization path and raised TypeError: Object of type type is not JSON serializable instead of emitting a log entry.

The logger also did not provide a clean path to preserve stack trace information for this case.

Solution/Changes

Add exception-aware normalization in the logger for both:

  • BaseException instances
  • exception classes such as the value returned by sys.exc_info()[0]

Exception payloads are now converted into JSON-safe dictionaries. When the logger is called from an active except block, it captures the current exception message and formatted stack trace from sys.exc_info() so the original documented pattern produces useful structured logs instead of crashing.

This PR also keeps the new logger.exception(...) helper for directly logging the active exception stack trace at error severity.

Testing

Automated tests:

  • Added automated tests for logger.error(..., error=exception) with exception instances.
  • Added a regression test for the exact issue logging library doesn't support Exceptions #172 pattern using logger.error(..., error=sys.exc_info()[0]).
  • Added tests covering self-referential exception arguments and cyclic payloads.
  • Added a test covering logger.exception(...) and verifying that stack trace information is included.

Manual verification:

  • Verified the original emulator repro now logs structured exception output with message and stack trace instead of raising a JSON serialization error.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces exception logging capabilities to the logger, including a new exception function to log active stack traces and utility functions to safely serialize exceptions and handle circular references in exception arguments. Comprehensive tests have been added to verify these changes. The review feedback suggests improving type safety by changing the type of the refs parameter from set[_typing.Any] to set[int] in both _exception_from_args and _remove_circular, as it is used to track object IDs.

Comment thread src/firebase_functions/logger.py
Comment thread src/firebase_functions/logger.py Outdated
@IzaakGough IzaakGough marked this pull request as ready for review June 3, 2026 10:04
@IzaakGough IzaakGough requested a review from a team June 3, 2026 10:04
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 this pull request may close these issues.

2 participants