fix(logger): fix exception on flush without buffer#6794
fix(logger): fix exception on flush without buffer#6794leandrodamascena merged 3 commits intoaws-powertools:developfrom
Conversation
pull-request-size
bot
added
the
size/XS
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #6794 +/- ##
===========================================
+ Coverage 96.21% 96.23% +0.01%
===========================================
Files 273 273
Lines 12691 12691
Branches 946 946
===========================================
+ Hits 12211 12213 +2
+ Misses 375 374 -1
+ Partials 105 104 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Hey @tonnico thanks for sending this PR. Your fork doesn't allow modification, so, can you please add this test to make sure we are testing this situation?
def test_flush_buffer_without_buffer_config(stdout, service_name, lambda_context, monkeypatch):
# Set initial trace ID for first Lambda invocation
monkeypatch.setenv(constants.XRAY_TRACE_ID_ENV, "1-67c39786-5908a82a246fb67f3089263f")
# GIVEN A logger without buffer configuration
logger = Logger(level="DEBUG", service=service_name, stream=stdout)
@logger.inject_lambda_context(flush_buffer_on_uncaught_error=True)
def handler(event, context):
# Log messages that should be flushed when an exception occurs
logger.debug("this log line will be flushed after error - 1")
logger.debug("this log line will be flushed after error - 2")
raise ValueError("Test error")
# WHEN Invoking the handler and expecting a ValueError
# AND flush_buffer_on_uncaught_error is True but there is no logger buffer configuration
with pytest.raises(ValueError):
handler({}, lambda_context)
# THEN Verify that buffered log messages are flushed without any exception
log = capture_multiple_logging_statements_output(stdout)
assert len(log) == 2, "Expected two log messages to be flushed"
leandrodamascena
changed the title
Co-authored-by: leandrodamascena <leandro.damascena@gmail.com>
pull-request-size
bot
added
size/M
Hello @leandrodamascena. Thank you for the test. However, I have a slightly different view of the behaviour. When no buffer configuration is set and the I changed some comments and values and hope this is ok for you. |
|
Thanks for fixing that, I agree 100% with you! Just let the CI completes and I'll merge!! |
* fix: exception on flush without buffer * fix(logger): add test for flush without buffer Co-authored-by: leandrodamascena <leandro.damascena@gmail.com> --------- Co-authored-by: Leandro Damascena <lcdama@amazon.pt> Co-authored-by: leandrodamascena <leandro.damascena@gmail.com>
2 participants
Issue number: #6793
Summary
Changes
User experience
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.