diff --git a/web/src/App.jsx b/web/src/App.jsx index a3798d43f7..fbe3458e72 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 { ServerError, 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,7 @@ const ATTEMPTS = 3; */ function App() { const client = useInstallerClient(); - const { attempt } = useInstallerClientStatus(); + const { error } = useInstallerClientStatus(); const { products } = useProduct(); const { language } = useInstallerL10n(); const [status, setStatus] = useState(undefined); @@ -75,9 +72,8 @@ function App() { }, [client, setPhase, setStatus]); const Content = () => { - if (!client || !products) { - return (attempt > ATTEMPTS) ? : ; - } + if (error) return ; + if (!products) return ; if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) { return ; diff --git a/web/src/client/http.js b/web/src/client/http.js index 37a4ebdbef..d7d6c105d9 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 = 15; +const ATTEMPT_INTERVAL = 1000; + /** * Agama WebSocket client. * @@ -38,11 +55,70 @@ class WSClient { * @param {URL} url - Websocket URL. */ constructor(url) { - this.client = new WebSocket(url.toString()); - this.client.onmessage = (event) => { + this.url = url.toString(); + + this.handlers = { + error: [], + close: [], + open: [], + events: [] + }; + + this.reconnectAttempts = 0; + this.client = this.buildClient(); + } + + 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); + } + + buildClient() { + const client = new WebSocket(this.url); + client.onopen = () => { + console.log("Websocket connected"); + this.reconnectAttempts = 0; + clearTimeout(this.timeout); + + return this.dispatchOpenEvent(); + }; + + client.onmessage = (event) => { this.dispatchEvent(event); }; - this.handlers = []; + + client.onclose = () => { + console.log(`WebSocket closed`); + this.dispatchCloseEvent(); + this.timeout = setTimeout(() => this.connect(this.reconnectAttempts + 1), ATTEMPT_INTERVAL); + }; + + client.onerror = (e) => { + console.error("WebSocket error:", e); + this.dispatchErrorEvent(); + }; + + return client; + } + + connect(attempt = 0) { + this.reconnectAttempts = attempt; + if (attempt > MAX_ATTEMPTS) { + console.log("Max number of WebSocket connection attempts reached."); + return; + } + console.log(`Reconnecting WebSocket(attempt: ${attempt})`); + this.client = this.buildClient(); } /** @@ -55,10 +131,60 @@ class WSClient { * @return {RemoveFn} */ onEvent(func) { - this.handlers.push(func); + this.handlers.events.push(func); + return () => { + const position = this.handlers.events.indexOf(func); + if (position > -1) this.handlers.events.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.handlers.close.push(func); + return () => { - const position = this.handlers.indexOf(func); - if (position > -1) this.handlers.splice(position, 1); + const position = this.handlers.close.indexOf(func); + if (position > -1) this.handlers.close.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.handlers.open.push(func); + + return () => { + const position = this.handlers.open.indexOf(func); + if (position > -1) this.handlers.open.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.handlers.error.push(func); + + return () => { + const position = this.handlers.error.indexOf(func); + if (position > -1) this.handlers.error.splice(position, 1); }; } @@ -71,7 +197,34 @@ class WSClient { */ dispatchEvent(event) { const eventObject = JSON.parse(event.data); - this.handlers.forEach((f) => f(eventObject)); + this.handlers.events.forEach((f) => f(eventObject)); + } + + /** + * @private + * + * Dispatchs a close event by running all its handlers. + */ + dispatchCloseEvent() { + this.handlers.close.forEach((f) => f()); + } + + /** + * @private + * + * Dispatchs an error event by running all its handlers. + */ + dispatchErrorEvent() { + this.handlers.error.forEach((f) => f()); + } + + /** + * @private + * + * Dispatchs a close event by running all its handlers. + */ + dispatchOpenEvent() { + this.handlers.open.forEach((f) => f()); } } @@ -169,6 +322,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. * 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/ServerError.jsx similarity index 86% rename from web/src/components/core/DBusError.jsx rename to web/src/components/core/ServerError.jsx index 20c90c895c..27aafd7318 100644 --- a/web/src/components/core/DBusError.jsx +++ b/web/src/components/core/ServerError.jsx @@ -28,19 +28,19 @@ import { locationReload } from "~/utils"; const ErrorIcon = () => ; -function DBusError() { +function ServerError() { return ( // TRANSLATORS: page title - +
} /> - {_("Could not connect to the D-Bus service. Please, check whether it is running.")} + {_("Please, check whether it is running.")}
@@ -55,4 +55,4 @@ function DBusError() { ); } -export default DBusError; +export default ServerError; diff --git a/web/src/components/core/DBusError.test.jsx b/web/src/components/core/ServerError.test.jsx similarity index 82% rename from web/src/components/core/DBusError.test.jsx rename to web/src/components/core/ServerError.test.jsx index 360e471e11..db4fbaddd5 100644 --- a/web/src/components/core/DBusError.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 { DBusError } from "~/components/core"; +import { ServerError } 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("ServerError", () => { + it("includes a generic server problem message", () => { + plainRender(); + screen.getByText(/Cannot connect to 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 4b12c532a6..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 DBusError } from "./DBusError"; 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"; diff --git a/web/src/context/installer.jsx b/web/src/context/installer.jsx index 85e1bd7ba6..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, attempt: 0 }); +const InstallerClientStatusContext = React.createContext({ connected: false, error: false }); /** * Returns the D-Bus installer client @@ -48,7 +48,8 @@ function useInstallerClient() { * * @typedef {object} ClientStatus * @property {boolean} connected - whether the client is connected - * @property {number} attempt - number of attempt to connect + * @property {boolean} error - whether the client present an error and cannot + * reconnect * * @return {ClientStatus} installer client status */ @@ -61,8 +62,6 @@ function useInstallerClientStatus() { return context; } -const INTERVAL = 2000; - /** * @param {object} props * @param {import("~/client").InstallerClient|undefined} [props.client] client to connect to @@ -73,23 +72,16 @@ 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); + const [connected, setConnected] = useState(false); + const [error, setError] = useState(false); 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 +91,31 @@ 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; - return value.onDisconnect(() => setValue(null)); + value.onConnect(() => { + setConnected(true); + setError(false); + }); + + 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..25354eaf5e 100644 --- a/web/src/context/installer.test.jsx +++ b/web/src/context/installer.test.jsx @@ -27,16 +27,12 @@ import { InstallerClientProvider, useInstallerClientStatus } from "./installer"; jest.mock("~/client"); -let onDisconnectFn = jest.fn(); -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}`}
); @@ -46,59 +42,18 @@ describe("installer context", () => { beforeEach(() => { createDefaultClient.mockImplementation(() => { return { - isConnected: isConnectedFn, - onDisconnect: onDisconnectFn + onConnect: jest.fn(), + onDisconnect: jest.fn() }; }); }); - 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); - }); - - 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"); - }); + it("reports the status through the useInstallerClientStatus hook", async () => { + plainRender( + + + + ); + await screen.findByText("connected: false"); }); }); diff --git a/web/src/context/installerL10n.test.jsx b/web/src/context/installerL10n.test.jsx index 11794e002c..50af75fa4b 100644 --- a/web/src/context/installerL10n.test.jsx +++ b/web/src/context/installerL10n.test.jsx @@ -33,12 +33,13 @@ const getUILocaleFn = jest.fn().mockResolvedValue(); const setUILocaleFn = jest.fn().mockResolvedValue(); const client = { + onConnect: jest.fn(), + onDisconnect: jest.fn(), l10n: { getUILocale: getUILocaleFn, setUILocale: setUILocaleFn, getUIKeymap: jest.fn().mockResolvedValue("en"), }, - onDisconnect: jest.fn(), }; jest.mock("~/languages.json", () => ({ 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) {