Skip to content

Conversation

ktripp
Copy link

@ktripp ktripp commented Apr 10, 2019

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #14 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #14   +/-   ##
=======================================
  Coverage   91.64%   91.64%           
=======================================
  Files           6        6           
  Lines         359      359           
=======================================
  Hits          329      329           
  Misses         30       30
Impacted Files Coverage Δ
exception_reports/storages.py 93.75% <0%> (ø) ⬆️
exception_reports/logs.py 86.04% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 283d170...15d2e0b. Read the comment docs.

record.data["error_report"] = create_exception_report(exc_type, exc_value, tb, self.output_format, self.storage_backend)
except Exception as e:
logger.warning(f"Error generating exception report {repr(e)}") # noqa
logger.error(f"Error generating exception report {repr(e)}", exc_info=True) # noqa
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason to not is that this could create an infinite recursion situation. Clearly only doing a warning is great either, but this might be making things worse.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I didn't really explain. One of the functionalities of the library is to create exception reports for error level logs so this would be calling itself.

@brycedrennan
Copy link
Contributor

What was the error that caused this?

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

Successfully merging this pull request may close these issues.

3 participants