feat: add Initializer and Synchronizer source contracts #259
Merged
kinyoklion merged 10 commits intomainfrom Apr 29, 2026
Merged
feat: add Initializer and Synchronizer source contracts #259kinyoklion merged 10 commits intomainfrom
kinyoklion merged 10 commits intomainfrom
Conversation
Adds source.dart with the Initializer/Synchronizer interfaces and the SelectorGetter/PingHandler typedefs. These are the contracts for FDv2 data sources: Initializer for one-shot results, Synchronizer for streaming results, SelectorGetter for lazy selector reads, and PingHandler for legacy ping-driven polls. Concrete implementations follow in subsequent commits.
Adds the HTTP layer for FDv2 polling: - endpoints.dart: FDv2 polling and streaming path constants, uniform across mobile and browser. - requestor.dart: FDv2Requestor — pure HTTP. Builds GET URLs with the encoded context in the path or POST URLs with the context JSON in the body. Adds basis (when the selector is non-empty) and withReasons query params. Tracks ETag across requests via if-none-match. Returns a RequestorResponse record. - polling_base.dart: FDv2PollingBase — wraps the requestor with FDv2 protocol semantics. Network errors produce interrupted; the x-ld-fd-fallback header produces terminalError with fdv1Fallback set; 304 produces a none-type ChangeSetResult; recoverable 4xx/5xx produce interrupted; non-recoverable 4xx produce terminalError; 200 bodies are parsed as FDv2EventsCollection and run through a fresh FDv2ProtocolHandler. No data source integration yet — that follows in SDK-2184.
Fixes from review of PR #259: - pollOnce now honors its 'never throws' contract: widen the try/catch in _parseBody to cover the FDv2EventsCollection.fromJson cast and per-event fromJson casts. Malformed event shapes (non-Map elements in events/payloads, wrong-type object fields) return interrupted instead of propagating a TypeError. - ETag is now persisted only on 200. A 5xx with an ETag could otherwise poison the next request and cause a follow-up 304 to be misclassified as 'cache is current' even when the SDK has no successful payload. A 304 leaves the existing ETag alone (it confirms the stored value). - URL building uses Uri parsing instead of string concatenation, so custom polling URLs with embedded query parameters (e.g. a relay proxy with a token) are preserved correctly. Our query params merge with the base URL's. - Network exception messages no longer echo into the public StatusResult.message. They are categorized into fixed strings (Network/TLS/Timeout); the full err.toString() stays in the warn log only. SocketException, TlsException, and HandshakeException details (remote IP, cert CN, OS error codes) no longer surface on dataSourceStatus.lastError.message. - x-ld-fd-fallback header is matched case-insensitively. Doc notes that the header takes precedence over body and status code. - Debug log line no longer interpolates the full URL (which embeds the base64url-encoded context in GET mode). - HTTP error paths (4xx / 5xx) now log warn/error like the network and JSON parse paths do. - FDv2Endpoints.streaming doc now reflects that the constant serves both POST and GET (via streamingGet). - FDv2Requestor doc states the serial-only contract for request(). - Defensive guard in the requestor: basis is omitted when the selector's state string is empty, even if isEmpty=false (covers Selector(state: '', version: N)). New tests cover malformed event shapes, ETag handling on 4xx/5xx and 304, custom polling URL with query parameters, basis/withReasons on POST mode, fallback header precedence over 200/304, case- insensitive header matching, intent-none on 200, heartbeat-only response, and exception-message sanitization.
- Drop the raw exception interpolation from the warn log on network errors. http.ClientException's toString() formats as 'ClientException: <msg>, uri=<full-url>', which embeds the base64url-encoded context in GET mode. Log only the sanitized category string in both warn and the public StatusResult.message. - Use 'is' checks (TimeoutException, http.ClientException) instead of substring matches against runtimeType.toString(). The previous code was dead in practice because IOClient wraps SocketException / HttpException as ClientException before they reach _describeError, and runtimeType.toString() is mangled by dart2js minification on web. The 'is' approach is minification-safe. - Reject empty-string ETags. An empty unquoted token is invalid per RFC 7232 and 'if-none-match: ' on the next request can be interpreted by some servers as 'match anything', pinning the SDK to a permanent 304. - Use queryParametersAll instead of queryParameters when reading the base URL's query so duplicate-key params like ?dup=1&dup=2 round-trip both values. - Drop server-controlled exception detail from the parse-error log; log the type name at error level and the full err+stack at debug. Capture the stack trace. - Fix misleading test comment that suggested two outcome paths existed when only one does. Tests added: warn-log content does not contain encoded context on a network-error path (the round-1 leak through a different channel), per-type pinning of _describeError (TimeoutException -> 'timed out', http.ClientException -> 'Network error', other -> 'Polling request failed'), empty-string ETag is not stored, 304-with-new-ETag does not overwrite the stored ETag, base URL with trailing slash does not produce a double-slash, base URL with duplicate-key query params preserves all values. Shared CapturingLogAdapter extracted to test/data_sources/fdv2/support/.
kinyoklion
commented
Apr 28, 2026
|
|
||
| /// Builds the streaming GET path with the base64url-encoded context | ||
| /// embedded in the URL path. | ||
| static String streamingGet(String encodedContext) => |
Member
Author
There was a problem hiding this comment.
In the FDv1 endpoints these methods included the decision about where the credential would go. That now isn't at the endpoints layer. It isn't covered in this PR, but a distinction will need to be made about when to put into a header versus when to use a query parameter. I think this a discriminator on the event source capability in JS.
Member
Author
There was a problem hiding this comment.
We will either add a "capabilities" field to the event source, or have some hadCapability(SSECapabilities.requestHeaders) method. So the event source can determine if it uses headers or query parameters.
Previously the fallback header short-circuited the response: any fallback signal returned a terminalError and discarded the body. Treat the header as an annotation instead. The body is still parsed, 304 is still treated as no-op, and HTTP errors are still classified by status code; the fdv1Fallback flag is stamped on the resulting FDv2SourceResult. The orchestrator can consume the data and transition to FDv1 in the same step rather than throwing away fresh state. Tests cover fallback alongside: a 200 with a valid payload (still ChangeSetResult, with fallback=true), a 304 (still none-type ChangeSetResult, tagged), a non-recoverable 4xx (terminalError, tagged), a recoverable 5xx (interrupted, tagged), and a malformed body (interrupted, tagged). Case-insensitive header match and 'false' header behavior are unchanged.
Use Mock implements LDLogAdapter (mocktail) for the two tests that assert log content, matching validating_persistence_test.dart, hook_runner_test.dart, env_context_modifier_test.dart, and operations_test.dart. Drop the bespoke CapturingLogAdapter and the support/ folder it lived in.
Member
Author
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 94bb233. Configure here.
Replace `if (x != null) { ... x! ... }` with
`if (x case final v?) { ... v ... }`. Case-pattern narrowing gives a
non-null local without the runtime assertion of the bang operator.
For the basis check, the case-pattern combines the null narrowing and
the non-empty guard via `when`, replacing the transitive
`basis.isNotEmpty && basis.state!.isNotEmpty` pattern with a single
`if (basis.state case final state? when state.isNotEmpty)`.
| /// | ||
| /// These paths are uniform across mobile and browser SDKs; FDv2 does | ||
| /// not distinguish between platforms at the endpoint level. | ||
| abstract final class FDv2Endpoints { |
Contributor
There was a problem hiding this comment.
Why is this abstract?
Member
Author
There was a problem hiding this comment.
In this situation it is basically like static, but there isn't a static class modifier.
So this is abstract, and cannot be instantiated, and final so it cannot be inherited from.
tanderson-ld
approved these changes
Apr 29, 2026
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
2 participants
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.
BEGIN_COMMIT_OVERRIDE
feat: add Initializer and Synchronizer source contracts
feat: add FDv2 requestor and polling base
END_COMMIT_OVERRIDE
Adds the HTTP layer for FDv2 polling, plus the Initializer and Synchronizer contracts that future polling and streaming sources will implement.
The requestor is a thin HTTP wrapper: builds the URL, sends GET (context in path) or POST (context in body), tracks ETag for if-none-match, and returns a
response record. It knows nothing about the FDv2 protocol.
The polling base wraps the requestor with FDv2 semantics: classifies HTTP errors as interrupted vs terminal, treats 304 as a no-op change set, detects the
x-ld-fd-fallback header, and runs the response body through a fresh protocol handler to produce an FDv2SourceResult.
Note
Medium Risk
Adds new FDv2 polling HTTP/parse pipeline (URL construction, ETag handling, status/error classification) where mistakes could affect data freshness, retry behavior, or leak PII via logs, though changes are largely additive and covered by tests.
Overview
Introduces initial FDv2 source abstractions (
Initializer,Synchronizer, plus selector/ping typedefs) to standardize one-shot initialization and long-lived update streams.Adds an FDv2 polling implementation split into a thin HTTP layer (
FDv2Requestor) and a protocol-aware wrapper (FDv2PollingBase): supports GET (context in path) vs POST (context in body), merges custom base URLs/query params, tracksETag/if-none-match, treats304as a no-op payload, classifies recoverable vs terminal HTTP errors, parses FDv2 event collections intoFDv2SourceResult, and annotates results withx-ld-fd-fallback/x-ld-envidwhile sanitizing error/log messages to avoid leaking encoded context.Adds comprehensive unit tests for endpoint paths, request construction/ETag/query handling, response parsing/error cases, fallback behavior, and log sanitization.
Reviewed by Cursor Bugbot for commit eba8fb0. Bugbot is set up for automated code reviews on this repo. Configure here.