-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Add ping and poll pattern support for Relay Proxy compatibility #234
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Add ping and poll pattern support for Relay Proxy compatibility #234
Conversation
This implementation adds support for the ping and poll pattern to the Flutter SDK's streaming data source, enabling compatibility with the LaunchDarkly Relay Proxy. This is needed for customer POVs that require Relay Proxy support. Key features: - Async polling triggered by ping events (improves on Android's synchronous approach) - In-order poll processing with generation counter (prevents out-of-order updates vs iOS) - Retry mechanism with exponential backoff for failed polls (adds resilience not in other mobile SDKs) - ETag support for efficient polling (304 Not Modified responses) - Proper error handling and cleanup Changes: - Added 'ping' event type to SSEClient subscription - Added polling infrastructure with HttpClient, URIs, and request methods - Added poll generation counter to prevent race conditions - Added retry logic with Backoff utility from event_source_client - Added ETag support for conditional requests - Updated stop() method to properly clean up polling state Implementation by: John Winstead Integrated by: Devin AI Co-Authored-By: [email protected] <[email protected]>
|
Prompt hidden (unlisted session) |
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
…ce leak - Add close() method to HttpClient class that closes underlying http.Client - Call _pollingClient.close() in StreamingDataSource.stop() method - Fixes dart analyzer error about unclosed HttpClient resource Addresses Bugbot finding about resource leak. Co-Authored-By: [email protected] <[email protected]>
…error - Add Backoff to public exports in launchdarkly_event_source_client.dart - Update import in streaming_data_source.dart to use public API instead of internal /src path - Fixes dart analyzer implementation_imports violation that was causing CI failures Co-Authored-By: [email protected] <[email protected]>
- Remove redundant import since Backoff is now available through public export - Resolves dart analyzer unnecessary_import warning - Completes the fix for exporting Backoff publicly Co-Authored-By: [email protected] <[email protected]>
- Create shared DataSourceRequestor class for HTTP request handling - Replace pollGeneration counter with request chain tracking - Refactor PollingDataSource to use requestor - Refactor StreamingDataSource to use requestor for ping-triggered polling - Consolidate duplicate request/response handling code This provides better request lifecycle management by tracking request chains instead of generation counters, automatically discarding responses from stale request chains. Co-Authored-By: [email protected] <[email protected]>
- Add missing data_source.dart import to DataSourceRequestor - Add back _getEnvironmentIdFromHeaders method to StreamingDataSource - Remove unused imports from PollingDataSource (http, credential_type, default_config, get_environment_id) - Remove unused imports from StreamingDataSource (http) - Remove unused _credential field from PollingDataSource These fixes address all 12 analyzer issues found in CI: - 4 errors (undefined class/methods) - 8 warnings (unused imports/fields) Co-Authored-By: [email protected] <[email protected]>
…onstructor The _credential field was removed but the constructor initializer list still referenced it, causing a compilation error. Co-Authored-By: [email protected] <[email protected]>
…ne limit Split long lines at line 224 (logger.error call) and line 249 (_buildUri call) to meet Dart's 80-character line length requirement. Co-Authored-By: [email protected] <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Incorrect Delay Calculation Neglects Elapsed Time
Incorrect delay calculation: The max() function compares (_pollingInterval.inMilliseconds - timeSincePoll.inMilliseconds) with _pollingInterval.inMilliseconds. Since timeSincePoll is always positive (time elapsed during the poll), the first argument will always be less than the second, so max() will always return _pollingInterval.inMilliseconds. This means the delay will always be the full polling interval, completely ignoring the time already spent polling. The correct logic should use max(0, _pollingInterval.inMilliseconds - timeSincePoll.inMilliseconds) to ensure the delay is never negative while properly accounting for elapsed time.
packages/common_client/lib/src/data_sources/polling_data_source.dart#L168-L171
flutter-client-sdk/packages/common_client/lib/src/data_sources/polling_data_source.dart
Lines 168 to 171 in ddf87fc
| // we want to poll after 25 seconds. | |
| final delay = Duration( | |
| milliseconds: max( | |
| _pollingInterval.inMilliseconds - timeSincePoll.inMilliseconds, |
Bug: Resource leak: close HTTP client on stop requested
Resource leak: The HTTP client _client is never closed when the polling data source stops. Unlike the streaming data source which properly closes _pollingClient in its stop() method (line 158 in streaming_data_source.dart), the polling data source's stop() method doesn't close the client, leading to a resource leak. The stop() method should call _client.close() to properly clean up resources.
packages/common_client/lib/src/data_sources/polling_data_source.dart#L188-L193
flutter-client-sdk/packages/common_client/lib/src/data_sources/polling_data_source.dart
Lines 188 to 193 in ddf87fc
| void restart() { | |
| // For polling there is no persistent connection, so this function | |
| // has no effect. | |
| } | |
| @override |
Summary
Adds ping and poll pattern support to the Flutter SDK's streaming data source, enabling compatibility with the LaunchDarkly Relay Proxy. This implementation was requested for a customer POV and differs from iOS/Android SDKs by using async polling with in-order processing and retry mechanism.
Link to Devin run: https://app.devin.ai/sessions/566c53f955f743ed96eb59b2a22f564e
Requested by: Ryan Lamb ([email protected])
Original implementation by: John Winstead
Key Changes
'ping'event type to SSEClient subscriptionImplementation Highlights
Async polling with in-order processing:
_pollGeneration) that increments with each pingRetry mechanism:
Backoffutility from event_source_clientETag optimization:
If-None-Matchheader on subsequent pollsDifferences from iOS/Android SDKs
Per JW's notes:
Human Review Checklist
Please carefully review:
Generation counter logic (
_pollWithRetry,_isValidGeneration):stop()sufficient to cancel in-flight polls?Retry and backoff behavior (
_pollWithRetry,_waitForBackoff):Error handling (
_handlePollingError):ETag state management (
_lastEtag,_buildPollingHeaders,_updateEtagFromResponse):Resource cleanup (
stop()):_pollActiveSincereset sufficient?Integration with streaming:
Requirements
Related Issues
This enables Relay Proxy compatibility for Flutter SDK, addressing customer POV requirements where ping streams are needed.
Testing Plan
Recommended testing:
Additional Context
The Flutter SDK previously only supported streaming with
put,patch, anddeleteevents. The Relay Proxy uses ping events to signal when clients should poll for updates. This implementation adds that missing capability while improving upon the patterns used in iOS and Android SDKs.Note
Adds ping-triggered polling with ETag and retry/backoff to streaming, refactors polling to a reusable requestor, and exposes Backoff/utilities needed.
streaming_data_source.dart):pingSSE events; trigger async poll requests usingHttpClient+DataSourceRequestor.Backoff).polling_data_source.dart):DataSourceRequestorfor requests, ETag, and response handling.data_source_requestor.dart):http_client.dart):close()to free underlying client resources.'ping'in subscribed SSE event types.Backoffand use it inHtmlSseClientfor reconnect/backoff.Written by Cursor Bugbot for commit ddf87fc. This will update automatically on new commits. Configure here.