From a09496b0d46c0e064c99ea84f88da28a64647e3e Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 May 2024 09:05:00 +0100 Subject: [PATCH 01/12] Adapted http client for reconnecting automatically --- web/src/client/http.js | 197 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 191 insertions(+), 6 deletions(-) diff --git a/web/src/client/http.js b/web/src/client/http.js index 37a4ebdbef..d77eae9429 100644 --- a/web/src/client/http.js +++ b/web/src/client/http.js @@ -26,6 +26,23 @@ * @return {void} */ +/** + * Enum for the WebSocket states. + * + * + */ + +const SocketStates = Object.freeze({ + CONNECTED: 0, + CONNECTING: 1, + CLOSING: 2, + CLOSED: 3, + UNRECOVERABLE: 4, +}); + +const MAX_ATTEMPTS = 10; +const ATTEMPT_INTERVAL = 2000; + /** * Agama WebSocket client. * @@ -38,11 +55,65 @@ class WSClient { * @param {URL} url - Websocket URL. */ constructor(url) { - this.client = new WebSocket(url.toString()); + this.url = url.toString(); + + this.event_handlers = []; + this.close_handlers = []; + this.error_handlers = []; + this.open_handlers = []; + this.reconnectAttempts = 0; + this.connect(); + } + + wsState() { + const state = this.client.readyState; + if ((state !== SocketStates.CONNECTED) && (this.reconnectAttempts >= MAX_ATTEMPTS)) return SocketStates.UNRECOVERABLE; + + return state; + } + + isRecoverable() { + return (this.wsState() !== SocketStates.UNRECOVERABLE); + } + + isConnected() { + return (this.wsState() === SocketStates.CONNECTED); + } + + subscribe_to_events() { + this.client.onopen = () => { + console.log("Connected websocket"); + this.reconnectAttempts = 0; + if (this.timeout !== undefined) clearTimeout(this.timeout); + + return this.dispatchOpenEvent(); + }; + this.client.onmessage = (event) => { this.dispatchEvent(event); }; - this.handlers = []; + + this.client.onclose = () => { + console.log(`Closed WebSocket`); + this.dispatchCloseEvent(); + this.timeout = setTimeout(() => this.connect(this.reconnectAttempts + 1), ATTEMPT_INTERVAL); + }; + + this.client.onerror = (e) => { + console.error("WebSocket error:", e); + this.dispatchErrorEvent(); + }; + } + + connect(attempt = 0) { + this.reconnectAttempts = attempt; + if (attempt > 10) { + console.log("Max number of WebSocket connection attempts reached."); + return; + } + console.log(`Reconnecting WebSocket(attempt: ${attempt})`); + this.client = new WebSocket(this.url); + this.subscribe_to_events(); } /** @@ -55,10 +126,60 @@ class WSClient { * @return {RemoveFn} */ onEvent(func) { - this.handlers.push(func); + this.event_handlers.push(func); + return () => { + const position = this.event_handlers.indexOf(func); + if (position > -1) this.event_handlers.splice(position, 1); + }; + } + + /** + * Registers a handler for close socket. + * + * The handler is executed when the socket is close. + * + * @param {(object) => void} func - Handler function to register. + * @return {RemoveFn} + */ + onClose(func) { + this.close_handlers.push(func); + + return () => { + const position = this.close_handlers.indexOf(func); + if (position > -1) this.close_handlers.splice(position, 1); + }; + } + + /** + * Registers a handler for open socket. + * + * The handler is executed when the socket is open. + * @param {(object) => void} func - Handler function to register. + * @return {RemoveFn} + */ + onOpen(func) { + this.open_handlers.push(func); + + return () => { + const position = this.open_handlers.indexOf(func); + if (position > -1) this.open_handlers.splice(position, 1); + }; + } + + /** + * Registers a handler for socket errors. + * + * The handler is executed when an error is reported by the socket. + * + * @param {(object) => void} func - Handler function to register. + * @return {RemoveFn} + */ + onError(func) { + this.error_handlers.push(func); + return () => { - const position = this.handlers.indexOf(func); - if (position > -1) this.handlers.splice(position, 1); + const position = this.error_handlers.indexOf(func); + if (position > -1) this.error_handlers.splice(position, 1); }; } @@ -71,7 +192,34 @@ class WSClient { */ dispatchEvent(event) { const eventObject = JSON.parse(event.data); - this.handlers.forEach((f) => f(eventObject)); + this.event_handlers.forEach((f) => f(eventObject)); + } + + /** + * @private + * + * Dispatchs a close event by running all its handlers. + */ + dispatchCloseEvent() { + this.close_handlers.forEach((f) => f()); + } + + /** + * @private + * + * Dispatchs an error event by running all its handlers. + */ + dispatchErrorEvent() { + this.error_handlers.forEach((f) => f()); + } + + /** + * @private + * + * Dispatchs a close event by running all its handlers. + */ + dispatchOpenEvent() { + this.open_handlers.forEach((f) => f()); } } @@ -169,6 +317,43 @@ class HTTPClient { return response; } + /** + * Registers a handler for being called when the socket is closed + * + * @param {() => void} func - Handler function to register. + * @return {RemoveFn} - Function to remove the handler. + */ + onClose(func) { + return this.ws.onClose(() => { + func(); + }); + } + + /** + * + * Registers a handler for being called when there is some error in the socket + * + * @param {(event: Object) => void} func - Handler function to register. + * @return {RemoveFn} - Function to remove the handler. + */ + onError(func) { + return this.ws.onError((event) => { + func(event); + }); + } + + /** + * Registers a handler for being called when the socket is opened + * + * @param {(event: Object) => void} func - Handler function to register. + * @return {RemoveFn} - Function to remove the handler. + */ + onOpen(func) { + return this.ws.onOpen((event) => { + func(event); + }); + } + /** * Registers a handler for a given type of events. * From dfebff547226ddce102c4989b4674fe5fdf01b2e Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 May 2024 12:10:23 +0100 Subject: [PATCH 02/12] Component changes for reconnecting socket automatically --- web/src/App.jsx | 11 ++---- web/src/client/index.js | 24 +++++------- .../{DBusError.jsx => WebSocketError.jsx} | 10 ++--- ...Error.test.jsx => WebSocketError.test.jsx} | 12 +++--- web/src/components/core/index.js | 2 +- web/src/context/installer.jsx | 39 ++++++------------- 6 files changed, 35 insertions(+), 63 deletions(-) rename web/src/components/core/{DBusError.jsx => WebSocketError.jsx} (86%) rename web/src/components/core/{DBusError.test.jsx => WebSocketError.test.jsx} (81%) diff --git a/web/src/App.jsx b/web/src/App.jsx index a3798d43f7..bcae3b9e90 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -27,13 +27,10 @@ import { useProduct } from "./context/product"; import { INSTALL, STARTUP } from "~/client/phase"; import { BUSY } from "~/client/status"; -import { DBusError, If, Installation } from "~/components/core"; +import { WebSocketError, If, Installation } from "~/components/core"; import { Loading } from "./components/layout"; import { useInstallerL10n } from "./context/installerL10n"; -// D-Bus connection attempts before displaying an error. -const ATTEMPTS = 3; - /** * Main application component. * @@ -43,7 +40,6 @@ const ATTEMPTS = 3; */ function App() { const client = useInstallerClient(); - const { attempt } = useInstallerClientStatus(); const { products } = useProduct(); const { language } = useInstallerL10n(); const [status, setStatus] = useState(undefined); @@ -75,9 +71,8 @@ function App() { }, [client, setPhase, setStatus]); const Content = () => { - if (!client || !products) { - return (attempt > ATTEMPTS) ? : ; - } + if (!client || !client.isRecoverable()) return ; + if (!products) return ; if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) { return ; diff --git a/web/src/client/index.js b/web/src/client/index.js index 590e562fc1..e8cc3f63a5 100644 --- a/web/src/client/index.js +++ b/web/src/client/index.js @@ -36,7 +36,7 @@ import { HTTPClient } from "./http"; * @typedef {object} InstallerClient * @property {L10nClient} l10n - localization client. * @property {ManagerClient} manager - manager client. - * @property {Monitor} monitor - service monitor. + * property {Monitor} monitor - service monitor. (FIXME) * @property {NetworkClient} network - network client. * @property {ProductClient} product - product client. * @property {SoftwareClient} software - software client. @@ -46,7 +46,9 @@ import { HTTPClient } from "./http"; * @property {() => Promise} issues - issues from all contexts. * @property {(handler: IssuesHandler) => (() => void)} onIssuesChange - registers a handler to run * when issues from any context change. It returns a function to deregister the handler. - * @property {() => Promise} isConnected - determines whether the client is connected + * @property {() => boolean} isConnected - determines whether the client is connected + * @property {() => boolean} isRecoverable - determines whether the client is recoverable after disconnected + * @property {(handler: () => void) => (() => void)} onConnect - registers a handler to run * @property {(handler: () => void) => (() => void)} onDisconnect - registers a handler to run * when the connection is lost. It returns a function to deregister the * handler. @@ -122,15 +124,8 @@ const createClient = (url) => { }; }; - const isConnected = async () => { - // try { - // await manager.getStatus(); - // return true; - // } catch (e) { - // return false; - // } - return true; - }; + const isConnected = () => client.ws?.isConnected() || false; + const isRecoverable = () => !!client.ws?.isRecoverable(); return { l10n, @@ -145,10 +140,9 @@ const createClient = (url) => { issues, onIssuesChange, isConnected, - onDisconnect: (handler) => { - return () => { }; - }, - // onDisconnect: (handler) => monitor.onDisconnect(handler), + isRecoverable, + onConnect: (handler) => client.ws.onOpen(handler), + onDisconnect: (handler) => client.ws.onClose(handler), }; }; diff --git a/web/src/components/core/DBusError.jsx b/web/src/components/core/WebSocketError.jsx similarity index 86% rename from web/src/components/core/DBusError.jsx rename to web/src/components/core/WebSocketError.jsx index 20c90c895c..9c1445d725 100644 --- a/web/src/components/core/DBusError.jsx +++ b/web/src/components/core/WebSocketError.jsx @@ -28,19 +28,19 @@ import { locationReload } from "~/utils"; const ErrorIcon = () => ; -function DBusError() { +function WebSocketError() { return ( // TRANSLATORS: page title - +
} /> - {_("Could not connect to the D-Bus service. Please, check whether it is running.")} + {_("Could not connect to the HTTP server. Please, check whether it is running.")}
@@ -55,4 +55,4 @@ function DBusError() { ); } -export default DBusError; +export default WebSocketError; diff --git a/web/src/components/core/DBusError.test.jsx b/web/src/components/core/WebSocketError.test.jsx similarity index 81% rename from web/src/components/core/DBusError.test.jsx rename to web/src/components/core/WebSocketError.test.jsx index 360e471e11..01f3410fd3 100644 --- a/web/src/components/core/DBusError.test.jsx +++ b/web/src/components/core/WebSocketError.test.jsx @@ -24,19 +24,19 @@ import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import * as utils from "~/utils"; -import { DBusError } from "~/components/core"; +import { WebSocketError } from "~/components/core"; jest.mock("~/components/core/Sidebar", () => () =>
Agama sidebar
); -describe("DBusError", () => { - it("includes a generic D-Bus connection problem message", () => { - plainRender(); - screen.getByText(/Could not connect to the D-Bus service/i); +describe("WebSocketError", () => { + it("includes a generic websocket connection problem message", () => { + plainRender(); + screen.getByText(/Could not connect to the HTTP server/i); }); it("calls location.reload when user clicks on 'Reload'", async () => { jest.spyOn(utils, "locationReload").mockImplementation(utils.noop); - const { user } = plainRender(); + const { user } = plainRender(); const reloadButton = await screen.findByRole("button", { name: /Reload/i }); await user.click(reloadButton); expect(utils.locationReload).toHaveBeenCalled(); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 4b12c532a6..80e35ce770 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -21,7 +21,7 @@ export { default as About } from "./About"; export { default as PageMenu } from "./PageMenu"; -export { default as DBusError } from "./DBusError"; +export { default as WebSocketError } from "./WebSocketError"; export { default as Description } from "./Description"; export { default as Disclosure } from "./Disclosure"; export { default as Sidebar } from "./Sidebar"; diff --git a/web/src/context/installer.jsx b/web/src/context/installer.jsx index 85e1bd7ba6..e86a2302a7 100644 --- a/web/src/context/installer.jsx +++ b/web/src/context/installer.jsx @@ -27,7 +27,7 @@ import { createDefaultClient } from "~/client"; const InstallerClientContext = React.createContext(null); // TODO: we use a separate context to avoid changing all the codes to // `useInstallerClient`. We should merge them in the future. -const InstallerClientStatusContext = React.createContext({ connected: false, attempt: 0 }); +const InstallerClientStatusContext = React.createContext({ connected: false }); /** * Returns the D-Bus installer client @@ -48,7 +48,6 @@ function useInstallerClient() { * * @typedef {object} ClientStatus * @property {boolean} connected - whether the client is connected - * @property {number} attempt - number of attempt to connect * * @return {ClientStatus} installer client status */ @@ -61,8 +60,6 @@ function useInstallerClientStatus() { return context; } -const INTERVAL = 2000; - /** * @param {object} props * @param {import("~/client").InstallerClient|undefined} [props.client] client to connect to @@ -73,23 +70,14 @@ const INTERVAL = 2000; * @param {React.ReactNode} [props.children] - content to display within the provider */ function InstallerClientProvider({ - children, client = null, interval = INTERVAL + children, client = null }) { const [value, setValue] = useState(client); - const [attempt, setAttempt] = useState(0); useEffect(() => { const connectClient = async () => { const client = await createDefaultClient(); - if (await client.isConnected()) { - setValue(client); - setAttempt(0); - return; - } - - console.warn(`Failed to connect to Agama API (attempt ${attempt + 1})`); - await new Promise(resolve => setTimeout(resolve, interval)); - setAttempt(attempt + 1); + setValue(client); }; // allow hot replacement for the clients code @@ -99,28 +87,23 @@ function InstallerClientProvider({ console.log("[Agama HMR] A client module has been updated"); const updated_client = await createDefaultClient(); - if (await updated_client.isConnected()) { - console.log("[Agama HMR] Using new clients"); - setValue(updated_client); - } else { - console.warn("[Agama HMR] Updating clients failed, using full page reload"); - window.location.reload(); - } + console.log("[Agama HMR] Using new clients"); + setValue(updated_client); }); } if (!value) connectClient(); - }, [setValue, value, setAttempt, attempt, interval]); + }, [setValue, value]); - useEffect(() => { - if (!value) return; + const connected = () => { + if (!value) return false; - return value.onDisconnect(() => setValue(null)); - }, [value]); + return value.isConnected(); + }; return ( - + {children} From 947da124ca0a7e20feca0b0ba670bc28ad6e633f Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 May 2024 14:40:51 +0100 Subject: [PATCH 03/12] Some fixes for properly report the connection error --- web/src/App.jsx | 3 +- web/src/client/http.js | 67 ++++++++++++++++-------------- web/src/context/installer.jsx | 24 ++++++++--- web/src/context/installer.test.jsx | 41 +----------------- 4 files changed, 58 insertions(+), 77 deletions(-) diff --git a/web/src/App.jsx b/web/src/App.jsx index bcae3b9e90..5982958f9f 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -40,6 +40,7 @@ import { useInstallerL10n } from "./context/installerL10n"; */ function App() { const client = useInstallerClient(); + const { connected, error } = useInstallerClientStatus(); const { products } = useProduct(); const { language } = useInstallerL10n(); const [status, setStatus] = useState(undefined); @@ -71,7 +72,7 @@ function App() { }, [client, setPhase, setStatus]); const Content = () => { - if (!client || !client.isRecoverable()) return ; + if (error) return ; if (!products) return ; if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) { diff --git a/web/src/client/http.js b/web/src/client/http.js index d77eae9429..73b44523ce 100644 --- a/web/src/client/http.js +++ b/web/src/client/http.js @@ -40,8 +40,8 @@ const SocketStates = Object.freeze({ UNRECOVERABLE: 4, }); -const MAX_ATTEMPTS = 10; -const ATTEMPT_INTERVAL = 2000; +const MAX_ATTEMPTS = 15; +const ATTEMPT_INTERVAL = 1000; /** * Agama WebSocket client. @@ -57,12 +57,15 @@ class WSClient { constructor(url) { this.url = url.toString(); - this.event_handlers = []; - this.close_handlers = []; - this.error_handlers = []; - this.open_handlers = []; + this.handlers = { + error: [], + close: [], + open: [], + events: [] + }; + this.reconnectAttempts = 0; - this.connect(); + this.client = this.buildClient(); } wsState() { @@ -80,8 +83,9 @@ class WSClient { return (this.wsState() === SocketStates.CONNECTED); } - subscribe_to_events() { - this.client.onopen = () => { + buildClient() { + const client = new WebSocket(this.url); + client.onopen = () => { console.log("Connected websocket"); this.reconnectAttempts = 0; if (this.timeout !== undefined) clearTimeout(this.timeout); @@ -89,31 +93,32 @@ class WSClient { return this.dispatchOpenEvent(); }; - this.client.onmessage = (event) => { + client.onmessage = (event) => { this.dispatchEvent(event); }; - this.client.onclose = () => { + client.onclose = () => { console.log(`Closed WebSocket`); this.dispatchCloseEvent(); this.timeout = setTimeout(() => this.connect(this.reconnectAttempts + 1), ATTEMPT_INTERVAL); }; - this.client.onerror = (e) => { + client.onerror = (e) => { console.error("WebSocket error:", e); this.dispatchErrorEvent(); }; + + return client; } connect(attempt = 0) { this.reconnectAttempts = attempt; - if (attempt > 10) { + if (attempt > MAX_ATTEMPTS) { console.log("Max number of WebSocket connection attempts reached."); return; } console.log(`Reconnecting WebSocket(attempt: ${attempt})`); - this.client = new WebSocket(this.url); - this.subscribe_to_events(); + this.client = this.buildClient(); } /** @@ -126,10 +131,10 @@ class WSClient { * @return {RemoveFn} */ onEvent(func) { - this.event_handlers.push(func); + this.handlers.events.push(func); return () => { - const position = this.event_handlers.indexOf(func); - if (position > -1) this.event_handlers.splice(position, 1); + const position = this.handlers.events.indexOf(func); + if (position > -1) this.handlers.events.splice(position, 1); }; } @@ -142,11 +147,11 @@ class WSClient { * @return {RemoveFn} */ onClose(func) { - this.close_handlers.push(func); + this.handlers.close.push(func); return () => { - const position = this.close_handlers.indexOf(func); - if (position > -1) this.close_handlers.splice(position, 1); + const position = this.handlers.close.indexOf(func); + if (position > -1) this.handlers.close.splice(position, 1); }; } @@ -158,11 +163,11 @@ class WSClient { * @return {RemoveFn} */ onOpen(func) { - this.open_handlers.push(func); + this.handlers.open.push(func); return () => { - const position = this.open_handlers.indexOf(func); - if (position > -1) this.open_handlers.splice(position, 1); + const position = this.handlers.open.indexOf(func); + if (position > -1) this.handlers.open.splice(position, 1); }; } @@ -175,11 +180,11 @@ class WSClient { * @return {RemoveFn} */ onError(func) { - this.error_handlers.push(func); + this.handlers.error.push(func); return () => { - const position = this.error_handlers.indexOf(func); - if (position > -1) this.error_handlers.splice(position, 1); + const position = this.handlers.error.indexOf(func); + if (position > -1) this.handlers.error.splice(position, 1); }; } @@ -192,7 +197,7 @@ class WSClient { */ dispatchEvent(event) { const eventObject = JSON.parse(event.data); - this.event_handlers.forEach((f) => f(eventObject)); + this.handlers.events.forEach((f) => f(eventObject)); } /** @@ -201,7 +206,7 @@ class WSClient { * Dispatchs a close event by running all its handlers. */ dispatchCloseEvent() { - this.close_handlers.forEach((f) => f()); + this.handlers.close.forEach((f) => f()); } /** @@ -210,7 +215,7 @@ class WSClient { * Dispatchs an error event by running all its handlers. */ dispatchErrorEvent() { - this.error_handlers.forEach((f) => f()); + this.handlers.error.forEach((f) => f()); } /** @@ -219,7 +224,7 @@ class WSClient { * Dispatchs a close event by running all its handlers. */ dispatchOpenEvent() { - this.open_handlers.forEach((f) => f()); + this.handlers.open.forEach((f) => f()); } } diff --git a/web/src/context/installer.jsx b/web/src/context/installer.jsx index e86a2302a7..dccf73d0f5 100644 --- a/web/src/context/installer.jsx +++ b/web/src/context/installer.jsx @@ -27,7 +27,7 @@ import { createDefaultClient } from "~/client"; const InstallerClientContext = React.createContext(null); // TODO: we use a separate context to avoid changing all the codes to // `useInstallerClient`. We should merge them in the future. -const InstallerClientStatusContext = React.createContext({ connected: false }); +const InstallerClientStatusContext = React.createContext({ connected: false, error: false }); /** * Returns the D-Bus installer client @@ -48,6 +48,8 @@ function useInstallerClient() { * * @typedef {object} ClientStatus * @property {boolean} connected - whether the client is connected + * @property {boolean} error - whether the client present an error and cannot + * reconnect * * @return {ClientStatus} installer client status */ @@ -73,6 +75,8 @@ function InstallerClientProvider({ children, client = null }) { const [value, setValue] = useState(client); + const [connected, setConnected] = useState(false); + const [error, setError] = useState(false); useEffect(() => { const connectClient = async () => { @@ -95,15 +99,23 @@ function InstallerClientProvider({ if (!value) connectClient(); }, [setValue, value]); - const connected = () => { - if (!value) return false; + useEffect(() => { + if (!value) return; + + value.onConnect(() => { + setConnected(true); + setError(false); + }); - return value.isConnected(); - }; + value.onDisconnect(() => { + setConnected(false); + setError(!value.isRecoverable()); + }); + }, [value]); return ( - + {children} diff --git a/web/src/context/installer.test.jsx b/web/src/context/installer.test.jsx index dc0763e1cd..f33901209f 100644 --- a/web/src/context/installer.test.jsx +++ b/web/src/context/installer.test.jsx @@ -32,11 +32,10 @@ const isConnectedFn = jest.fn(); // Helper component to check the client status. const ClientStatus = () => { - const { attempt, connected } = useInstallerClientStatus(); + const { connected } = useInstallerClientStatus(); return (
    -
  • {`attempt: ${attempt}`}
  • {`connected: ${connected}`}
); @@ -52,21 +51,6 @@ describe("installer context", () => { }); }); - describe("when there are problems connecting to the D-Bus service", () => { - beforeEach(() => { - isConnectedFn.mockResolvedValueOnce(false); - isConnectedFn.mockResolvedValueOnce(true); - }); - - it("reports each attempt through the useInstallerClientStatus hook", async () => { - plainRender( - - - ); - await screen.findByText("attempt: 1"); - }); - }); - describe("when the client is connected", () => { beforeEach(() => { isConnectedFn.mockResolvedValue(true); @@ -74,31 +58,10 @@ describe("installer context", () => { it("reports the status through the useInstallerClientStatus hook", async () => { plainRender( - + ); await screen.findByText("connected: true"); }); }); - - describe("when the D-Bus service is disconnected", () => { - beforeEach(() => { - isConnectedFn.mockResolvedValue(true); - }); - - it("reconnects to the D-Bus service", async () => { - const [onDisconnect, callbacks] = createCallbackMock(); - onDisconnectFn = onDisconnect; - - plainRender( - - - - ); - await screen.findByText("connected: true"); - const [disconnect] = callbacks; - act(disconnect); - await screen.findByText("connected: false"); - }); - }); }); From 12ffebdd27042f1b4d4fbdbda9ad86e8b32fceeb Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Fri, 24 May 2024 15:15:28 +0100 Subject: [PATCH 04/12] Use a more generic server error --- web/src/App.jsx | 6 +++--- .../core/{WebSocketError.jsx => ServerError.jsx} | 10 +++++----- ...{WebSocketError.test.jsx => ServerError.test.jsx} | 12 ++++++------ web/src/components/core/index.js | 2 +- 4 files changed, 15 insertions(+), 15 deletions(-) rename web/src/components/core/{WebSocketError.jsx => ServerError.jsx} (86%) rename web/src/components/core/{WebSocketError.test.jsx => ServerError.test.jsx} (81%) diff --git a/web/src/App.jsx b/web/src/App.jsx index 5982958f9f..fbe3458e72 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -27,7 +27,7 @@ import { useProduct } from "./context/product"; import { INSTALL, STARTUP } from "~/client/phase"; import { BUSY } from "~/client/status"; -import { WebSocketError, If, Installation } from "~/components/core"; +import { ServerError, If, Installation } from "~/components/core"; import { Loading } from "./components/layout"; import { useInstallerL10n } from "./context/installerL10n"; @@ -40,7 +40,7 @@ import { useInstallerL10n } from "./context/installerL10n"; */ function App() { const client = useInstallerClient(); - const { connected, error } = useInstallerClientStatus(); + const { error } = useInstallerClientStatus(); const { products } = useProduct(); const { language } = useInstallerL10n(); const [status, setStatus] = useState(undefined); @@ -72,7 +72,7 @@ function App() { }, [client, setPhase, setStatus]); const Content = () => { - if (error) return ; + if (error) return ; if (!products) return ; if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) { diff --git a/web/src/components/core/WebSocketError.jsx b/web/src/components/core/ServerError.jsx similarity index 86% rename from web/src/components/core/WebSocketError.jsx rename to web/src/components/core/ServerError.jsx index 9c1445d725..b968e7f115 100644 --- a/web/src/components/core/WebSocketError.jsx +++ b/web/src/components/core/ServerError.jsx @@ -28,19 +28,19 @@ import { locationReload } from "~/utils"; const ErrorIcon = () => ; -function WebSocketError() { +function ServerError() { return ( // TRANSLATORS: page title - +
} /> - {_("Could not connect to the HTTP server. Please, check whether it is running.")} + {_("Could not connect to the Agama server. Please, check whether it is running.")}
@@ -55,4 +55,4 @@ function WebSocketError() { ); } -export default WebSocketError; +export default ServerError; diff --git a/web/src/components/core/WebSocketError.test.jsx b/web/src/components/core/ServerError.test.jsx similarity index 81% rename from web/src/components/core/WebSocketError.test.jsx rename to web/src/components/core/ServerError.test.jsx index 01f3410fd3..c6273b508f 100644 --- a/web/src/components/core/WebSocketError.test.jsx +++ b/web/src/components/core/ServerError.test.jsx @@ -24,19 +24,19 @@ import { screen } from "@testing-library/react"; import { plainRender } from "~/test-utils"; import * as utils from "~/utils"; -import { WebSocketError } from "~/components/core"; +import { ServerError } from "~/components/core"; jest.mock("~/components/core/Sidebar", () => () =>
Agama sidebar
); -describe("WebSocketError", () => { - it("includes a generic websocket connection problem message", () => { - plainRender(); - screen.getByText(/Could not connect to the HTTP server/i); +describe("ServerError", () => { + it("includes a generic server problem message", () => { + plainRender(); + screen.getByText(/Could not connect to the Agama server/i); }); it("calls location.reload when user clicks on 'Reload'", async () => { jest.spyOn(utils, "locationReload").mockImplementation(utils.noop); - const { user } = plainRender(); + const { user } = plainRender(); const reloadButton = await screen.findByRole("button", { name: /Reload/i }); await user.click(reloadButton); expect(utils.locationReload).toHaveBeenCalled(); diff --git a/web/src/components/core/index.js b/web/src/components/core/index.js index 80e35ce770..3eace19445 100644 --- a/web/src/components/core/index.js +++ b/web/src/components/core/index.js @@ -21,7 +21,6 @@ export { default as About } from "./About"; export { default as PageMenu } from "./PageMenu"; -export { default as WebSocketError } from "./WebSocketError"; export { default as Description } from "./Description"; export { default as Disclosure } from "./Disclosure"; export { default as Sidebar } from "./Sidebar"; @@ -56,6 +55,7 @@ export { default as ShowTerminalButton } from "./ShowTerminalButton"; export { default as NumericTextInput } from "./NumericTextInput"; export { default as PasswordInput } from "./PasswordInput"; export { default as Selector } from "./Selector"; +export { default as ServerError } from "./ServerError"; export { default as ExpandableSelector } from "./ExpandableSelector"; export { default as OptionsPicker } from "./OptionsPicker"; export { default as Reminder } from "./Reminder"; From e9de7322516a2d31e36cd09fe89f364ae99dda33 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Mon, 27 May 2024 08:58:23 +0100 Subject: [PATCH 05/12] Small changes based on code review --- web/src/client/http.js | 6 +++--- web/src/context/installer.test.jsx | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/web/src/client/http.js b/web/src/client/http.js index 73b44523ce..d7d6c105d9 100644 --- a/web/src/client/http.js +++ b/web/src/client/http.js @@ -86,9 +86,9 @@ class WSClient { buildClient() { const client = new WebSocket(this.url); client.onopen = () => { - console.log("Connected websocket"); + console.log("Websocket connected"); this.reconnectAttempts = 0; - if (this.timeout !== undefined) clearTimeout(this.timeout); + clearTimeout(this.timeout); return this.dispatchOpenEvent(); }; @@ -98,7 +98,7 @@ class WSClient { }; client.onclose = () => { - console.log(`Closed WebSocket`); + console.log(`WebSocket closed`); this.dispatchCloseEvent(); this.timeout = setTimeout(() => this.connect(this.reconnectAttempts + 1), ATTEMPT_INTERVAL); }; diff --git a/web/src/context/installer.test.jsx b/web/src/context/installer.test.jsx index f33901209f..b448ab7971 100644 --- a/web/src/context/installer.test.jsx +++ b/web/src/context/installer.test.jsx @@ -27,7 +27,7 @@ import { InstallerClientProvider, useInstallerClientStatus } from "./installer"; jest.mock("~/client"); -let onDisconnectFn = jest.fn(); +const onDisconnectFn = jest.fn(); const isConnectedFn = jest.fn(); // Helper component to check the client status. From 5db3e70e0a303549183782b2f2025f63e165e8da Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 28 May 2024 11:00:52 +0100 Subject: [PATCH 06/12] Mock client onConnect and onDisconnect --- web/src/components/overview/L10nSection.test.jsx | 2 ++ web/src/components/overview/NetworkSection.test.jsx | 2 ++ web/src/components/overview/ProductSection.test.jsx | 2 ++ web/src/components/overview/SoftwareSection.test.jsx | 2 ++ web/src/components/overview/StorageSection.test.jsx | 3 ++- web/src/components/overview/UsersSection.test.jsx | 2 ++ 6 files changed, 12 insertions(+), 1 deletion(-) diff --git a/web/src/components/overview/L10nSection.test.jsx b/web/src/components/overview/L10nSection.test.jsx index b94a79ef99..3634f84c94 100644 --- a/web/src/components/overview/L10nSection.test.jsx +++ b/web/src/components/overview/L10nSection.test.jsx @@ -50,6 +50,8 @@ beforeEach(() => { // if defined outside, the mock is cleared automatically createClient.mockImplementation(() => { return { + onConnect: jest.fn(), + onDisconnect: jest.fn(), l10n: l10nClientMock, }; }); diff --git a/web/src/components/overview/NetworkSection.test.jsx b/web/src/components/overview/NetworkSection.test.jsx index 8ae4733467..571cbe882f 100644 --- a/web/src/components/overview/NetworkSection.test.jsx +++ b/web/src/components/overview/NetworkSection.test.jsx @@ -54,6 +54,8 @@ beforeEach(() => { // if defined outside, the mock is cleared automatically createClient.mockImplementation(() => { return { + onConnect: jest.fn(), + onDisconnect: jest.fn(), network: { devices: devicesFn, onNetworkChange: onNetworkChangeEventFn, diff --git a/web/src/components/overview/ProductSection.test.jsx b/web/src/components/overview/ProductSection.test.jsx index a55279e1cf..4a2d9c3198 100644 --- a/web/src/components/overview/ProductSection.test.jsx +++ b/web/src/components/overview/ProductSection.test.jsx @@ -49,6 +49,8 @@ beforeEach(() => { createClient.mockImplementation(() => { return { + onConnect: jest.fn(), + onDisconnect: jest.fn(), product: { getIssues: jest.fn().mockResolvedValue(issues), onIssuesChange: jest.fn() diff --git a/web/src/components/overview/SoftwareSection.test.jsx b/web/src/components/overview/SoftwareSection.test.jsx index 1cafd24ea5..7af93eee5d 100644 --- a/web/src/components/overview/SoftwareSection.test.jsx +++ b/web/src/components/overview/SoftwareSection.test.jsx @@ -45,6 +45,8 @@ let getIssuesFn = jest.fn().mockResolvedValue([]); beforeEach(() => { createClient.mockImplementation(() => { return { + onConnect: jest.fn(), + onDisconnect: jest.fn(), software: { getStatus: getStatusFn, getProgress: getProgressFn, diff --git a/web/src/components/overview/StorageSection.test.jsx b/web/src/components/overview/StorageSection.test.jsx index 251f751d93..f734fa6b0f 100644 --- a/web/src/components/overview/StorageSection.test.jsx +++ b/web/src/components/overview/StorageSection.test.jsx @@ -47,6 +47,7 @@ const proposalResult = { }; const storageMock = { + probe: jest.fn().mockResolvedValue(0), proposal: { getAvailableDevices: jest.fn().mockResolvedValue(availableDevices), @@ -70,7 +71,7 @@ let storage; beforeEach(() => { storage = { ...storageMock, proposal: { ...storageMock.proposal } }; - createClient.mockImplementation(() => ({ storage })); + createClient.mockImplementation(() => ({ storage, onConnect: jest.fn(), onDisconnect: jest.fn() })); }); it("probes storage if the storage devices are deprecated", async () => { diff --git a/web/src/components/overview/UsersSection.test.jsx b/web/src/components/overview/UsersSection.test.jsx index 6e0bca8c23..9342a1367a 100644 --- a/web/src/components/overview/UsersSection.test.jsx +++ b/web/src/components/overview/UsersSection.test.jsx @@ -54,6 +54,8 @@ beforeEach(() => { // if defined outside, the mock is cleared automatically createClient.mockImplementation(() => { return { + onConnect: jest.fn(), + onDisconnect: jest.fn(), users: { ...userClientMock, } From f1bba4eb51f2caa59700eabbeca4e706e3a8d0e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 28 May 2024 16:11:25 +0100 Subject: [PATCH 07/12] Fix installer context test --- web/src/context/installer.test.jsx | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/web/src/context/installer.test.jsx b/web/src/context/installer.test.jsx index b448ab7971..25354eaf5e 100644 --- a/web/src/context/installer.test.jsx +++ b/web/src/context/installer.test.jsx @@ -27,9 +27,6 @@ import { InstallerClientProvider, useInstallerClientStatus } from "./installer"; jest.mock("~/client"); -const onDisconnectFn = jest.fn(); -const isConnectedFn = jest.fn(); - // Helper component to check the client status. const ClientStatus = () => { const { connected } = useInstallerClientStatus(); @@ -45,23 +42,18 @@ describe("installer context", () => { beforeEach(() => { createDefaultClient.mockImplementation(() => { return { - isConnected: isConnectedFn, - onDisconnect: onDisconnectFn + onConnect: jest.fn(), + onDisconnect: jest.fn() }; }); }); - describe("when the client is connected", () => { - beforeEach(() => { - isConnectedFn.mockResolvedValue(true); - }); - - it("reports the status through the useInstallerClientStatus hook", async () => { - plainRender( - - - ); - await screen.findByText("connected: true"); - }); + it("reports the status through the useInstallerClientStatus hook", async () => { + plainRender( + + + + ); + await screen.findByText("connected: false"); }); }); From 7cbdf94138f32a1d941e9f1902db830231b74ad6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 28 May 2024 16:52:16 +0100 Subject: [PATCH 08/12] Fix client mocking --- web/src/context/installerL10n.test.jsx | 2 ++ web/src/test-utils.js | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/web/src/context/installerL10n.test.jsx b/web/src/context/installerL10n.test.jsx index 11794e002c..9837a1d57e 100644 --- a/web/src/context/installerL10n.test.jsx +++ b/web/src/context/installerL10n.test.jsx @@ -33,6 +33,8 @@ const getUILocaleFn = jest.fn().mockResolvedValue(); const setUILocaleFn = jest.fn().mockResolvedValue(); const client = { + onConnect: jest.fn(), + onDisconnect: jest.fn(), l10n: { getUILocale: getUILocaleFn, setUILocale: setUILocaleFn, diff --git a/web/src/test-utils.js b/web/src/test-utils.js index 5355bfa2c4..77b00d9a67 100644 --- a/web/src/test-utils.js +++ b/web/src/test-utils.js @@ -74,6 +74,10 @@ jest.mock('react-router-dom', () => ({ const Providers = ({ children, withL10n }) => { const client = createClient(new URL("https://localhost")); + if (!client.onConnect) { + client.onConnect = noop; + } + // FIXME: workaround to fix the tests. We should inject // the client instead of mocking `createClient`. if (!client.onDisconnect) { From 20cfcb3dd3c026a8b1391949ca815028582358eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 28 May 2024 16:54:44 +0100 Subject: [PATCH 09/12] Revert "Mock client onConnect and onDisconnect" This reverts commit 5db3e70e0a303549183782b2f2025f63e165e8da. --- web/src/components/overview/L10nSection.test.jsx | 2 -- web/src/components/overview/NetworkSection.test.jsx | 2 -- web/src/components/overview/ProductSection.test.jsx | 2 -- web/src/components/overview/SoftwareSection.test.jsx | 2 -- web/src/components/overview/StorageSection.test.jsx | 3 +-- web/src/components/overview/UsersSection.test.jsx | 2 -- 6 files changed, 1 insertion(+), 12 deletions(-) diff --git a/web/src/components/overview/L10nSection.test.jsx b/web/src/components/overview/L10nSection.test.jsx index 3634f84c94..b94a79ef99 100644 --- a/web/src/components/overview/L10nSection.test.jsx +++ b/web/src/components/overview/L10nSection.test.jsx @@ -50,8 +50,6 @@ beforeEach(() => { // if defined outside, the mock is cleared automatically createClient.mockImplementation(() => { return { - onConnect: jest.fn(), - onDisconnect: jest.fn(), l10n: l10nClientMock, }; }); diff --git a/web/src/components/overview/NetworkSection.test.jsx b/web/src/components/overview/NetworkSection.test.jsx index 571cbe882f..8ae4733467 100644 --- a/web/src/components/overview/NetworkSection.test.jsx +++ b/web/src/components/overview/NetworkSection.test.jsx @@ -54,8 +54,6 @@ beforeEach(() => { // if defined outside, the mock is cleared automatically createClient.mockImplementation(() => { return { - onConnect: jest.fn(), - onDisconnect: jest.fn(), network: { devices: devicesFn, onNetworkChange: onNetworkChangeEventFn, diff --git a/web/src/components/overview/ProductSection.test.jsx b/web/src/components/overview/ProductSection.test.jsx index 4a2d9c3198..a55279e1cf 100644 --- a/web/src/components/overview/ProductSection.test.jsx +++ b/web/src/components/overview/ProductSection.test.jsx @@ -49,8 +49,6 @@ beforeEach(() => { createClient.mockImplementation(() => { return { - onConnect: jest.fn(), - onDisconnect: jest.fn(), product: { getIssues: jest.fn().mockResolvedValue(issues), onIssuesChange: jest.fn() diff --git a/web/src/components/overview/SoftwareSection.test.jsx b/web/src/components/overview/SoftwareSection.test.jsx index 7af93eee5d..1cafd24ea5 100644 --- a/web/src/components/overview/SoftwareSection.test.jsx +++ b/web/src/components/overview/SoftwareSection.test.jsx @@ -45,8 +45,6 @@ let getIssuesFn = jest.fn().mockResolvedValue([]); beforeEach(() => { createClient.mockImplementation(() => { return { - onConnect: jest.fn(), - onDisconnect: jest.fn(), software: { getStatus: getStatusFn, getProgress: getProgressFn, diff --git a/web/src/components/overview/StorageSection.test.jsx b/web/src/components/overview/StorageSection.test.jsx index f734fa6b0f..251f751d93 100644 --- a/web/src/components/overview/StorageSection.test.jsx +++ b/web/src/components/overview/StorageSection.test.jsx @@ -47,7 +47,6 @@ const proposalResult = { }; const storageMock = { - probe: jest.fn().mockResolvedValue(0), proposal: { getAvailableDevices: jest.fn().mockResolvedValue(availableDevices), @@ -71,7 +70,7 @@ let storage; beforeEach(() => { storage = { ...storageMock, proposal: { ...storageMock.proposal } }; - createClient.mockImplementation(() => ({ storage, onConnect: jest.fn(), onDisconnect: jest.fn() })); + createClient.mockImplementation(() => ({ storage })); }); it("probes storage if the storage devices are deprecated", async () => { diff --git a/web/src/components/overview/UsersSection.test.jsx b/web/src/components/overview/UsersSection.test.jsx index 9342a1367a..6e0bca8c23 100644 --- a/web/src/components/overview/UsersSection.test.jsx +++ b/web/src/components/overview/UsersSection.test.jsx @@ -54,8 +54,6 @@ beforeEach(() => { // if defined outside, the mock is cleared automatically createClient.mockImplementation(() => { return { - onConnect: jest.fn(), - onDisconnect: jest.fn(), users: { ...userClientMock, } From a2c2f2af70c910b97ef9555fdb3e5f3dd702a287 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Tue, 28 May 2024 17:00:46 +0100 Subject: [PATCH 10/12] Remove a duplicate key --- web/src/context/installerL10n.test.jsx | 1 - 1 file changed, 1 deletion(-) diff --git a/web/src/context/installerL10n.test.jsx b/web/src/context/installerL10n.test.jsx index 9837a1d57e..50af75fa4b 100644 --- a/web/src/context/installerL10n.test.jsx +++ b/web/src/context/installerL10n.test.jsx @@ -40,7 +40,6 @@ const client = { setUILocale: setUILocaleFn, getUIKeymap: jest.fn().mockResolvedValue("en"), }, - onDisconnect: jest.fn(), }; jest.mock("~/languages.json", () => ({ From 7cd86ed99fdca5c0ae08368100dff6a6fe180a56 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 28 May 2024 17:03:04 +0100 Subject: [PATCH 11/12] Remove redundant part of the error message --- web/src/components/core/ServerError.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/core/ServerError.jsx b/web/src/components/core/ServerError.jsx index b968e7f115..27aafd7318 100644 --- a/web/src/components/core/ServerError.jsx +++ b/web/src/components/core/ServerError.jsx @@ -40,7 +40,7 @@ function ServerError() { icon={} /> - {_("Could not connect to the Agama server. Please, check whether it is running.")} + {_("Please, check whether it is running.")} From 82ac691f3bb59a488e68ce27bc0a6cd1c6052e24 Mon Sep 17 00:00:00 2001 From: Knut Anderssen Date: Tue, 28 May 2024 17:06:07 +0100 Subject: [PATCH 12/12] Fix ServerError test --- web/src/components/core/ServerError.test.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/core/ServerError.test.jsx b/web/src/components/core/ServerError.test.jsx index c6273b508f..db4fbaddd5 100644 --- a/web/src/components/core/ServerError.test.jsx +++ b/web/src/components/core/ServerError.test.jsx @@ -31,7 +31,7 @@ jest.mock("~/components/core/Sidebar", () => () =>
Agama sidebar
); describe("ServerError", () => { it("includes a generic server problem message", () => { plainRender(); - screen.getByText(/Could not connect to the Agama server/i); + screen.getByText(/Cannot connect to Agama server/i); }); it("calls location.reload when user clicks on 'Reload'", async () => {