feat: RETRY-conformance contract tests for FDv1 streaming and polling - #401
Closed
tanderson-ld wants to merge 6 commits into
Closed
feat: RETRY-conformance contract tests for FDv1 streaming and polling#401tanderson-ld wants to merge 6 commits into
tanderson-ld wants to merge 6 commits into
Conversation
Aligns terminology with the RETRY specification's `unexpected` failure classification. `recoverableErrors` stays as-is (the name is still accurate; SDKs do recover from these). Test names also updated to "do not retry after unexpected HTTP error" for consistency. No behavioral change. The tests still assert legacy permanent-stop behavior; subsequent commits will gate them behind absence of the retry-conformance capability.
Declares two new capabilities in servicedef/service_params.go, following the CapabilityFDv1Fallback pattern: - retry-conformance-fdv1-streaming: SDK's FDv1 streaming data source conforms to the RETRY specification (no permanent stops; extended- regime backoff on unexpected errors; TLS/cert failures classified as unexpected). - retry-conformance-fdv1-polling: same for the FDv1 polling data source, additionally with the PollInterval wait floor. Capabilities are scoped per data source so an SDK can partially adopt. An SDK declaring either capability must also honor the corresponding extended-regime timing knobs on the streaming / polling params (added in a later commit). Capability presence gates the new retry-conformance tests; capability absence gates the legacy permanent-stop tests.
The two "do not retry after unexpected HTTP error" tests describe legacy permanent-stop behavior that is being retired by the RETRY specification. Under RETRY, 401 / 403 / other-4xx responses trigger an extended-regime backoff rather than a permanent stop. These tests are now guarded so they run only when the SDK does NOT declare the retry-conformance-fdv1-streaming capability. SDKs that declare the capability will run the new "retry after unexpected HTTP error" tests instead (added in a later commit). Once every SDK reports the capability, both the gate and the legacy tests can be removed. No behavioral change for SDKs today: none report the new capability, so all continue to run the legacy tests.
Adds test-mode timing knobs on SDKConfigStreamingParams and
SDKConfigPollingParams, analogous to the existing InitialRetryDelayMS.
These allow retry-conformance tests to compress the RETRY specification's
extended regime (default 5 min -> 1 hour) into an observable window.
Streaming params get both knobs:
- ExtendedInitialDelayMS: initial delay of the extended regime.
- ResetThresholdMS: reset threshold returning to the normal regime.
Polling params get only ExtendedInitialDelayMS; polling's reset is
count-based (2 consecutive successful poll responses per the polling
spec), not time-based.
An SDK declaring CapabilityRetryConformanceFDv1{Streaming,Polling} MUST
honor the corresponding fields. SDKs that don't declare the capability
can ignore the fields; the new fields silently pass through JSON
deserialization.
Adds four new subtests to doServerSideStreamRetryTests, all guarded by CapabilityRetryConformanceFDv1Streaming being present. They assert RETRY-conformant behavior for unexpected HTTP errors (401 / 403 / other 4xx): - retry after unexpected HTTP error on initial connect: sequence of two 401/403/405 followed by a successful stream open; SDK reaches ready state and evaluates against the successful stream's data. - retry after unexpected HTTP error on reconnect: initial stream, then two errors mid-stream, then a successful second stream; SDK evaluates against the second stream's data. - enters extended-regime backoff after unexpected HTTP error: paired-bound assertion: after a 401, SDK does NOT reconnect within the normal-regime window (100 ms) AND DOES reconnect within the extended-regime window (3 s). Proves the SDK's timing is genuinely extended-regime, not normal-regime and not permanent stop. - does not permanently stop under sustained unexpected HTTP errors: endpoint returns 401 forever; SDK still making connection attempts after several observations. Supporting infrastructure: - retryConformanceStreamConfig helper sets InitialRetryDelayMS = 1 ms and ExtendedInitialDelayMS = 500 ms. - compressedExtendedInitialDelay constant documents the choice of 500 ms as a discriminating window. - extendedRegimeConnectionTimeout (3 s) generously bounds the wait for an extended-regime reconnect. These tests skip against every SDK that does not report the capability; they exercise real behavior only against SDKs that have implemented RETRY-conforming streaming.
Adds a new file sdktests/server_side_poll_retry.go with a first-pass polling retry-conformance test. Guarded by CapabilityRetryConformanceFDv1Polling. Marked LongRunning because server-side polling enforces a PollInterval minimum of 30 seconds; the extended-regime effective initial delay is max(ExtendedInitialDelayMS, PollInterval), so tests take ~30-60 seconds per iteration. Follows the same pattern as PollingIntervalTests. This first pass covers only the core "no permanent stop" guarantee: after an unexpected HTTP error (401 / 403 / 405), the SDK makes another poll request and eventually evaluates against the successful poll's data. Additional polling scenarios (extended-regime shape, wait-floor verification, reset after 2 consecutive successful polls) are deferred to follow-up work; they require careful attention to the LongRunning timing budget and possibly SDK-side changes to bypass the PollInterval minimum for test-mode. Wired into doServerSidePollTests via a new "retry behavior" subtest.
tanderson-ld
changed the title
4 tasks
Contributor
Author
|
Superseded by #404, which targets |
tanderson-ld
added a commit
that referenced
this pull request
Aug 19, 2026
…ling (#404) ## Summary Adds contract tests exercising the RETRY specification's non-permanent-stop guarantee for the FDv1 streaming and polling data sources. Introduces two capability gates so SDKs can adopt independently: - `retry-conformance-fdv1-streaming` - `retry-conformance-fdv1-polling` The new tests exercise the SDK's real production 5-minute extended-regime timing — no test-harness bypass, no compression knobs on the servicedef, no test-only surface in the SDK's public API. They are marked as long-running (`t.LongRunning()`) and only run when the harness is invoked with `-enable-long-running-tests`. Legacy "do not retry after unrecoverable HTTP error" scenarios remain, gated behind the ABSENCE of the streaming capability so SDKs can migrate on their own schedule. Once every SDK reports the new capabilities, both the legacy tests and the gates can be removed. ## What's added **Streaming (guarded by `retry-conformance-fdv1-streaming` + `-enable-long-running-tests`):** - retry after unexpected HTTP error on initial connect (401/403/405) - retry after unexpected HTTP error on reconnect (401/403/405) - enters extended-regime backoff after unexpected HTTP error (paired-bound timing assertion) - does not permanently stop under sustained unexpected HTTP errors - returns to normal-regime backoff after healthy operation (streaming spec's activeSince reset: 75 s silent stream after an extended-regime reconnect, then next reconnect must be at normal-regime timing) **Polling (guarded by `retry-conformance-fdv1-polling` + `-enable-long-running-tests`):** - retry after unexpected HTTP error on initial poll (401/403/405) - returns to normal-regime cadence after two consecutive successful polls (RETRY §1.8.1 count-based reset) ## Design: long-running instead of compressed timings An earlier revision of this PR exposed extended-regime timing knobs on the servicedef (`ExtendedInitialDelayMS`, `ResetThresholdMS`) so tests could compress the 5-minute extended-initial delay into a ~30-second observable window. Reviewers pushed back because that shape forced SDKs to expose corresponding test-only knobs on their public API surface just to be testable by the harness. This revision drops all compression knobs and exercises real production timing instead. That makes each retry-conformance subtest take several minutes of wall clock, which is why the tests are marked `t.LongRunning()` and gated behind `-enable-long-running-tests`. Per-SDK CI schedules for the long-running suite are out of scope for this PR (planned follow-up work). Framework support for `t.LongRunning()` and the `-enable-long-running-tests` flag was ported from v3 in #428, a prerequisite that landed before this PR's refactor. ## Relationship to PR #401 Supersedes #401, which targeted `v3` and exercised FDv2 code paths — out of scope for the initial RETRY-conformance work, which is FDv1-only. This PR targets `v2` and drives the FDv1 streaming/polling paths in the SDK testservices. #401 will be closed with a pointer here. SDK-2808 ## Test plan - [x] Contract tests pass against go-server-sdk PR #429 (the reference server SDK implementation) at real production 5-minute extended-regime timing (~3-hour run with `-enable-long-running-tests`) - [x] Legacy "do not retry" tests still pass for SDKs without the new capability (verified by SDK capability check) - [x] Retry-conformance tests are correctly SKIPPED for SDKs without the capability - [x] Retry-conformance tests are correctly SKIPPED when `-enable-long-running-tests` is not set
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds contract test coverage for the RETRY specification's non-permanent-stop guarantee, applied to FDv1 streaming and polling data sources. Every SDK that adopts RETRY-conforming behavior will opt into these tests via the new capabilities; SDKs that have not yet adopted keep passing the existing legacy tests, unmodified in behavior.
Draft status: this PR can land test scaffolding, capabilities, and the tests themselves — but no SDK reports the new capabilities yet, so the new tests all skip today. The Go server SDK (reference implementation per the parent epic) will be the first SDK to opt in; that work happens separately.
Design document
The implementation follows the plan at
individual/tanderson/tickets/SDK-2808/implementation/plan.mdinsdk-scratchpad.Structure — 6 commits
refactor: rename unrecoverableErrors to unexpectedErrors— terminology alignment with the RETRY specification. No behavior change.feat: add retry-conformance-fdv1-{streaming,polling} capabilities— two new capabilities inservicedef/service_params.go, following theCapabilityFDv1Fallbackpattern.refactor: guard legacy "do not retry" tests behind capability absence— old permanent-stop tests now skip on SDKs that declare the RETRY-conformance capability. No SDK declares it today, so no test coverage changes.feat: add ExtendedInitialDelayMS and ResetThresholdMS servicedef fields— test-mode timing knobs analogous to the existingInitialRetryDelayMS. Streaming gets both; polling gets onlyExtendedInitialDelayMS(polling's reset is count-based).test: server-side streaming retry-conformance scenarios— 4 new subtests: retry after 401/403/405 on initial connect, retry on reconnect, extended-regime engagement (paired-bound assertion), and sustained-failure no-permanent-stop.test: server-side polling retry-conformance scenarios— 1 new subtest (functional recovery after 401/403/405), markedLongRunningbecause polling's PollInterval minimum forces 30 s+ per iteration.What's NOT in this PR
Capability gating
Each new test skips unless the SDK declares the corresponding capability:
retry-conformance-fdv1-streaming— SDK's FDv1 streaming data source is RETRY-conformant AND honorsExtendedInitialDelayMS/ResetThresholdMSonSDKConfigStreamingParams.retry-conformance-fdv1-polling— SDK's FDv1 polling data source is RETRY-conformant AND honorsExtendedInitialDelayMSonSDKConfigPollingParams.Legacy "do not retry after unexpected HTTP error" tests skip when the corresponding capability IS present (mirror gating). Once every SDK reports the capability, both the gate and the legacy tests can be decommissioned.
Validation
go build ./...— clean.go vet ./...— clean.go test -c ./sdktests -o /dev/null— clean.Test plan
LongRunningtagging on the polling test is the right pattern (or suggests an alternative).