From 60ca325813bb1c5d89e669f2eb4b8e2b4476139c Mon Sep 17 00:00:00 2001
From: jaideeppyne <jaideeppyne1997@gmail.com>
Date: Tue, 18 Aug 2026 11:20:52 +0530
Subject: [PATCH] Fix flush thread crashing on a real server HTTP 500 (#25)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`FlushWorker.step` checked `getattr(response, "exception") != None` without a
default. On a network error the uploader returns a `Fake500` that carries an
`exception` attribute, but a genuine server HTTP 500 is a real
`requests.Response`, which has no such attribute. So on a real 500 the
`getattr` raised `AttributeError`, which propagated out of `step()`/`run()` and
killed the background flush thread — every subsequent log was then queued but
never sent for the life of the process.

Pass a default (`getattr(response, "exception", None) is not None`) so a real
Response skips the branch instead of crashing, while the network-error Fake500
path still logs as before.

Adds two regression tests: a real `requests.Response` 500 must not crash the
worker (fails on the old code with the exact AttributeError from #25), and the
Fake500 network-error path must still log. Note: the existing tests use
`MagicMock(status_code=500)`, whose auto-vivified `.exception` masks the bug, so
the new test uses a real Response.

Fixes #25.
---
 logtail/flusher.py    |  7 ++++++-
 tests/test_flusher.py | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)

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