Skip to content

fix(record): avoid ZeroDivisionError in HttpRecorder final-report fallback#1692

Open
WatchTree-19 wants to merge 1 commit into
openai:mainfrom
WatchTree-19:fix-httprecorder-zerodivision-empty-events
Open

fix(record): avoid ZeroDivisionError in HttpRecorder final-report fallback#1692
WatchTree-19 wants to merge 1 commit into
openai:mainfrom
WatchTree-19:fix-httprecorder-zerodivision-empty-events

Conversation

@WatchTree-19

Copy link
Copy Markdown

summary

HttpRecorder.record_final_report builds a final_report event and sends it with self._send_event([report_event]), but it does not append that event to self._events. For a run that recorded no sample events, self._events stays empty.

When the POST fails, _send_event runs:

self.failed_requests += len(events)
...
if self.failed_requests / len(self._events) > fail_threshold:   # len(self._events) == 0

so it raises ZeroDivisionError. record_final_report only catches RuntimeError, so the ZeroDivisionError propagates and crashes the run instead of falling back to the LocalRecorder.

fix

total_events = len(self._events) or len(events)
if self.failed_requests / total_events > fail_threshold:

_send_event is only ever called with a non-empty batch, so this keeps the ratio well-defined. In the empty-events case the threshold is now exceeded and a RuntimeError is raised, which record_final_report catches and uses to fall back to the LocalRecorder (the intended behavior). The normal (non-empty) path is unchanged.

test

adds test_http_recorder_final_report_no_events_falls_back_without_zerodivision to evals/record_test.py: with requests.post patched to fail and no recorded events, record_final_report must not raise and the report must be written by the fallback LocalRecorder. the test fails with ZeroDivisionError without the fix and passes with it.

…lback

HttpRecorder.record_final_report sends a final_report event that is not
appended to self._events. For a run that recorded no sample events,
self._events is empty, so a failed POST evaluated
failed_requests / len(self._events) and raised ZeroDivisionError. That
error escaped _send_event (record_final_report only catches RuntimeError),
crashing instead of falling back to the LocalRecorder.

Use len(self._events) or len(events) as the denominator so the ratio stays
well-defined; the failed final report now falls back to the LocalRecorder
as intended. Adds a regression test.

Signed-off-by: WatchTree-19 <119982314+WatchTree-19@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant