diff --git a/logtail/flusher.py b/logtail/flusher.py index 4f927ff..2b19f7c 100644 --- a/logtail/flusher.py +++ b/logtail/flusher.py @@ -80,7 +80,12 @@ def step(self): if delay is not None: time.sleep(delay) - if response.status_code == 500 and getattr(response, "exception") != None: + # A network error surfaces as a Fake500 that carries an `exception` + # attribute; a genuine server HTTP 500 is a real `requests.Response` + # that has no such attribute. Use a default so the latter doesn't + # raise AttributeError here and kill the flush thread (dropping all + # subsequent logs for the life of the process). + if response.status_code == 500 and getattr(response, "exception", None) is not None: print('Failed to send logs to Better Stack after {} retries: {}'.format(len(RETRY_SCHEDULE), response.exception)) self._clean = True diff --git a/tests/test_flusher.py b/tests/test_flusher.py index 68efd2b..f14e07f 100644 --- a/tests/test_flusher.py +++ b/tests/test_flusher.py @@ -1,6 +1,7 @@ # coding: utf-8 from __future__ import print_function, unicode_literals import mock +import requests import time import threading import unittest @@ -123,6 +124,48 @@ def sleep(time): self.assertEqual(self.uploader_calls, len(RETRY_SCHEDULE) + 1) self.assertEqual(self.sleep_calls, len(RETRY_SCHEDULE)) + @patch('logtail.flusher.time.sleep') + def test_step_does_not_crash_on_real_server_500(self, _mock_sleep): + # Regression: a genuine server HTTP 500 returns a real requests.Response, + # which (unlike the network-error Fake500) has no `.exception` attribute. + # `getattr(response, "exception")` without a default raised AttributeError + # here, which propagates out of step()/run() and kills the flush thread — + # every subsequent log is then queued but never sent. (Note: MagicMock + # would auto-vivify `.exception` and hide the bug, so use a real Response.) + first_frame = list(range(self.buffer_capacity)) + + def uploader(frame): + resp = requests.models.Response() + resp.status_code = 500 + return resp + + pipe, _, fw = self._setup_worker(uploader) + for log in first_frame: + pipe.put(log, block=False) + + fw.step() # must not raise AttributeError + + self.assertTrue(fw._clean) + + @patch('logtail.flusher.time.sleep') + def test_step_still_logs_network_error_fake500(self, _mock_sleep): + # The network-error path (Fake500 carrying an exception) must still log. + from logtail.uploader import Fake500 + + first_frame = list(range(self.buffer_capacity)) + + def uploader(frame): + return Fake500(Exception('network is down')) + + pipe, _, fw = self._setup_worker(uploader) + for log in first_frame: + pipe.put(log, block=False) + + with patch('builtins.print') as mock_print: + fw.step() + + mock_print.assert_called_once() + def test_shutdown_condition_empties_queue_and_shuts_down(self): self.buffer_capacity = 10 num_items = 5