Skip to content

Re-raise write() exceptions so Fluentd core retry can engage - #91

Closed
ian28223 wants to merge 1 commit into
DataDog:masterfrom
ian28223:ian.bucad/retry-on-write-exceptions
Closed

ian28223 wants to merge 1 commit into
DataDog:masterfrom
ian28223:ian.bucad/retry-on-write-exceptions

Conversation

@ian28223

@ian28223 ian28223 commented Aug 26, 2026

Copy link
Copy Markdown

What does this PR do?

write(chunk) currently wraps its entire body in rescue Exception => e ... end with no re-raise. Raw exceptions like Net::ReadTimeout or EOFError bypass the plugin's own send_retries (which only rescues RetryableError, raised for HTTP 5xx/429), and since write() returns normally regardless, Fluentd core's own buffer retry (update_retry_state / try_flush) never fires either. Net effect: on a transient network error, the in-flight batch of log records is silently dropped, with no retry from either the plugin or Fluentd core.

This PR re-raises after logging in write(chunk), and also re-raises in send_retries once its own retry budget is exhausted (previously it silently returned in that case too), so Fluentd's core retry/backoff can engage as designed.

Motivation

Reported by a customer (NRI) across two Zendesk tickets (Net::ReadTimeout and end of file reached variants of the same swallowed-exception path). Investigation traced the swallow-all rescue back to a 2020 fix that was addressing a different, narrower issue (persistent HTTP connection setup crashes) — the retry-suppression side effect looks unintentional and was never revisited. Filed as AGENT-16861.

Additional Notes

  • Added test coverage for both re-raise paths (test/plugin/test_out_datadog.rb).
  • Since send_retries already retries internally before this change, re-raising means a chunk can now also be retried by Fluentd core after the plugin's own retries are exhausted — i.e. two layers of retry instead of one. This trades a bit of duplicate retry effort for no longer silently dropping data, which seems like the right call, but flagging it explicitly in case reviewers want to simplify to a single retry layer instead.

(Supersedes #90, closed by an accidental branch rename.)

write(chunk) swallowed all exceptions (rescue Exception, no re-raise),
so raw errors like Net::ReadTimeout/EOFError bypassed the plugin's own
RetryableError-only retry path, and write() returning normally meant
Fluentd core's buffer retry never saw a failure either. On a transient
network blip the in-flight batch was silently dropped with no retry
from either side. send_retries had the same issue on retry exhaustion.

Fixes AGENT-16861.
@ian28223
ian28223 marked this pull request as ready for review August 26, 2026 07:44
@ian28223
ian28223 requested a review from a team as a code owner August 26, 2026 07:44
@ddrthall

Copy link
Copy Markdown
Collaborator

Incorporated in to #92, which adds the option to more aggressively retry a subset of error cases before finally raising the exception.

@ddrthall ddrthall closed this Aug 28, 2026
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.

2 participants