Skip to content

Revert "fix: replace fixed 15s WebSocket reconnect with exponential backoff + jitter"#5715

Merged
beastoin merged 1 commit intomainfrom
revert-5617-fix/issue-5527
Mar 16, 2026
Merged

Revert "fix: replace fixed 15s WebSocket reconnect with exponential backoff + jitter"#5715
beastoin merged 1 commit intomainfrom
revert-5617-fix/issue-5527

Conversation

@beastoin
Copy link
Collaborator

Reverts #5617

@beastoin beastoin merged commit a2eecf8 into main Mar 16, 2026
3 checks passed
@beastoin beastoin deleted the revert-5617-fix/issue-5527 branch March 16, 2026 08:59
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 16, 2026

Greptile Summary

This PR reverts #5617, which replaced the fixed 15-second WebSocket reconnection timer with exponential backoff + jitter. The revert restores the original Timer.periodic(15s) approach in _startKeepAliveServices(), removing the _reconnectAttempt counter, _maxBackoffSeconds cap, and _getReconnectDelay() method.

  • Restores fixed 15-second Timer.periodic reconnection with rate-limit guard in capture_provider.dart
  • Removes exponential backoff fields (_reconnectAttempt, _maxBackoffSeconds) and _getReconnectDelay() method
  • Removes _reconnectAttempt = 0 reset from onConnected() callback
  • The restored recursive call from _initiateWebsocket failure → _startKeepAliveServices() properly cancels/restarts the timer on each failure

Confidence Score: 4/5

  • This PR is a clean revert to known-working behavior with minimal risk.
  • Score of 4 reflects that this is a straightforward revert of a single file back to its previous known-working state. The restored code is logically sound — the fixed 15s timer with rate limiting is a simpler and more predictable reconnection strategy. The only reason it's not a 5 is that the PR description doesn't explain the motivation for the revert, and the fixed-interval approach may cause more aggressive reconnection load under sustained outages compared to the reverted exponential backoff.
  • No files require special attention — this is a clean revert to previously-shipped code.

Important Files Changed

Filename Overview
app/lib/providers/capture_provider.dart Reverts exponential backoff + jitter WebSocket reconnection back to fixed 15-second periodic timer. Clean revert with no logical issues; removes _reconnectAttempt, _maxBackoffSeconds, and _getReconnectDelay(), restoring Timer.periodic with rate limiting.

Sequence Diagram

sequenceDiagram
    participant WS as WebSocket
    participant CP as CaptureProvider
    participant Timer as Timer.periodic(15s)

    WS->>CP: onClosed() / onError()
    CP->>Timer: _startKeepAliveServices()
    Note over Timer: Cancel existing timer, start new 15s periodic

    loop Every 15 seconds
        Timer->>CP: Timer fires
        CP->>CP: Check rate limit (1/15s)
        CP->>CP: Check socket connected?
        alt Socket connected or device not ready
            CP->>Timer: Cancel timer
        else User not signed in
            CP->>Timer: Cancel timer
        else Device available
            CP->>WS: _initiateWebsocket(codec, source)
            alt Socket created successfully
                WS->>CP: onConnected()
                CP->>Timer: Next tick cancels (connected)
            else Socket creation failed
                CP->>Timer: _startKeepAliveServices() (restart timer)
            end
        end
    end
Loading

Last reviewed commit: 8ed4791

Comment on lines +1327 to +1331
_keepAliveTimer = Timer.periodic(const Duration(seconds: 15), (t) async {
Logger.debug("[Provider] keep alive");
// rate 1/15s
if (_keepAliveLastExecutedAt != null &&
DateTime.now().subtract(const Duration(seconds: 15)).isBefore(_keepAliveLastExecutedAt!)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant rate-limit check with fixed timer

The rate-limiting guard at lines 1330-1331 checks whether 15 seconds have elapsed since the last execution, but the Timer.periodic already fires at 15-second intervals. This makes the check effectively a no-op under normal conditions. It's not harmful, but it's dead code that could be removed for clarity — or if the intent is to guard against overlapping async callbacks, consider using a simple boolean lock instead.

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