Skip to content

Commit f6ab797

Browse files
committed
fix(batcher): Do not let a failed flush kill the flusher thread (#7138)
An unhandled exception inside `_flush_loop` terminated the batcher's daemon flusher thread. After that the buffer kept filling with nothing draining it. Every later log, metric or span was then dropped for the rest of the process lifetime once the queue hit its cap. Wrap the flush call in each loop (`Batcher._flush_loop` and `SpanBatcher._flush_loop`) in `capture_internal_exceptions()`, the SDK's own helper for errors that should be logged rather than propagated. A single bad batch is now swallowed and logged so the loop keeps running. Adds a regression test for each loop that drives one iteration where the flush raises then asserts the loop returns instead of propagating.
1 parent 1c3b50d commit f6ab797

4 files changed

Lines changed: 89 additions & 10 deletions

File tree

sentry_sdk/_batcher.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from typing import TYPE_CHECKING, Generic, TypeVar
77

88
from sentry_sdk.envelope import Envelope, Item, PayloadRef
9-
from sentry_sdk.utils import format_timestamp
9+
from sentry_sdk.utils import capture_internal_exceptions, format_timestamp
1010

1111
if TYPE_CHECKING:
1212
from typing import Any, Callable, Optional
@@ -100,7 +100,13 @@ def _flush_loop(self) -> None:
100100
while self._running:
101101
self._flush_event.wait(self.FLUSH_WAIT_TIME + random.random())
102102
self._flush_event.clear()
103-
self._flush()
103+
# A failure to serialize or send one batch must not kill the
104+
# flusher thread. If it did, the buffer would keep filling with
105+
# nothing draining it, and every later log or metric would be
106+
# dropped for the rest of the process lifetime. Swallow and log
107+
# the error instead so the loop keeps running.
108+
with capture_internal_exceptions():
109+
self._flush()
104110

105111
def add(self, item: "T") -> None:
106112
# Bail out if the current thread is already executing batcher code.

sentry_sdk/_span_batcher.py

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,11 @@
99

1010
from sentry_sdk._batcher import Batcher
1111
from sentry_sdk.envelope import Envelope, Item, PayloadRef
12-
from sentry_sdk.utils import format_timestamp, serialize_attribute
12+
from sentry_sdk.utils import (
13+
capture_internal_exceptions,
14+
format_timestamp,
15+
serialize_attribute,
16+
)
1317

1418
if TYPE_CHECKING:
1519
from typing import Any, Callable, Optional
@@ -91,14 +95,19 @@ def _flush_loop(self) -> None:
9195
self._flush_event.wait(timeout=self.FLUSH_WAIT_TIME + jitter)
9296
self._flush_event.clear()
9397

94-
self._flush(only_pending=True)
98+
# A failure in one flush must not kill the flusher thread, or the
99+
# span buffer would keep filling with nothing draining it and every
100+
# later span would be dropped for the rest of the process lifetime.
101+
# Swallow and log the error instead so the loop keeps running.
102+
with capture_internal_exceptions():
103+
self._flush(only_pending=True)
95104

96-
if (
97-
time.monotonic() - self._last_full_flush
98-
>= self.FLUSH_WAIT_TIME + jitter
99-
):
100-
self._flush()
101-
self._last_full_flush = time.monotonic()
105+
if (
106+
time.monotonic() - self._last_full_flush
107+
>= self.FLUSH_WAIT_TIME + jitter
108+
):
109+
self._flush()
110+
self._last_full_flush = time.monotonic()
102111

103112
def add(self, span: "SpanJSON") -> None:
104113
# Bail out if the current thread is already executing batcher code.

tests/test_logs.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,3 +922,36 @@ def test_log_batcher_lock_reset_in_child_after_fork(sentry_init):
922922
original_lock.release()
923923
_, status = os.waitpid(pid, 0)
924924
assert os.WIFEXITED(status) and os.WEXITSTATUS(status) == 0
925+
926+
927+
@pytest.mark.tests_internal_exceptions
928+
def test_flush_loop_swallows_flush_exception():
929+
"""The flush loop must not let one failed flush kill the flusher thread.
930+
931+
Regression test for #7138: an unhandled exception inside _flush_loop
932+
terminated the daemon flusher thread. After that logs silently stopped
933+
being delivered and eventually got dropped at the queue cap. The loop must
934+
swallow the error and keep running.
935+
936+
Driven synchronously on a bare batcher: _flush raises once and then stops
937+
the loop, so _flush_loop returns cleanly on fixed code and propagates the
938+
exception on unfixed code.
939+
"""
940+
from sentry_sdk._batcher import Batcher
941+
942+
calls = []
943+
944+
class ExplodingBatcher(Batcher):
945+
def _flush(self):
946+
calls.append(1)
947+
self._running = False # exit the loop after this one iteration
948+
raise RuntimeError("boom in flush")
949+
950+
batcher = ExplodingBatcher(
951+
capture_func=lambda envelope: None,
952+
record_lost_func=lambda *a, **k: None,
953+
)
954+
batcher._flush_event.set() # so the loop's wait() returns at once
955+
batcher._flush_loop()
956+
957+
assert calls == [1]

tests/tracing/test_span_batcher.py

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -541,3 +541,34 @@ def test_span_batcher_lock_reset_in_child_after_fork(sentry_init):
541541
original_lock.release()
542542
_, status = os.waitpid(pid, 0)
543543
assert os.WIFEXITED(status) and os.WEXITSTATUS(status) == 0
544+
545+
546+
@pytest.mark.tests_internal_exceptions
547+
def test_flush_loop_swallows_flush_exception():
548+
"""The flush loop must not let one failed flush kill the flusher thread.
549+
550+
Regression test for #7138: an unhandled exception inside _flush_loop
551+
terminated the daemon flusher thread. After that the span buffer filled up
552+
with nothing draining it, and every later span was dropped for the rest of
553+
the process lifetime. The loop must swallow the error and keep running.
554+
555+
Driven synchronously on a bare batcher: _flush raises once and then stops
556+
the loop, so _flush_loop returns cleanly on fixed code and propagates the
557+
exception on unfixed code.
558+
"""
559+
calls = []
560+
561+
class ExplodingSpanBatcher(SpanBatcher):
562+
def _flush(self, only_pending=False):
563+
calls.append(1)
564+
self._running = False # exit the loop after this one iteration
565+
raise RuntimeError("boom in flush")
566+
567+
batcher = ExplodingSpanBatcher(
568+
capture_func=lambda envelope: None,
569+
record_lost_func=lambda *a, **k: None,
570+
)
571+
batcher._flush_event.set() # so the loop's wait() returns at once
572+
batcher._flush_loop()
573+
574+
assert calls == [1]

0 commit comments

Comments
 (0)