feat: support logging of exceptions#277
Open
IzaakGough wants to merge 13 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fix
firebase_functions.loggerso exception values can be logged safely without failing JSON serialization, including the documentedsys.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:
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 raisedTypeError: Object of type type is not JSON serializableinstead 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:
BaseExceptioninstancessys.exc_info()[0]Exception payloads are now converted into JSON-safe dictionaries. When the logger is called from an active
exceptblock, it captures the current exception message and formatted stack trace fromsys.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:
logger.error(..., error=exception)with exception instances.logger.error(..., error=sys.exc_info()[0]).logger.exception(...)and verifying that stack trace information is included.Manual verification: