chore: add useWithRetry hook by BrunoQuaresma · Pull Request #18699 · coder/coder · GitHub | Latest TMZ Celebrity News & Gossip | Watch TMZ Live
Skip to content

chore: add useWithRetry hook #18699

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

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

BrunoQuaresma
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma commented Jul 1, 2025

  • Add useWithRetry hook with simplified interface (call, retryAt, isLoading)
  • Implement exponential backoff with configurable options
  • Include comprehensive tests covering all retry scenarios
  • Remove old useRetry hook and replace with useWithRetry
  • Reset TerminalPage files to main branch state for clean implementation

useWithRetry Hook Features

  • Simple interface: Returns { call, retryAt, isLoading } for easy integration
  • Exponential backoff: Configurable initial delay, max delay, multiplier, and max attempts
  • Automatic retry scheduling: Handles retry timing and countdown updates
  • Proper cleanup: Cancels timers on unmount and when new calls are made
  • TypeScript support: Full type safety with proper interfaces
  • Test coverage: Comprehensive test suite covering all scenarios

Configuration Options

  • maxAttempts (default: 3)
  • initialDelay (default: 1000ms)
  • maxDelay (default: 8000ms)
  • multiplier (default: 2)

Usage Example

const { call: connectTerminal, isLoading, retryAt } = useWithRetry(terminal.connect)

return (
  <div>
    <Button onClick={connectTerminal} isDisabled={isLoading}>
      Connect to the terminal
    </Button>
    {retryAt && <span>Retrying in <Countdown endDate={retryAt} /></span>}
  </div>
)

This provides a cleaner, more focused approach to retry functionality that can be easily integrated into the TerminalPage component.

blink-so bot and others added 13 commits July 1, 2025 14:06
- Update ConnectionStatus type: replace 'initializing' with 'connecting'
- Create useRetry hook with exponential backoff logic
- Add comprehensive tests for useRetry hook
- Export useRetry from hooks index

Implements:
- Initial delay: 1 second
- Max delay: 30 seconds
- Backoff multiplier: 2
- Max retry attempts: 10

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Fix startRetrying to immediately perform first retry
- Adjust retry scheduling conditions
- Fix delay calculation for exponential backoff

Still debugging test failures
- Fix attemptCount to represent attempts started, not completed
- Fix exponential backoff delay calculation
- Fix retry scheduling conditions for proper max attempts handling
- All 10 useRetry tests now pass
- No regressions in existing test suite

Implements correct behavior:
- attemptCount increments when retry starts
- Exponential backoff: 1s, 2s, 4s, 8s, 16s, 30s (capped)
- Respects maxAttempts limit
- Manual retry cancels automatic retries
- State resets properly on success

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Add parentheses around arrow function parameter
- Fix indentation

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Replace setTimeout/setInterval with window.setTimeout/window.setInterval
- Replace clearTimeout/clearInterval with window.clearTimeout/window.clearInterval
- Fixes TypeScript error: Type 'Timeout' is not assignable to type 'number'
- Ensures proper browser environment timer types

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
Convert useRetry hook from multiple useState calls to a single useReducer
for cleaner state management. This improves code clarity and makes state
transitions more predictable.

Changes:
- Replace 5 useState calls with single useReducer
- Add RetryState interface and RetryAction union type
- Implement retryReducer function for all state transitions
- Update all state access to use state object
- Replace setState calls with dispatch calls throughout

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Create TerminalRetryConnection component with countdown display and retry button
- Add comprehensive Storybook stories covering all retry states
- Integrate component with TerminalAlerts for proper positioning
- Use consistent TerminalAlert styling for seamless integration
- Ensure proper resize handling through existing MutationObserver

Implements Phase 2 of terminal reconnection feature as outlined in:
coder/internal#659

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Change 'import type { FC }' to 'import { type FC }' to match existing patterns
- Verified with prettier that formatting is correct

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
…ectedAlert

