diff --git a/.vscode/settings.json b/.vscode/settings.json index 7fef4af975..6057824039 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -54,6 +54,7 @@ } }, + "tailwindCSS.classFunctions": ["cva", "cn"], "[css][html][markdown][yaml]": { "editor.defaultFormatter": "esbenp.prettier-vscode" }, diff --git a/site/src/components/Alert/ErrorAlert.tsx b/site/src/components/Alert/ErrorAlert.tsx index 0198ea4e99..2a8da27e03 100644 --- a/site/src/components/Alert/ErrorAlert.tsx +++ b/site/src/components/Alert/ErrorAlert.tsx @@ -4,9 +4,11 @@ import type { FC } from "react"; import { Link } from "../Link/Link"; import { Alert, AlertDetail, type AlertProps } from "./Alert"; -export const ErrorAlert: FC< +type ErrorAlertProps = Readonly< Omit & { error: unknown } -> = ({ error, ...alertProps }) => { +>; + +export const ErrorAlert: FC = ({ error, ...alertProps }) => { const message = getErrorMessage(error, "Something went wrong."); const detail = getErrorDetail(error); const status = getErrorStatus(error); diff --git a/site/src/components/Badge/Badge.stories.tsx b/site/src/components/Badge/Badge.stories.tsx index 524d0e3642..9754262742 100644 --- a/site/src/components/Badge/Badge.stories.tsx +++ b/site/src/components/Badge/Badge.stories.tsx @@ -21,6 +21,24 @@ export const Warning: Story = { }, }; +export const Destructive: Story = { + args: { + variant: "destructive", + }, +}; + +export const Info: Story = { + args: { + variant: "info", + }, +}; + +export const Green: Story = { + args: { + variant: "green", + }, +}; + export const SmallWithIcon: Story = { args: { variant: "default", diff --git a/site/src/components/Badge/Badge.tsx b/site/src/components/Badge/Badge.tsx index c3d0b27475..ca6a08eb60 100644 --- a/site/src/components/Badge/Badge.tsx +++ b/site/src/components/Badge/Badge.tsx @@ -23,8 +23,8 @@ const badgeVariants = cva( destructive: "border border-solid border-border-destructive bg-surface-red text-highlight-red shadow", green: - "border border-solid border-surface-green bg-surface-green text-highlight-green shadow", - info: "border border-solid border-surface-sky bg-surface-sky text-highlight-sky shadow", + "border border-solid border-border-green bg-surface-green text-highlight-green shadow", + info: "border border-solid border-border-sky bg-surface-sky text-highlight-sky shadow", }, size: { xs: "text-2xs font-regular h-5 [&_svg]:hidden rounded px-1.5", @@ -50,7 +50,7 @@ const badgeVariants = cva( defaultVariants: { variant: "default", size: "md", - border: "solid", + border: "none", hover: false, }, }, diff --git a/site/src/components/Latency/Latency.stories.tsx b/site/src/components/Latency/Latency.stories.tsx index 370873c056..9fb4a011c6 100644 --- a/site/src/components/Latency/Latency.stories.tsx +++ b/site/src/components/Latency/Latency.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { screen, userEvent, within } from "storybook/test"; import { Latency } from "./Latency"; const meta: Meta = { @@ -32,3 +33,19 @@ export const Loading: Story = { isLoading: true, }, }; + +export const NoLatency: Story = { + args: { + latency: undefined, + }, + + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const tooltipTrigger = canvas.getByLabelText(/Latency not available/i); + await userEvent.hover(tooltipTrigger); + + // Need to await getting the tooltip because the tooltip doesn't open + // immediately on hover + await screen.findByRole("tooltip", { name: /Latency not available/i }); + }, +}; diff --git a/site/src/components/Latency/Latency.tsx b/site/src/components/Latency/Latency.tsx index 136d79bb5b..84292616bc 100644 --- a/site/src/components/Latency/Latency.tsx +++ b/site/src/components/Latency/Latency.tsx @@ -1,22 +1,24 @@ import { useTheme } from "@emotion/react"; import CircularProgress from "@mui/material/CircularProgress"; import Tooltip from "@mui/material/Tooltip"; -import { visuallyHidden } from "@mui/utils"; import { Abbr } from "components/Abbr/Abbr"; import { CircleHelpIcon } from "lucide-react"; import type { FC } from "react"; +import { cn } from "utils/cn"; import { getLatencyColor } from "utils/latency"; interface LatencyProps { latency?: number; isLoading?: boolean; - size?: number; + className?: string; + iconClassName?: string; } export const Latency: FC = ({ latency, isLoading, - size = 14, + className, + iconClassName, }) => { const theme = useTheme(); // Always use the no latency color for loading. @@ -24,28 +26,29 @@ export const Latency: FC = ({ if (isLoading) { return ( - - + + ); } if (!latency) { - const notAvailableText = "Latency not available"; return ( - - <> - {notAvailableText} - - - + + ); } return ( -
- Latency: +
+ Latency: {latency.toFixed(0)} ms
diff --git a/site/src/hooks/useEmbeddedMetadata.ts b/site/src/hooks/useEmbeddedMetadata.ts index f4a1d59528..64926755ec 100644 --- a/site/src/hooks/useEmbeddedMetadata.ts +++ b/site/src/hooks/useEmbeddedMetadata.ts @@ -229,13 +229,12 @@ export function makeUseEmbeddedMetadata( manager.getMetadata, ); - // biome-ignore lint/correctness/useExhaustiveDependencies(manager.clearMetadataByKey): baked into containing hook const stableMetadataResult = useMemo(() => { return { metadata, clearMetadataByKey: manager.clearMetadataByKey, }; - }, [metadata]); + }, [manager, metadata]); return stableMetadataResult; }; diff --git a/site/src/index.css b/site/src/index.css index 854d0f4bf1..6486e16256 100644 --- a/site/src/index.css +++ b/site/src/index.css @@ -32,6 +32,8 @@ --surface-purple: 251 91% 95%; --border-default: 240 6% 90%; --border-success: 142 76% 36%; + --border-sky: 203 90% 40%; + --border-green: 138 82% 82%; --border-warning: 30.66, 97.16%, 72.35%; --border-destructive: 0 84% 60%; --border-hover: 240 5% 34%; @@ -75,6 +77,8 @@ --border-success: 142 76% 36%; --border-warning: 30.66, 97.16%, 72.35%; --border-destructive: 0 91% 71%; + --border-sky: 194 90% 62%; + --border-green: 143 77% 87%; --border-hover: 240, 5%, 34%; --overlay-default: 240 10% 4% / 80%; --highlight-purple: 252 95% 85%; diff --git a/site/src/modules/dashboard/Navbar/MobileMenu.tsx b/site/src/modules/dashboard/Navbar/MobileMenu.tsx index eb40230c91..92125bcfcd 100644 --- a/site/src/modules/dashboard/Navbar/MobileMenu.tsx +++ b/site/src/modules/dashboard/Navbar/MobileMenu.tsx @@ -162,7 +162,7 @@ const ProxySettingsSub: FC = ({ proxyContextValue }) => { {p.name} {p.display_name || p.name} {latency ? ( - + ) : ( )} diff --git a/site/src/modules/dashboard/Navbar/ProxyMenu.tsx b/site/src/modules/dashboard/Navbar/ProxyMenu.tsx index 4a0cc4d9cf..2ed60982ab 100644 --- a/site/src/modules/dashboard/Navbar/ProxyMenu.tsx +++ b/site/src/modules/dashboard/Navbar/ProxyMenu.tsx @@ -92,7 +92,6 @@ export const ProxyMenu: FC = ({ proxyContextValue }) => { ) : ( @@ -191,6 +190,7 @@ export const ProxyMenu: FC = ({ proxyContextValue }) => { {proxy.display_name} diff --git a/site/src/modules/resources/AgentLogs/AgentLogs.stories.tsx b/site/src/modules/resources/AgentLogs/AgentLogs.stories.tsx index 081ffdc506..bd4f3364b7 100644 --- a/site/src/modules/resources/AgentLogs/AgentLogs.stories.tsx +++ b/site/src/modules/resources/AgentLogs/AgentLogs.stories.tsx @@ -10,6 +10,7 @@ const meta: Meta = { sources: MockSources, logs: MockLogs, height: MockLogs.length * AGENT_LOG_LINE_HEIGHT, + overflowed: false, }, parameters: { layout: "fullscreen", @@ -19,6 +20,11 @@ const meta: Meta = { export default meta; type Story = StoryObj; -const Default: Story = {}; +export const Default: Story = {}; -export { Default as AgentLogs }; +export const Overflowed: Story = { + args: { + className: "max-h-[420px]", + overflowed: true, + }, +}; diff --git a/site/src/modules/resources/AgentLogs/AgentLogs.tsx b/site/src/modules/resources/AgentLogs/AgentLogs.tsx index c7cb357134..6c9ee75d69 100644 --- a/site/src/modules/resources/AgentLogs/AgentLogs.tsx +++ b/site/src/modules/resources/AgentLogs/AgentLogs.tsx @@ -1,178 +1,189 @@ -import type { Interpolation, Theme } from "@emotion/react"; -import Tooltip from "@mui/material/Tooltip"; +import MuiTooltip from "@mui/material/Tooltip"; import type { WorkspaceAgentLogSource } from "api/typesGenerated"; +import { Badge } from "components/Badge/Badge"; import type { Line } from "components/Logs/LogLine"; -import { type ComponentProps, forwardRef, type JSX, useMemo } from "react"; +import { + Tooltip, + TooltipContent, + TooltipProvider, + TooltipTrigger, +} from "components/Tooltip/Tooltip"; +import { type ComponentProps, forwardRef, type JSX } from "react"; import { FixedSizeList as List } from "react-window"; +import { cn } from "utils/cn"; import { AGENT_LOG_LINE_HEIGHT, AgentLogLine } from "./AgentLogLine"; +// Fallback log used in places where we must always have a valid log source. +// We need this to support deployments that were made before `coder_script` was +// created and that haven't restarted their agents yet +const fallbackLog: WorkspaceAgentLogSource = { + created_at: "", + display_name: "Logs", + icon: "", + id: "00000000-0000-0000-0000-000000000000", + workspace_agent_id: "", +}; + type AgentLogsProps = Omit< ComponentProps, - "children" | "itemSize" | "itemCount" + "children" | "itemSize" | "itemCount" | "itemKey" > & { logs: readonly Line[]; sources: readonly WorkspaceAgentLogSource[]; + overflowed: boolean; }; export const AgentLogs = forwardRef( - ({ logs, sources, ...listProps }, ref) => { - const logSourceByID = useMemo(() => { - const sourcesById: { [id: string]: WorkspaceAgentLogSource } = {}; - for (const source of sources) { - sourcesById[source.id] = source; - } - return sourcesById; - }, [sources]); + ({ logs, sources, overflowed, className, ...listProps }, ref) => { + const logSourceById = Object.fromEntries(sources.map((s) => [s.id, s])); + const getLogSource = (id: string) => logSourceById[id] || fallbackLog; return ( - - {({ index, style }) => { - const log = logs[index]; - // getLogSource always returns a valid log source. - // This is necessary to support deployments before `coder_script`. - // Existed that haven't restarted their agents. - const getLogSource = (id: string): WorkspaceAgentLogSource => { - return ( - logSourceByID[id] || { - created_at: "", - display_name: "Logs", - icon: "", - id: "00000000-0000-0000-0000-000000000000", - workspace_agent_id: "", - } - ); - }; - const logSource = getLogSource(log.sourceId); +
+ logs[index]?.id || index} + // We need the div selector to be able to apply the padding + // top from startupLogs + className={cn( + "pt-4 [&>div]:relative bg-surface-secondary", + // Add extra padding so that overflow indicator can't + // fully cover up lines of text + overflowed && "pb-10", + className, + )} + > + {({ index, style }) => { + const log = logs[index]; + const logSource = getLogSource(log.sourceId); - let assignedIcon = false; - let icon: JSX.Element; - // If no icon is specified, we show a deterministic - // colored circle to identify unique scripts. - if (logSource.icon) { - icon = ( - - ); - } else { - icon = ( -
- ); - assignedIcon = true; - } - - let nextChangesSource = false; - if (index < logs.length - 1) { - nextChangesSource = - getLogSource(logs[index + 1].sourceId).id !== log.sourceId; - } - // We don't want every line to repeat the icon, because - // that is ugly and repetitive. This removes the icon - // for subsequent lines of the same source and shows a - // line instead, visually indicating they are from the - // same source. - if ( - index > 0 && - getLogSource(logs[index - 1].sourceId).id === log.sourceId - ) { - icon = ( -
-
({ - height: nextChangesSource ? "50%" : "100%", - width: 2, - background: theme.experimental.l1.outline, - borderRadius: 2, - })} + let assignedIcon = false; + let icon: JSX.Element; + // If no icon is specified, we show a deterministic + // colored circle to identify unique scripts. + if (logSource.icon) { + icon = ( + - {nextChangesSource && ( -
({ - height: 2, - width: "50%", - top: "calc(50% - 2px)", - left: "calc(50% - 1px)", - background: theme.experimental.l1.outline, - borderRadius: 2, - position: "absolute", - })} - /> - )} -
- ); - } + ); + } else { + icon = ( +
+ ); + assignedIcon = true; + } - return ( - - {logSource.display_name} - {assignedIcon && ( - -
- No icon specified! - - )} - - } + const doesNextLineHaveDifferentSource = + index < logs.length - 1 && + getLogSource(logs[index + 1].sourceId).id !== log.sourceId; + + // We don't want every line to repeat the icon, because + // that is ugly and repetitive. This removes the icon + // for subsequent lines of the same source and shows a + // line instead, visually indicating they are from the + // same source. + const shouldHideSource = + index > 0 && + getLogSource(logs[index - 1].sourceId).id === log.sourceId; + if (shouldHideSource) { + icon = ( +
+
+ {doesNextLineHaveDifferentSource && ( +
+ )} +
+ ); + } + + return ( + + {logSource.display_name} + {assignedIcon && ( + +
+ No icon specified! + + )} + + } + > + {icon} + + } + /> + ); + }} + + + {overflowed && ( + + + + - {icon} - - } - /> - ); - }} - + Logs overflowed + + + +

