Skip to content

Commit

Permalink
Reconnect the HTTP client websocket automatically (#1254)
Browse files Browse the repository at this point in the history
## Problem

When the current WebSocket is closed or there is an error the frontend
is not aware of it and does not react properly just stop receiving
notifications. It would be nice to reconnect silently reporting in case
that it is impossible to reconnect after a while.

- Trello Card:
https://trello.com/c/Kqaez0RH/3632-2-agama-adapt-connection-error-handling

## Solution

When the WebSocket is closed we will try to reconnect during a while.
From that point in advance the state of the socket will be unrecoverable
needing a user manual reconnection. See below the console log screenshot
and the error page shown in case the Websocket was not connected
automatically.

## Screenshots

![Screenshot from 2024-05-27
09-02-43](https://github.com/openSUSE/agama/assets/7056681/1b80cc31-96f0-4264-8175-32f28c8750b1)
![Screenshot from 2024-05-27
09-02-02](https://github.com/openSUSE/agama/assets/7056681/d08a796c-2351-4591-af55-36671e56fd6f)
  • Loading branch information
teclator authored May 28, 2024
2 parents 109cead + 82ac691 commit 7f8997d
Show file tree
Hide file tree
Showing 10 changed files with 258 additions and 123 deletions.
12 changes: 4 additions & 8 deletions web/src/App.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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);
Expand Down Expand Up @@ -75,9 +72,8 @@ function App() {
}, [client, setPhase, setStatus]);

const Content = () => {
if (!client || !products) {
return (attempt > ATTEMPTS) ? <DBusError /> : <Loading />;
}
if (error) return <ServerError />;
if (!products) return <Loading />;

if ((phase === STARTUP && status === BUSY) || phase === undefined || status === undefined) {
return <Loading />;
Expand Down
204 changes: 197 additions & 7 deletions web/src/client/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand All @@ -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();
}

/**
Expand All @@ -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);
};
}

Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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.
*
Expand Down
24 changes: 9 additions & 15 deletions web/src/client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -46,7 +46,9 @@ import { HTTPClient } from "./http";
* @property {() => Promise<Issues>} 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<boolean>} 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.
Expand Down Expand Up @@ -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,
Expand All @@ -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),
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,19 +28,19 @@ import { locationReload } from "~/utils";

const ErrorIcon = () => <Icon name="error" className="icon-xxxl" />;

function DBusError() {
function ServerError() {
return (
// TRANSLATORS: page title
<Page icon="problem" title={_("D-Bus Error")}>
<Page icon="problem" title={_("Agama Error")}>
<Center>
<EmptyState variant="xl">
<EmptyStateHeader
titleText={_("Cannot connect to D-Bus")}
titleText={_("Cannot connect to Agama server")}
headingLevel="h2"
icon={<EmptyStateIcon icon={ErrorIcon} />}
/>
<EmptyStateBody>
{_("Could not connect to the D-Bus service. Please, check whether it is running.")}
{_("Please, check whether it is running.")}
</EmptyStateBody>
</EmptyState>
</Center>
Expand All @@ -55,4 +55,4 @@ function DBusError() {
);
}

export default DBusError;
export default ServerError;
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => () => <div>Agama sidebar</div>);

describe("DBusError", () => {
it("includes a generic D-Bus connection problem message", () => {
plainRender(<DBusError />);
screen.getByText(/Could not connect to the D-Bus service/i);
describe("ServerError", () => {
it("includes a generic server problem message", () => {
plainRender(<ServerError />);
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(<DBusError />);
const { user } = plainRender(<ServerError />);
const reloadButton = await screen.findByRole("button", { name: /Reload/i });
await user.click(reloadButton);
expect(utils.locationReload).toHaveBeenCalled();
Expand Down
Loading

0 comments on commit 7f8997d

Please sign in to comment.