connection tester#1533
Open
arnaud-robin wants to merge 2 commits into
Open
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
PR Summary by QodoAdd connection test feature (backend API + frontend page)
AI Description
Diagram
High-Level Assessment
Files changed (23)
|
Code Review by Qodo
1. Missing unmount cleanup
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+117
to
+236
| const runTest = async () => { | ||
| abortRef.current?.abort() | ||
| const controller = new AbortController() | ||
| abortRef.current = controller | ||
| const { signal } = controller | ||
|
|
||
| setIsRunning(true) | ||
| setSteps(createInitialSteps()) | ||
| stopVideoTrack() | ||
|
|
||
| try { | ||
| await runStep('browser', signal, async () => { | ||
| const browser = getBrowser() | ||
| if (!browser) throw new Error('Browser not detected') | ||
| return { | ||
| summary: `${browser.name} ${browser.version}`, | ||
| data: { | ||
| name: browser.name, | ||
| version: browser.version, | ||
| os: browser.os, | ||
| osVersion: browser.osVersion, | ||
| }, | ||
| } | ||
| }) | ||
| if (signal.aborted) return | ||
|
|
||
| const microphoneOk = await runStep('microphone', signal, async () => { | ||
| const track = await createLocalAudioTrack() | ||
| const label = | ||
| track.mediaStreamTrack.label || | ||
| track.mediaStreamTrack.getSettings().deviceId || | ||
| '' | ||
| track.stop() | ||
| return { summary: label, data: { label } } | ||
| }) | ||
| if (signal.aborted) return | ||
| if (!microphoneOk) openPermissionsDialog('audioinput') | ||
|
|
||
| const cameraOk = await runStep('camera', signal, async () => { | ||
| const track = await createLocalVideoTrack() | ||
| videoTrackRef.current = track | ||
| setVideoTrack(track) | ||
| const settings = track.mediaStreamTrack.getSettings() | ||
| const label = track.mediaStreamTrack.label || '' | ||
| return { | ||
| summary: label, | ||
| data: { | ||
| label, | ||
| width: settings.width, | ||
| height: settings.height, | ||
| }, | ||
| } | ||
| }) | ||
| if (signal.aborted) return | ||
| if (!cameraOk) openPermissionsDialog('videoinput') | ||
|
|
||
| await runStep('devices', signal, async () => { | ||
| const devices = await navigator.mediaDevices.enumerateDevices() | ||
| return { | ||
| summary: String(devices.length), | ||
| data: groupDevicesByKind(devices), | ||
| } | ||
| }) | ||
| if (signal.aborted) return | ||
|
|
||
| let checker: ConnectionCheck | ||
| try { | ||
| const { livekit } = await fetchConnectionTestToken() | ||
| checker = new ConnectionCheck(livekit.url, livekit.token) | ||
| } catch (error) { | ||
| skipSteps( | ||
| LIVEKIT_STEP_IDS, | ||
| getErrorMessage(error, 'Failed to fetch test token') | ||
| ) | ||
| return | ||
| } | ||
|
|
||
| await runStep('websocket', signal, async () => | ||
| fromCheckInfo(await checker.checkWebsocket()) | ||
| ) | ||
| await runStep('webrtc', signal, async () => | ||
| fromCheckInfo(await checker.checkWebRTC()) | ||
| ) | ||
| await runStep('turn', signal, async () => | ||
| fromCheckInfo(await checker.checkTURN()) | ||
| ) | ||
| await runStep('reconnect', signal, async () => | ||
| fromCheckInfo(await checker.checkReconnect()) | ||
| ) | ||
|
|
||
| if (!microphoneOk) { | ||
| skipSteps(['publishAudio'], 'Microphone permission required') | ||
| } else { | ||
| await runStep('publishAudio', signal, async () => | ||
| fromCheckInfo(await checker.checkPublishAudio()) | ||
| ) | ||
| } | ||
|
|
||
| if (!cameraOk) { | ||
| skipSteps(['publishVideo'], 'Camera permission required') | ||
| } else { | ||
| stopVideoTrack() | ||
| await runStep('publishVideo', signal, async () => | ||
| fromCheckInfo(await checker.checkPublishVideo()) | ||
| ) | ||
| } | ||
| } finally { | ||
| if (!signal.aborted) { | ||
| stopVideoTrack() | ||
| setIsRunning(false) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const reset = () => { | ||
| abortRef.current?.abort() | ||
| stopVideoTrack() | ||
| setSteps(createInitialSteps()) | ||
| setIsRunning(false) | ||
| } |
There was a problem hiding this comment.
1. Missing unmount cleanup 🐞 Bug ☼ Reliability
useConnectionTestRunner starts async work and may create local media tracks, but it never aborts/stops them on component unmount; navigating away mid-run can leave the camera capture running and keep background work alive. Cleanup currently only happens via reset() or the finally block when not aborted, and the ConnectionTest route never calls reset() on unmount.
Agent Prompt
### Issue description
The connection test runner can keep capturing camera/video and continue async work if the user navigates away while the test is running, because the hook does not register any unmount cleanup to abort the current run and stop tracks.
### Issue Context
- `runTest()` creates `AbortController` and can create a `LocalVideoTrack`, but there is no `useEffect(() => () => ...)` cleanup in the hook.
- Cleanup is conditional (`finally` only stops track when `!signal.aborted`) and `reset()` is not called by the route on unmount.
### Fix Focus Areas
- src/frontend/src/features/connection-test/hooks/useConnectionTestRunner.ts[59-236]
- src/frontend/src/features/connection-test/routes/ConnectionTest.tsx[71-116]
### Suggested implementation notes
- Add a `useEffect` in `useConnectionTestRunner` with a cleanup function that:
- calls `abortRef.current?.abort()`
- stops any current `videoTrackRef.current` (and clears it)
- prevents further state updates (e.g., via an `isMounted` ref or by checking `signal.aborted` plus a `didUnmount` flag).
- Alternatively (or additionally), have `ConnectionTest.tsx` destructure `reset` and call it in an unmount cleanup effect.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
124
to
139
|
|
||
| token = ( | ||
| AccessToken( | ||
| api_key=settings.LIVEKIT_CONFIGURATION["api_key"], | ||
| api_secret=settings.LIVEKIT_CONFIGURATION["api_secret"], | ||
| ) | ||
| api_key=settings.LIVEKIT_CONFIGURATION["api_key"], | ||
| api_secret=settings.LIVEKIT_CONFIGURATION["api_secret"], | ||
| ) | ||
| .with_grants(video_grants) | ||
| .with_identity(identity) | ||
| .with_name(display_name) | ||
| .with_attributes( | ||
| {"color": color, "room_admin": "true" if is_admin_or_owner else "false"} | ||
| ) | ||
| ) | ||
| if ttl is not None: | ||
| token = token.with_ttl(ttl) | ||
|
|
There was a problem hiding this comment.
2. Ruff checks will fail 🐞 Bug ⚙ Maintainability
Backend changes introduce Ruff format/isort violations (notably in core.utils.generate_token indentation and standard-library import ordering, plus import sectioning in the new connection_test module), which will fail the CI steps that run ruff format . --diff and ruff check .. This blocks merge because the backend workflow enforces Ruff formatting and linting.
Agent Prompt
### Issue description
Some backend Python changes are not compliant with the repository's enforced Ruff formatter and Ruff isort rules (import ordering/sectioning and formatting/indentation). CI runs Ruff formatting in diff mode and Ruff lint checks, so these files will fail.
### Issue Context
The backend GitHub workflow runs:
- `ruff format . --diff`
- `ruff check .`
### Fix Focus Areas
- src/backend/core/utils.py[9-18]
- src/backend/core/utils.py[125-139]
- src/backend/core/api/connection_test.py[3-15]
- src/backend/core/tests/test_api_connection_test.py[3-12]
- .github/workflows/meet.yml[113-136]
### Suggested implementation notes
- Run `ruff format` on `src/backend` and commit the result.
- Fix import ordering/sectioning to satisfy `ruff check` (isort rules), e.g.:
- ensure standard-library `from ... import ...` lines are ordered correctly (`datetime` vs `functools`),
- separate `django` imports from other third-party imports with a blank line as Ruff/isort expects.
- After edits, re-run `ruff format` and `ruff check` locally to confirm clean output.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Currently users have no way to reliably test their connection before joining a room. We then want to create a connection test page to adress this issue. The testing will require a dedicated LiveKit token without going through the room API, which is tied to registered meetings, lobby rules, and longer-lived access tokens. We introduce a new API endpoint GET /api/v1.0/connection-test/ to issue a dedicated token for diagnostics, even for anonymous users. Each request creates a new room so users never share the same LiveKit room during tests. Tokens are short-lived (default 10 minutes) to limit reuse, and the endpoint is throttled to prevent abuse.
Introduce a new connection test page to allow users to verify their device and network compatibility with the application. The feature also supports generating and downloading a detailed report of the test results.
arnaud-robin
force-pushed
the
connection-tester
branch
from
July 24, 2026 21:57
e092cea to
681a8b1
Compare
|
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.
Purpose
Users currently have no reliable way to check browser, media devices, and LiveKit connectivity before joining a room. Support also lacks a simple diagnostic report when people hit network issues (e.g.
could not establish pc connection).Proposal
Add a connection test page that runs local device checks plus LiveKit
ConnectionCheck(WebSocket, WebRTC, TURN, publish audio/video, etc.), backed by a throttledGET /api/v1.0/connection-test/endpoint that issues a short-lived token for an ephemeral room (including for anonymous users). When the run finishes, users can download a JSON report of the results.