Skip to content

feat: conform FDv1 streaming and polling data sources to the RETRY spec - #347

Draft
tanderson-ld wants to merge 1 commit into
mainfrom
ta/SDK-2791/dotnet-core-retry
Draft

feat: conform FDv1 streaming and polling data sources to the RETRY spec#347
tanderson-ld wants to merge 1 commit into
mainfrom
ta/SDK-2791/dotnet-core-retry

Conversation

@tanderson-ld

Copy link
Copy Markdown
Contributor

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

The third box is deliberately unchecked. net462 is Windows-only and has not been
exercised locally; this PR's CI matrix (ubuntu-22.04, windows-latest, macos-latest)
is the first place it will run.

Important

This branch cannot compile in CI yet, by design. The source calls
IEventSource.SetTemporaryRetryDelayBounds / ClearTemporaryRetryDelayBounds, which
only exist in the unreleased LaunchDarkly.EventSource 6.0.0. The local package pin and
nuget.config used to build this work are intentionally not committed, so the
PackageReference here still reads 5.3.1. Draft until the dependency chain lands.

Related issues

Implements the RETRY specification for the FDv1 streaming and polling data sources, the
.NET counterpart of:

Where the references disagree, this follows Java, since the two implementations are
structurally close.

Describe the solution you've provided

Failures are classified as Normal or Unexpected, and an unexpected failure engages an
extended backoff regime (5 minute initial delay, 1 hour ceiling) instead of terminating
the data source.

  • Unexpected HTTP statuses such as 401, 403 and 405 no longer permanently stop a data
    source. Both sources stay Interrupted and keep retrying at extended-regime timing.
  • Streaming applies the regime through the EventSource temporary retry-delay bounds;
    sustained healthy operation clears them.
  • Polling gains PollingStrategy, carrying the attempt count and the regime, and
    reschedules each poll from the strategy rather than from a fixed interval.
  • Adds FailureClass and TaskExecutor.ScheduleTask to LaunchDarkly.InternalSdk.
  • Declares the retry-conformance-fdv1-streaming and retry-conformance-fdv1-polling
    contract-test capabilities.

Describe alternatives you've considered

Transport-failure classification. An earlier revision inspected exceptions for TLS and
certificate causes to classify them as Unexpected. That was withdrawn: the cases cannot
be recognised reliably across target frameworks, and misreading a transient fault as
unexpected costs the customer a 5-minute-to-1-hour outage, while the reverse costs some
retries. Transport failures now classify as Normal, matching the default-to-normal shape
of the spec's transport table.

Scheduling. Polling could have self-scheduled with a bare Task.Run, but keeping the
work in TaskExecutor preserves parity with Java's executor and keeps exception logging in
one place. ScheduleTask accepts a CancellationToken per BCL convention rather than
returning a source, and starts Task.Delay on the calling thread so argument validation
surfaces to the caller instead of being lost on a detached task.

Server-directed retry. Matches Java: an SSE retry: value sets the initial delay, capped
at 1 hour, and does not alter the configured maximum or the attempt count.

Additional context

Release sequencing, following the precedent set by #339 and #343:

  1. Release LaunchDarkly.InternalSdk carrying FailureClass and ScheduleTask
  2. Release LaunchDarkly.EventSource 6.0.0
  3. Bump both pins here, which is also what makes CI compile

Verification so far, on net8.0 only:

  • LaunchDarkly.InternalSdk.Tests — 222/222
  • LaunchDarkly.ServerSdk.Tests — 1644/1644 on five of six full runs

One failure occurred on the first run inside PollingDataSource.UpdateTaskAsync, has not
reproduced in nine subsequent runs, and is still being investigated; two candidate defects
have already been identified by inspection. Not ready for review until that is settled.

The harness-side retry-conformance suites are merged and released (sdk-test-harness
v2.40.0). They require -enable-long-running-tests and run at full production timing, so a
complete pass takes roughly 60-80 minutes; that run has not been performed yet. Note that
declaring the two capabilities also causes the harness to skip its legacy
permanent-stop tests, which no longer describe conformant behavior.

SDK-2791

Classify data source failures as normal or unexpected per RETRY 1.6.1 and
1.7.1, and apply an extended backoff regime (5 minute initial delay, 1 hour
ceiling) after an unexpected failure instead of terminating the data source.

- Unexpected HTTP statuses such as 401, 403 and 405 no longer permanently
  stop a data source. Both sources stay Interrupted and keep retrying at
  extended-regime timing.
- Streaming applies the regime through the EventSource temporary retry delay
  bounds; sustained healthy operation clears them.
- Polling gains PollingStrategy, which carries the attempt count and the
  regime, and reschedules each poll from the strategy rather than from a
  fixed interval.
- Transport failures classify as normal. Cases that cannot be recognised with
  confidence are deliberately treated as normal, since misreading a transient
  fault as unexpected is the more damaging error.
- Adds FailureClass and TaskExecutor.ScheduleTask to InternalSdk.
- Declares the retry-conformance-fdv1-streaming and
  retry-conformance-fdv1-polling contract-test capabilities.

Tests cover the HTTP and transport classification tables, PollingStrategy
delay progression and regime transitions, ScheduleTask timing, cancellation
and error handling, and data source lifecycle and bounds behaviour. The FDv2
data source test mocks implement the two new IEventSource members inertly,
since those sources do not use the extended-regime bounds.

Requires LaunchDarkly.EventSource 6.0.0, which is not yet released, and an
InternalSdk release carrying FailureClass and ScheduleTask before the SDK
pins can pick them up.

SDK-2791
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