- Simplify TerminalRetryConnection to show only retry status and button
- Keep existing DisconnectedAlert but add TerminalRetryConnection in actions
- Use TailwindCSS classes instead of CSS-in-JS
- Place retry component on the right side of the alert
- Make retry messages smaller and more concise
- Fix console.log usage in Storybook stories

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Remove RefreshSessionButton from DisconnectedAlert actions
- Keep only TerminalRetryConnection component in the actions area
- Simplify the actions layout by removing the wrapper div

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Integrate useRetry hook into TerminalPage component
- Add automatic retry trigger on disconnection
- Implement terminal content preservation during reconnection
- Update WebSocket connection logic to support retry mechanism
- Pass retry props to TerminalAlerts component for UI display
- Maintain existing terminal event handlers for data and resize
- Use exponential backoff with 1s initial delay, 30s max, 10 attempts

Completes Phase 3 of terminal reconnection feature as outlined in:
coder/internal#659

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
blink-so bot and others added 6 commits July 1, 2025 18:48
- Add WEBSOCKET_CLOSE_NORMAL constant (1000) for normal closure
- Replace magic number 1000 with descriptive constant name
- Improves code readability and maintainability

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
…Retry

- Add enabled prop to useRetry hook interface
- Remove startRetrying and stopRetrying methods from hook return
- Update TerminalPage to use enabled prop based on connection status
- Simplify retry logic with declarative enabled state
- Update tests to use enabled prop instead of manual start/stop calls
- Fix WebSocket close magic number with named constant

This makes the retry behavior more React-like and easier to reason about.
The hook now automatically starts/stops retrying based on the enabled prop.

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
The issue was that when a retry succeeded, the state was reset to initial
state (attemptCount: 0), which triggered the useEffect to start retrying
again, creating an infinite loop.

Fixed by adding a hasStartedRef to track whether we've already started
the initial retry, preventing the hook from restarting after success.

Also removed onRetryEvent from performRetry dependency array since it's
created with useEffectEvent and should be stable.

Fixes the timeout in the 'should reset state when retry succeeds' test.

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
The onRetryEvent is created with useEffectEvent and is stable across renders,
so it doesn't need to be in the dependency array. Added a suppression comment
to explain this to the linter.

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
… comment

Since onRetryEvent is created with useEffectEvent, it's stable across renders
and can be safely included in the dependency array without causing issues.
This is cleaner than using a linter suppression comment.

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
- Remove unnecessary startTimeRef and simplify timer management
- Consolidate reducer return statements for cleaner code
- Split complex useEffect into separate focused effects
- Remove redundant comments and simplify countdown logic
- Maintain same API and behavior while improving readability

Co-authored-by: BrunoQuaresma <3165839+BrunoQuaresma@users.noreply.github.com>
@BrunoQuaresma BrunoQuaresma changed the title feat: implement terminal reconnection Phase 3 - integrate retry logic feat: integrate retry logic Phase 3 Jul 1, 2025
@BrunoQuaresma BrunoQuaresma requested review from code-asher and a team July 1, 2025 20:03
@BrunoQuaresma
Copy link
Collaborator Author

@code-asher I also would appreciate a QA on this.

Copy link
Contributor

blink-so bot commented Jul 1, 2025

🧪 QA Testing Guide: Terminal Reconnection Feature

This guide will help you test the new terminal reconnection functionality with automatic retry logic.

🎯 What to Test

The terminal should now automatically attempt to reconnect when the WebSocket connection is lost, with exponential backoff retry logic and preserved terminal content.

🔧 Test Setup

  1. Start a workspace with a running agent
  2. Open the terminal in the Coder web UI
  3. Run some commands to generate terminal content (e.g., ls, pwd, echo "test")

📋 Test Scenarios

✅ Scenario 1: Network Interruption Simulation

Steps:

  1. Open browser DevTools → Network tab
  2. In the terminal, run a long-running command: ping google.com or tail -f /var/log/syslog
  3. In DevTools, enable "Offline" mode or throttle to "Offline"
  4. Wait 5-10 seconds
  5. Disable offline mode

Expected Behavior:

  • Terminal shows disconnected state
  • Retry attempts begin automatically (check for retry indicators in UI)
  • Terminal reconnects and resumes showing output
  • Previous terminal content is preserved (you should still see earlier commands)
  • No terminal clearing occurs during reconnection

✅ Scenario 2: Workspace Agent Restart