+ Startup logs exceeded the max size of{" "} + 1MB, and will + not continue to be written to the database. Logs will continue + to be written to the{" "} + + /tmp/coder-startup-script.log + {" "} + file in the workspace. +

+ + + + )} +
); }, ); -// These colors were picked at random. Feel free -// to add more, adjust, or change! Users will not -// depend on these colors. -const scriptDisplayColors = [ +// These colors were picked at random. Feel free to add more, adjust, or change! +// Users will not depend on these colors. +const scriptDisplayColors: readonly string[] = [ "#85A3B2", "#A37EB2", "#C29FDE", @@ -191,15 +202,3 @@ const determineScriptDisplayColor = (displayName: string): string => { }, 0); return scriptDisplayColors[Math.abs(hash) % scriptDisplayColors.length]; }; - -const styles = { - logs: (theme) => ({ - backgroundColor: theme.palette.background.paper, - paddingTop: 16, - - // We need this to be able to apply the padding top from startupLogs - "& > div": { - position: "relative", - }, - }), -} satisfies Record>; diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index ae8dadcc39..9838e7ca83 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -19,7 +19,6 @@ import { useCallback, useEffect, useLayoutEffect, - useMemo, useRef, useState, } from "react"; @@ -79,25 +78,9 @@ export const AgentRow: FC = ({ ["starting", "start_timeout"].includes(agent.lifecycle_state) && hasStartupFeatures, ); - const agentLogs = useAgentLogs(agent, showLogs); + const agentLogs = useAgentLogs({ agentId: agent.id, enabled: showLogs }); const logListRef = useRef(null); const logListDivRef = useRef(null); - const startupLogs = useMemo(() => { - const allLogs = agentLogs || []; - - const logs = [...allLogs]; - if (agent.logs_overflowed) { - logs.push({ - id: -1, - level: "error", - output: - "Startup logs exceeded the max size of 1MB, and will not continue to be written to the database! Logs will continue to be written to the /tmp/coder-startup-script.log file in the workspace.", - created_at: new Date().toISOString(), - source_id: "", - }); - } - return logs; - }, [agentLogs, agent.logs_overflowed]); const [bottomOfLogs, setBottomOfLogs] = useState(true); useEffect(() => { @@ -109,9 +92,9 @@ export const AgentRow: FC = ({ useLayoutEffect(() => { // If we're currently watching the bottom, we always want to stay at the bottom. if (bottomOfLogs && logListRef.current) { - logListRef.current.scrollToItem(startupLogs.length - 1, "end"); + logListRef.current.scrollToItem(agentLogs.length - 1, "end"); } - }, [showLogs, startupLogs, bottomOfLogs]); + }, [showLogs, agentLogs, bottomOfLogs]); // This is a bit of a hack on the react-window API to get the scroll position. // If we're scrolled to the bottom, we want to keep the list scrolled to the bottom. @@ -328,7 +311,8 @@ export const AgentRow: FC = ({ width={width} css={styles.startupLogs} onScroll={handleLogScroll} - logs={startupLogs.map((l) => ({ + overflowed={agent.logs_overflowed} + logs={agentLogs.map((l) => ({ id: l.id, level: l.level, output: l.output, @@ -541,7 +525,7 @@ const styles = { }, startupLogs: (theme) => ({ - maxHeight: 256, + maxHeight: 420, borderBottom: `1px solid ${theme.palette.divider}`, backgroundColor: theme.palette.background.paper, paddingTop: 16, diff --git a/site/src/modules/resources/useAgentLogs.test.ts b/site/src/modules/resources/useAgentLogs.test.ts index c4943c6f9d..27281d51a2 100644 --- a/site/src/modules/resources/useAgentLogs.test.ts +++ b/site/src/modules/resources/useAgentLogs.test.ts @@ -1,60 +1,196 @@ import { MockWorkspaceAgent } from "testHelpers/entities"; +import { + createMockWebSocket, + type MockWebSocketServer, +} from "testHelpers/websockets"; import { renderHook, waitFor } from "@testing-library/react"; +import * as apiModule from "api/api"; import type { WorkspaceAgentLog } from "api/typesGenerated"; -import WS from "jest-websocket-mock"; +import * as snackbarUtils from "components/GlobalSnackbar/utils"; +import { act } from "react"; +import { OneWayWebSocket } from "utils/OneWayWebSocket"; import { useAgentLogs } from "./useAgentLogs"; -/** - * TODO: WS does not support multiple tests running at once in isolation so we - * have one single test that test the most common scenario. - * Issue: https://github.com/romgain/jest-websocket-mock/issues/172 - */ +const millisecondsInOneMinute = 60_000; -describe.skip("useAgentLogs", () => { - afterEach(() => { - WS.clean(); +function generateMockLogs( + logCount: number, + baseDate = new Date("April 1, 1970"), +): readonly WorkspaceAgentLog[] { + return Array.from({ length: logCount }, (_, i) => { + // Make sure that the logs generated each have unique timestamps, so + // that we can test whether the hook is sorting them properly as it's + // receiving them over time + const logDate = new Date(baseDate.getTime() + i * millisecondsInOneMinute); + return { + id: i, + created_at: logDate.toISOString(), + level: "info", + output: `Log ${i}`, + source_id: "", + }; + }); +} + +// A mutable object holding the most recent mock WebSocket server that was +// created when initializing a mock WebSocket. Inner value will be undefined if +// the hook is disabled on mount, but will always be defined otherwise +type ServerResult = { current: MockWebSocketServer | undefined }; + +type MountHookOptions = Readonly<{ + initialAgentId: string; + enabled?: boolean; +}>; + +type MountHookResult = Readonly<{ + serverResult: ServerResult; + rerender: (props: { agentId: string; enabled: boolean }) => void; + displayError: jest.SpyInstance; + + // Note: the `current` property is only "halfway" readonly; the value is + // readonly, but the key is still mutable + hookResult: { current: readonly WorkspaceAgentLog[] }; +}>; + +function mountHook(options: MountHookOptions): MountHookResult { + const { initialAgentId, enabled = true } = options; + const serverResult: ServerResult = { current: undefined }; + + jest + .spyOn(apiModule, "watchWorkspaceAgentLogs") + .mockImplementation((agentId, params) => { + return new OneWayWebSocket({ + apiRoute: `/api/v2/workspaceagents/${agentId}/logs`, + searchParams: new URLSearchParams({ + follow: "true", + after: params?.after?.toString() || "0", + }), + websocketInit: (url) => { + const [mockSocket, mockServer] = createMockWebSocket(url); + serverResult.current = mockServer; + return mockSocket; + }, + }); + }); + + void jest.spyOn(console, "error").mockImplementation(() => {}); + const displayError = jest.spyOn(snackbarUtils, "displayError"); + + const { result: hookResult, rerender } = renderHook( + (props) => useAgentLogs(props), + { initialProps: { enabled, agentId: initialAgentId } }, + ); + + return { rerender, serverResult, hookResult, displayError }; +} + +describe("useAgentLogs", () => { + it("Automatically sorts logs that are received out of order", async () => { + const { hookResult, serverResult } = mountHook({ + initialAgentId: MockWorkspaceAgent.id, + }); + + const logs = generateMockLogs(10, new Date("september 9, 1999")); + const reversed = logs.toReversed(); + + for (const log of reversed) { + await act(async () => { + serverResult.current?.publishMessage( + new MessageEvent("message", { data: JSON.stringify([log]) }), + ); + }); + } + await waitFor(() => expect(hookResult.current).toEqual(logs)); }); - it("clear logs when disabled to avoid duplicates", async () => { - const server = new WS( - `ws://localhost/api/v2/workspaceagents/${ - MockWorkspaceAgent.id - }/logs?follow&after=0`, - ); - const { result, rerender } = renderHook( - ({ enabled }) => useAgentLogs(MockWorkspaceAgent, enabled), - { initialProps: { enabled: true } }, - ); - await server.connected; - - // Send 3 logs - server.send(JSON.stringify(generateLogs(3))); - await waitFor(() => { - expect(result.current).toHaveLength(3); + it("Never creates a connection if hook is disabled on mount", () => { + const { serverResult } = mountHook({ + initialAgentId: MockWorkspaceAgent.id, + enabled: false, }); - // Disable the hook - rerender({ enabled: false }); - await waitFor(() => { - expect(result.current).toHaveLength(0); + expect(serverResult.current).toBe(undefined); + }); + + it("Automatically closes the socket connection when the hook is disabled", async () => { + const { serverResult, rerender } = mountHook({ + initialAgentId: MockWorkspaceAgent.id, }); - // Enable the hook again - rerender({ enabled: true }); - await server.connected; - server.send(JSON.stringify(generateLogs(3))); + expect(serverResult.current?.isConnectionOpen).toBe(true); + rerender({ agentId: MockWorkspaceAgent.id, enabled: false }); await waitFor(() => { - expect(result.current).toHaveLength(3); + expect(serverResult.current?.isConnectionOpen).toBe(false); }); }); + + it("Automatically closes the old connection when the agent ID changes", () => { + const { serverResult, rerender } = mountHook({ + initialAgentId: MockWorkspaceAgent.id, + }); + + const serverConn1 = serverResult.current; + expect(serverConn1?.isConnectionOpen).toBe(true); + + rerender({ + enabled: true, + agentId: `${MockWorkspaceAgent.id}-new-value`, + }); + + const serverConn2 = serverResult.current; + expect(serverConn1).not.toBe(serverConn2); + expect(serverConn1?.isConnectionOpen).toBe(false); + expect(serverConn2?.isConnectionOpen).toBe(true); + }); + + it("Calls error callback when error is received (but only while hook is enabled)", async () => { + const { serverResult, rerender, displayError } = mountHook({ + initialAgentId: MockWorkspaceAgent.id, + // Start off disabled so that we can check that the callback is + // never called when there is no connection + enabled: false, + }); + + const errorEvent = new Event("error"); + await act(async () => serverResult.current?.publishError(errorEvent)); + expect(displayError).not.toHaveBeenCalled(); + + rerender({ agentId: MockWorkspaceAgent.id, enabled: true }); + await act(async () => serverResult.current?.publishError(errorEvent)); + expect(displayError).toHaveBeenCalledTimes(1); + }); + + // This is a protection to avoid duplicate logs when the hook goes back to + // being re-enabled + it("Clears logs when hook becomes disabled", async () => { + const { hookResult, serverResult, rerender } = mountHook({ + initialAgentId: MockWorkspaceAgent.id, + }); + + // Send initial logs so that we have something to clear out later + const initialLogs = generateMockLogs(3, new Date("april 5, 1997")); + const initialEvent = new MessageEvent("message", { + data: JSON.stringify(initialLogs), + }); + await act(async () => serverResult.current?.publishMessage(initialEvent)); + await waitFor(() => expect(hookResult.current).toEqual(initialLogs)); + + // Need to do the following steps multiple times to make sure that we + // don't break anything after the first disable + const mockDates: readonly string[] = ["october 3, 2005", "august 1, 2025"]; + for (const md of mockDates) { + // Disable the hook to clear current logs + rerender({ agentId: MockWorkspaceAgent.id, enabled: false }); + await waitFor(() => expect(hookResult.current).toHaveLength(0)); + + // Re-enable the hook and send new logs + rerender({ agentId: MockWorkspaceAgent.id, enabled: true }); + const newLogs = generateMockLogs(3, new Date(md)); + const newEvent = new MessageEvent("message", { + data: JSON.stringify(newLogs), + }); + await act(async () => serverResult.current?.publishMessage(newEvent)); + await waitFor(() => expect(hookResult.current).toEqual(newLogs)); + } + }); }); - -function generateLogs(count: number): WorkspaceAgentLog[] { - return Array.from({ length: count }, (_, i) => ({ - id: i, - created_at: new Date().toISOString(), - level: "info", - output: `Log ${i}`, - source_id: "", - })); -} diff --git a/site/src/modules/resources/useAgentLogs.ts b/site/src/modules/resources/useAgentLogs.ts index d7f810483a..300bedb9f7 100644 --- a/site/src/modules/resources/useAgentLogs.ts +++ b/site/src/modules/resources/useAgentLogs.ts @@ -1,38 +1,66 @@ import { watchWorkspaceAgentLogs } from "api/api"; -import type { WorkspaceAgent, WorkspaceAgentLog } from "api/typesGenerated"; +import type { WorkspaceAgentLog } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; import { useEffect, useState } from "react"; +type UseAgentLogsOptions = Readonly<{ + agentId: string; + enabled?: boolean; +}>; + export function useAgentLogs( - agent: WorkspaceAgent, - enabled: boolean, + options: UseAgentLogsOptions, ): readonly WorkspaceAgentLog[] { - const [logs, setLogs] = useState([]); + const { agentId, enabled = true } = options; + const [logs, setLogs] = useState([]); + + // Clean up the logs when the agent is not enabled, using a mid-render + // sync to remove any risk of screen flickering. Clearing the logs helps + // ensure that if the hook flips back to being enabled, we can receive a + // fresh set of logs from the beginning with zero risk of duplicates. + const [prevEnabled, setPrevEnabled] = useState(enabled); + if (!enabled && prevEnabled) { + setLogs([]); + setPrevEnabled(false); + } + if (enabled && !prevEnabled) { + setPrevEnabled(true); + } useEffect(() => { if (!enabled) { - // Clean up the logs when the agent is not enabled. So it can receive logs - // from the beginning without duplicating the logs. - setLogs([]); return; } - // Always fetch the logs from the beginning. We may want to optimize this in - // the future, but it would add some complexity in the code that maybe does - // not worth it. - const socket = watchWorkspaceAgentLogs(agent.id, { after: 0 }); + // Always fetch the logs from the beginning. We may want to optimize + // this in the future, but it would add some complexity in the code + // that might not be worth it. + const socket = watchWorkspaceAgentLogs(agentId, { after: 0 }); socket.addEventListener("message", (e) => { if (e.parseError) { console.warn("Error parsing agent log: ", e.parseError); return; } - setLogs((logs) => [...logs, ...e.parsedMessage]); + + if (e.parsedMessage.length === 0) { + return; + } + + setLogs((logs) => { + const newLogs = [...logs, ...e.parsedMessage]; + newLogs.sort((l1, l2) => { + const d1 = new Date(l1.created_at).getTime(); + const d2 = new Date(l2.created_at).getTime(); + return d1 - d2; + }); + return newLogs; + }); }); socket.addEventListener("error", (e) => { console.error("Error in agent log socket: ", e); displayError( - "Unable to watch the agent logs", + "Unable to watch agent logs", "Please try refreshing the browser", ); socket.close(); @@ -41,7 +69,7 @@ export function useAgentLogs( return () => { socket.close(); }; - }, [agent.id, enabled]); + }, [agentId, enabled]); return logs; } diff --git a/site/src/pages/TaskPage/TaskPage.tsx b/site/src/pages/TaskPage/TaskPage.tsx index 4d84d47fb5..b5d551bb6e 100644 --- a/site/src/pages/TaskPage/TaskPage.tsx +++ b/site/src/pages/TaskPage/TaskPage.tsx @@ -247,7 +247,7 @@ type TaskStartingAgentProps = { }; const TaskStartingAgent: FC = ({ agent }) => { - const logs = useAgentLogs(agent, true); + const logs = useAgentLogs({ agentId: agent.id }); const listRef = useRef(null); useLayoutEffect(() => { @@ -272,6 +272,11 @@ const TaskStartingAgent: FC = ({ agent }) => {
({ id: l.id, level: l.level, @@ -279,10 +284,6 @@ const TaskStartingAgent: FC = ({ agent }) => { sourceId: l.source_id, time: l.created_at, }))} - sources={agent.log_sources} - height={96 * 4} - width="100%" - ref={listRef} />
diff --git a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx index f6b2a51acd..c1d95e6d49 100644 --- a/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx +++ b/site/src/pages/WorkspaceBuildPage/WorkspaceBuildPageView.tsx @@ -6,7 +6,6 @@ import type { import { Alert } from "components/Alert/Alert"; import { ErrorAlert } from "components/Alert/ErrorAlert"; import { Loader } from "components/Loader/Loader"; -import type { Line } from "components/Logs/LogLine"; import { Margins } from "components/Margins/Margins"; import { FullWidthPageHeader, @@ -293,24 +292,20 @@ type AgentLogsContentProps = { }; const AgentLogsContent: FC = ({ agent }) => { - const logs = useAgentLogs(agent, true); - - if (!logs) { - return ; - } - + const logs = useAgentLogs({ agentId: agent.id }); return ( ((l) => ({ + height={560} + width="100%" + logs={logs.map((l) => ({ id: l.id, output: l.output, time: l.created_at, level: l.level, sourceId: l.source_id, }))} - height={560} - width="100%" /> ); }; diff --git a/site/tailwind.config.js b/site/tailwind.config.js index e4b40aa177..1250122bb2 100644 --- a/site/tailwind.config.js +++ b/site/tailwind.config.js @@ -58,6 +58,8 @@ module.exports = { border: { DEFAULT: "hsl(var(--border-default))", warning: "hsl(var(--border-warning))", + green: "hsl(var(--border-green))", + sky: "hsl(var(--border-sky))", destructive: "hsl(var(--border-destructive))", success: "hsl(var(--border-success))", hover: "hsl(var(--border-hover))",