Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/dockerflow/logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def convert_record(self, record):
fields["msg"] = message

# If there is an error, format it for nice output.
if record.exc_info is not None:
if record.exc_info:
fields["error"] = repr(record.exc_info[1])
fields["traceback"] = safer_format_traceback(*record.exc_info)

Expand Down
37 changes: 37 additions & 0 deletions tests/core/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
import logging
import logging.config
import os
import sys
import textwrap

import jsonschema
import pytest
from dockerflow.logging import JsonLogFormatter

logger_name = "tests"
Expand Down Expand Up @@ -116,6 +118,41 @@ def test_logging_error_tracebacks(caplog):
assert "ValueError" in details["Fields"]["traceback"]


@pytest.mark.skipif(sys.version_info < (3, 5), reason="Requires python >= 3.5")
def test_logging_exc_info_false_3x(caplog):
"""Ensure log formatter does not fail and does not include exception
traceback information when exc_info is False under Python 3.x"""
try:
raise ValueError("\n")
except Exception:
logging.exception("there was an error", exc_info=False)
details = assert_records(caplog.records)

assert details["Severity"] == 3
assert details["Fields"]["msg"] == "there was an error"
assert "error" not in details["Fields"]
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is causing a test error in Python 2, it seems that error and traceback are present, I'm not sure why. Any ideas of what to do here ? I haven't been dealing with Python 2 for a while now...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Curious, can you check with PDB what exc_info is on Python 2.7? Does it evaluate differently on 2.x?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • On python 3.8, when exc_info=False is passed, record.exc_info is False.
  • On python 2.7, when exc_info=False is passed... record.exc_info is a tuple with the exception and the traceback anyway...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, that is certainly disappointing, so as long as dockerflow supports Python 2, we should probably extend the condition so the tests pass.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could I just skip this test in python 2 ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmmyeah, how about we have a test for each Python version?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah let's do that. c650711

assert "traceback" not in details["Fields"]


@pytest.mark.skipif(sys.version_info >= (3,), reason="Requires python 2")
def test_logging_exc_info_false_2x(caplog):
"""Ensure log formatter does not fail *but* still includes exception
traceback information when exc_info is False under Python 2.x"""
try:
raise ValueError("\n")
except Exception:
logging.exception("there was an error", exc_info=False)
details = assert_records(caplog.records)

assert details["Severity"] == 3
assert details["Fields"]["msg"] == "there was an error"
# In Python 2, when exc_info=False is passed, we still see a tuple in the
# formatter, so we still include the error and traceback keys.
assert details["Fields"]["error"].startswith("ValueError('\\n'")
assert details["Fields"]["traceback"].startswith("Uncaught exception:")
assert "ValueError" in details["Fields"]["traceback"]


def test_ignore_json_message(caplog):
"""Ensure log formatter ignores messages that are JSON already"""
try:
Expand Down