Steps:

  1. SSH into your workspace or use another terminal
  2. Restart the Coder agent: sudo systemctl restart coder-agent (or equivalent)
  3. Observe the web terminal behavior

Expected Behavior:

  • Terminal detects disconnection
  • Automatic retry attempts start
  • Terminal reconnects once agent is back online
  • Terminal content remains intact

✅ Scenario 3: Manual Retry Testing

Steps:

  1. Simulate a disconnection (using DevTools offline mode)
  2. Look for manual retry button/option in the UI
  3. Click manual retry while automatic retries are in progress

Expected Behavior:

  • Manual retry should work immediately
  • Should cancel any scheduled automatic retry
  • Connection should be re-established

✅ Scenario 4: Retry Exhaustion

Steps:

  1. Stop the workspace or agent completely
  2. Wait for all retry attempts to be exhausted (should be 10 attempts)
  3. Observe final state

Expected Behavior:

  • Retries should follow exponential backoff (1s, 2s, 4s, 8s, 16s, 30s, 30s...)
  • After 10 failed attempts, retries should stop
  • UI should indicate connection failed
  • Terminal content should still be preserved

✅ Scenario 5: Successful Mid-Retry Reconnection

Steps:

  1. Simulate disconnection
  2. Let 2-3 retry attempts fail
  3. Restore connectivity before all retries are exhausted

Expected Behavior:

  • Connection should be re-established on next retry attempt
  • Retry counter should reset
  • Terminal should resume normal operation

🔍 What to Look For

✅ UI Indicators

  • Connection status indicators (connected/disconnected/retrying)
  • Retry attempt counter or progress
  • Time until next retry countdown
  • Manual retry button availability

✅ Terminal Behavior

  • Content preservation: Previous commands and output remain visible
  • No clearing: Terminal doesn't clear during reconnection
  • Input handling: Terminal input is disabled during disconnection
  • Resume functionality: Can type and execute commands after reconnection

✅ Retry Logic

  • Automatic start: Retries begin immediately on disconnection
  • Exponential backoff: Delays increase (1s → 2s → 4s → 8s → 16s → 30s max)
  • Attempt limit: Stops after 10 attempts
  • Manual override: Manual retry works and cancels automatic retries

🐛 Common Issues to Watch For

  • Terminal clearing: Content should NEVER be cleared during reconnection
  • Infinite retries: Should stop after 10 attempts
  • UI freezing: Interface should remain responsive during retries
  • Memory leaks: Check for timer cleanup (DevTools → Performance)
  • Double connections: Ensure old connections are properly closed

🌐 Browser Testing

Test across different browsers:

  • Chrome/Chromium
  • Firefox
  • Safari (if available)
  • Edge

📱 Network Conditions

Test with various network conditions:

  • Stable connection
  • Slow 3G simulation
  • Intermittent connectivity
  • Complete offline periods

🎉 Success Criteria

The feature is working correctly if:

  1. Terminal automatically reconnects after network issues
  2. All previous terminal content is preserved
  3. Retry logic follows exponential backoff pattern
  4. Manual retry option is available and functional
  5. UI provides clear feedback about connection status
  6. No memory leaks or performance issues
  7. Works consistently across different browsers

Questions or Issues? Please comment below with:

  • Browser and version
  • Steps to reproduce any problems
  • Screenshots/videos if helpful
  • Console errors (if any)

Happy testing! 🚀

@blink-so blink-so bot changed the title feat: integrate retry logic Phase 3 chore: add useWithRetry hook Jul 2, 2025
@blink-so blink-so bot changed the base branch from feature/terminal-reconnection-phase2 to main July 2, 2025 13:48
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

TMZ Celebrity News – Breaking Stories, Videos & Gossip

Looking for the latest TMZ celebrity news? You've come to the right place. From shocking Hollywood scandals to exclusive videos, TMZ delivers it all in real time.

Whether it’s a red carpet slip-up, a viral paparazzi moment, or a legal drama involving your favorite stars, TMZ news is always first to break the story. Stay in the loop with daily updates, insider tips, and jaw-dropping photos.

🎥 Watch TMZ Live

TMZ Live brings you daily celebrity news and interviews straight from the TMZ newsroom. Don’t miss a beat—watch now and see what’s trending in Hollywood.