Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new server-sent events (SSE) API endpoint is introduced to stream Docker container statistics, with a corresponding client-side implementation in the Svelte page. The client establishes and manages an EventSource connection to receive live container stats, updating the UI reactively and handling connection lifecycle and container removal events. Changes
Sequence Diagram(s)sequenceDiagram
participant UserBrowser as User Browser
participant SveltePage as Svelte +page.svelte
participant APIServer as /api/containers/[id]/stats/stream
participant DockerClient as Docker Client
UserBrowser->>SveltePage: Activate "stats" tab
SveltePage->>APIServer: Open EventSource connection (GET /stats/stream)
APIServer->>DockerClient: Fetch container stats (every 2s)
DockerClient-->>APIServer: Return stats
APIServer-->>SveltePage: Send SSE data event (stats JSON)
SveltePage-->>UserBrowser: Update stats UI
APIServer-->>SveltePage: Send keep-alive ping (every 15s)
APIServer-->>SveltePage: Send "removed" event if container is deleted
SveltePage->>SveltePage: Close EventSource on tab change or container stop
SveltePage->>APIServer: EventSource disconnects (on close/error)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| start(controller) { | ||
| // Track if the controller is still active | ||
| let isActive = true; | ||
| let pollInterval: ReturnType<typeof setInterval>; |
Check failure
Code scanning / ESLint
Require `const` declarations for variables that are never reassigned after declared Error
| // Track if the controller is still active | ||
| let isActive = true; | ||
| let pollInterval: ReturnType<typeof setInterval>; | ||
| let pingInterval: ReturnType<typeof setInterval>; |
Check failure
Code scanning / ESLint
Require `const` declarations for variables that are never reassigned after declared Error
| } catch (err) { | ||
| if (!isActive) return; | ||
|
|
||
| if ((err as any).statusCode === 404) { |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
| controller.enqueue(encoder.encode(`data: ${JSON.stringify({ removed: true })}\n\n`)); | ||
| cleanup(); | ||
| controller.close(); | ||
| } catch (e) { |
Check failure
Code scanning / ESLint
Disallow unused variables Error
| try { | ||
| controller.enqueue(encoder.encode(':\n\n')); | ||
| } catch (err) { | ||
| if (err && typeof err === 'object' && 'code' in err && (err as any).code === 'ERR_INVALID_STATE') { |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🔭 Outside diff range comments (1)
src/routes/containers/[id]/+page.svelte (1)
738-744: 🧹 Nitpick (assertive)Don’t hard-code
eth0; choose the first available interfaceContainers can have arbitrary interface names (e.g.
eth1,enp0s3,veth…).
Selecting the first key avoids silently showing “0 B” on systems whereeth0is absent.- <div class="text-sm font-medium mt-1">{formatBytes(stats.networks?.eth0?.rx_bytes || 0)}</div> + {@const ifaces = Object.keys(stats.networks || {})} + {@const first = ifaces[0]} + <div class="text-sm font-medium mt-1"> + {formatBytes(first ? stats.networks[first]?.rx_bytes || 0 : 0)} + </div>Apply the same pattern for
tx_bytes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/routes/api/containers/[id]/stats/stream/+server.ts(1 hunks)src/routes/containers/[id]/+page.svelte(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/routes/api/containers/[id]/stats/stream/+server.ts (1)
src/lib/services/docker/core.ts (1)
getDockerClient(24-32)
🪛 Biome (1.9.4)
src/routes/api/containers/[id]/stats/stream/+server.ts
[error] 17-17: This let declares a variable that is only assigned once.
'pollInterval' is only assigned here.
(lint/style/useConst)
[error] 18-18: This let declares a variable that is only assigned once.
'pingInterval' is only assigned here.
(lint/style/useConst)
🪛 GitHub Check: ESLint
src/routes/api/containers/[id]/stats/stream/+server.ts
[failure] 17-17: Require const declarations for variables that are never reassigned after declared
'pollInterval' is never reassigned. Use 'const' instead.
[failure] 18-18: Require const declarations for variables that are never reassigned after declared
'pingInterval' is never reassigned. Use 'const' instead.
[failure] 54-54: Disallow the any type
Unexpected any. Specify a different type.
[failure] 59-59: Disallow unused variables
'e' is defined but never used.
[failure] 76-76: Disallow the any type
Unexpected any. Specify a different type.
🪛 GitHub Actions: Svelte Check
src/routes/api/containers/[id]/stats/stream/+server.ts
[error] 45-45: 'err' is of type 'unknown'. This causes a type error when accessing 'err.code'.
| export const GET: RequestHandler = async ({ params, request }) => { | ||
| const { id } = params; | ||
| const docker = getDockerClient(); | ||
|
|
||
| try { | ||
| const container = docker.getContainer(id); | ||
| const encoder = new TextEncoder(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate id parameter early to avoid empty-ID look-ups
If id is missing or an empty string, docker.getContainer(id) will return a stub that later throws a 404.
Returning a 400 response early makes the failure explicit and saves unnecessary work.
+ if (!id) {
+ throw error(400, 'Container id is required');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const GET: RequestHandler = async ({ params, request }) => { | |
| const { id } = params; | |
| const docker = getDockerClient(); | |
| try { | |
| const container = docker.getContainer(id); | |
| const encoder = new TextEncoder(); | |
| export const GET: RequestHandler = async ({ params, request }) => { | |
| const { id } = params; | |
| if (!id) { | |
| throw error(400, 'Container id is required'); | |
| } | |
| const docker = getDockerClient(); | |
| try { | |
| const container = docker.getContainer(id); | |
| const encoder = new TextEncoder(); |
🤖 Prompt for AI Agents (early access)
In src/routes/api/containers/[id]/stats/stream/+server.ts around lines 5 to 11,
the code does not validate the `id` parameter before using it to get a Docker
container, which can lead to unnecessary work and a less clear 404 error later.
Add an early check to verify that `id` is present and not an empty string; if it
is invalid, immediately return a 400 Bad Request response to explicitly indicate
the client error and avoid calling `docker.getContainer` with an empty or
missing ID.
| let pollInterval: ReturnType<typeof setInterval>; | ||
| let pingInterval: ReturnType<typeof setInterval>; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Use const for non-reassigned interval handles
pollInterval and pingInterval are never reassigned after their initial setInterval call, so const is the correct declaration and satisfies Biome / ESLint.
- let pollInterval: ReturnType<typeof setInterval>;
- let pingInterval: ReturnType<typeof setInterval>;
+ const pollInterval: ReturnType<typeof setInterval> = setInterval(async () => { /* … */ }, 2000);
+ const pingInterval: ReturnType<typeof setInterval> = setInterval(() => { /* … */ }, 15000);You can then delete the later assignments, only keeping the clearInterval calls.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 17-17: This let declares a variable that is only assigned once.
'pollInterval' is only assigned here.
(lint/style/useConst)
[error] 18-18: This let declares a variable that is only assigned once.
'pingInterval' is only assigned here.
(lint/style/useConst)
🪛 GitHub Check: ESLint
[failure] 17-17: Require const declarations for variables that are never reassigned after declared
'pollInterval' is never reassigned. Use 'const' instead.
[failure] 18-18: Require const declarations for variables that are never reassigned after declared
'pingInterval' is never reassigned. Use 'const' instead.
🤖 Prompt for AI Agents (early access)
In src/routes/api/containers/[id]/stats/stream/+server.ts around lines 17 to 18,
change the declarations of pollInterval and pingInterval from let to const since
they are assigned once and never reassigned. Remove any subsequent reassignments
to these variables, keeping only the clearInterval calls to properly clear the
intervals.
| if ((err as any).statusCode === 404) { | ||
| try { | ||
| controller.enqueue(encoder.encode(`data: ${JSON.stringify({ removed: true })}\n\n`)); | ||
| cleanup(); | ||
| controller.close(); | ||
| } catch (e) { | ||
| // Ignore errors if controller is already closed | ||
| cleanup(); | ||
| } | ||
| } else { | ||
| console.error('Container stats error:', err); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Eliminate any casts and unused variable
The (err as any).statusCode cast and unused catch variable e both violate the lint rules.
- } catch (e) {
- // Ignore errors if controller is already closed
- cleanup();
- }
+ } catch {
+ // Ignore errors if controller is already closed
+ cleanup();
+ }
…
- if ((err as any).statusCode === 404) {
+ if (typeof err === 'object' && err !== null && 'statusCode' in err && (err as { statusCode?: number }).statusCode === 404) {This keeps TypeScript strict-null-safe without resorting to any.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ((err as any).statusCode === 404) { | |
| try { | |
| controller.enqueue(encoder.encode(`data: ${JSON.stringify({ removed: true })}\n\n`)); | |
| cleanup(); | |
| controller.close(); | |
| } catch (e) { | |
| // Ignore errors if controller is already closed | |
| cleanup(); | |
| } | |
| } else { | |
| console.error('Container stats error:', err); | |
| } | |
| } | |
| if ( | |
| typeof err === 'object' && | |
| err !== null && | |
| 'statusCode' in err && | |
| (err as { statusCode?: number }).statusCode === 404 | |
| ) { | |
| try { | |
| controller.enqueue( | |
| encoder.encode(`data: ${JSON.stringify({ removed: true })}\n\n`) | |
| ); | |
| cleanup(); | |
| controller.close(); | |
| } catch { | |
| // Ignore errors if controller is already closed | |
| cleanup(); | |
| } | |
| } else { | |
| console.error('Container stats error:', err); | |
| } |
🧰 Tools
🪛 GitHub Check: ESLint
[failure] 54-54: Disallow the any type
Unexpected any. Specify a different type.
[failure] 59-59: Disallow unused variables
'e' is defined but never used.
🤖 Prompt for AI Agents (early access)
In src/routes/api/containers/[id]/stats/stream/+server.ts around lines 54 to 66,
remove the use of the `any` cast on `err` by properly typing the error or using
a type guard to safely access `statusCode`. Also, eliminate the unused catch
variable `e` by either removing it or handling it appropriately. This will
comply with lint rules and maintain strict TypeScript type safety.
| const cleanup = () => { | ||
| isActive = false; | ||
| clearInterval(pollInterval); | ||
| clearInterval(pingInterval); | ||
| }; | ||
|
|
||
| // Listen for client disconnects | ||
| request.signal.addEventListener('abort', cleanup); | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Detach the abort listener during cleanup
addEventListener adds a strong reference to cleanup.
When the client disconnects we already call cleanup; removing the listener prevents a potential retain cycle and makes the intent explicit.
- request.signal.addEventListener('abort', cleanup);
+ const { signal } = request;
+ signal.addEventListener('abort', cleanup);
…
const cleanup = () => {
isActive = false;
clearInterval(pollInterval);
clearInterval(pingInterval);
+ signal.removeEventListener('abort', cleanup);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const cleanup = () => { | |
| isActive = false; | |
| clearInterval(pollInterval); | |
| clearInterval(pingInterval); | |
| }; | |
| // Listen for client disconnects | |
| request.signal.addEventListener('abort', cleanup); | |
| const cleanup = () => { | |
| isActive = false; | |
| clearInterval(pollInterval); | |
| clearInterval(pingInterval); | |
| // Detach the abort listener to avoid retain cycles | |
| signal.removeEventListener('abort', cleanup); | |
| }; | |
| // Listen for client disconnects | |
| const { signal } = request; | |
| signal.addEventListener('abort', cleanup); |
🤖 Prompt for AI Agents (early access)
In src/routes/api/containers/[id]/stats/stream/+server.ts around lines 21 to 29,
the abort event listener added to request.signal is not removed during cleanup,
which can cause a retain cycle. Modify the cleanup function to also call
request.signal.removeEventListener('abort', cleanup) to detach the listener when
cleaning up, ensuring no lingering references remain.
| try { | ||
| controller.enqueue(encoder.encode(`data: ${JSON.stringify(stats)}\n\n`)); | ||
| } catch (err) { | ||
| // Controller is closed | ||
| if (err.code === 'ERR_INVALID_STATE') { | ||
| cleanup(); | ||
| } else { | ||
| console.error('Enqueue error:', err); |
There was a problem hiding this comment.
Type-safely detect closed stream without using err.code
controller.enqueue throws a DOMException with name === 'InvalidStateError'; accessing err.code on an unknown value fails Svelte-check.
- } catch (err) {
- // Controller is closed
- if (err.code === 'ERR_INVALID_STATE') {
+ } catch (err: unknown) {
+ if (err instanceof DOMException && err.name === 'InvalidStateError') {
cleanup();
} else {
console.error('Enqueue error:', err);
}
}This removes the type error and the need for the any cast.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| controller.enqueue(encoder.encode(`data: ${JSON.stringify(stats)}\n\n`)); | |
| } catch (err) { | |
| // Controller is closed | |
| if (err.code === 'ERR_INVALID_STATE') { | |
| cleanup(); | |
| } else { | |
| console.error('Enqueue error:', err); | |
| try { | |
| controller.enqueue(encoder.encode(`data: ${JSON.stringify(stats)}\n\n`)); | |
| } catch (err: unknown) { | |
| if (err instanceof DOMException && err.name === 'InvalidStateError') { | |
| cleanup(); | |
| } else { | |
| console.error('Enqueue error:', err); | |
| } | |
| } |
🧰 Tools
🪛 GitHub Actions: Svelte Check
[error] 45-45: 'err' is of type 'unknown'. This causes a type error when accessing 'err.code'.
🤖 Prompt for AI Agents (early access)
In src/routes/api/containers/[id]/stats/stream/+server.ts around lines 41 to 48,
the error handling for a closed stream incorrectly checks err.code, causing type
errors. Update the catch block to check if the caught error is a DOMException
and if its name property equals 'InvalidStateError' to detect a closed stream
safely without type assertions. This ensures type-safe error detection and
removes the need for any casting.
| controller.enqueue(encoder.encode(`data: ${JSON.stringify(stats)}\n\n`)); | ||
| } catch (err) { | ||
| // Controller is closed | ||
| if (err && typeof err === 'object' && 'code' in err && (err as any).code === 'ERR_INVALID_STATE') { |
Check failure
Code scanning / ESLint
Disallow the `any` type Error
1 participant
Summary by CodeRabbit