Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconnect the HTTP client websocket automatically #1254

Merged
merged 12 commits into from
May 28, 2024
Merged

Conversation

teclator
Copy link
Contributor

@teclator teclator commented May 24, 2024

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.

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
Screenshot from 2024-05-27 09-02-02

web/src/client/http.js Outdated Show resolved Hide resolved
web/src/client/http.js Outdated Show resolved Hide resolved
web/src/client/http.js Outdated Show resolved Hide resolved
web/src/client/http.js Outdated Show resolved Hide resolved
@teclator teclator marked this pull request as ready for review May 28, 2024 08:58
headingLevel="h2"
icon={<EmptyStateIcon icon={ErrorIcon} />}
/>
<EmptyStateBody>
{_("Could not connect to the D-Bus service. Please, check whether it is running.")}
{_("Could not connect to the Agama server. Please, check whether it is running.")}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say this part of the message is not useful at all. The first sentence is basically a repetition of the bigger "Cannot connect to...". And, as a user, how can I check whether the service is running?

By now, I would remove the first sentence. About the second part, we should link to some page explaining how to proceed. But that's out of the scope of this card.

Copy link
Contributor Author

@teclator teclator May 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, maybe we could maintain the first part and in the second part show some info about the different errors although by now we are only reporting it in case of a websocket connection error..

@teclator teclator merged commit 7f8997d into master May 28, 2024
2 checks passed
@teclator teclator deleted the force_reconnect branch May 28, 2024 16:12
imobachgs added a commit that referenced this pull request May 29, 2024
The `Probe` D-Bus function is a blocking one. It causes the call `POST
/api/manager/probe` to be a long-lived request which is causing some
problems (see #1254).

The proposed fix is to return immediately and run the request as a
separate Tokio task. After all, we will get an update on the status
through a WebSocket. If needed, we could also emit an error through the
WebSocket.
@imobachgs imobachgs mentioned this pull request Jun 27, 2024
imobachgs added a commit that referenced this pull request Jun 27, 2024
Prepare for releasing Agama 9. It includes the following pull requests:

- #1101
- #1202
- #1228
- #1231
- #1236
- #1238
- #1239
- #1240
- #1242
- #1243
- #1244
- #1245
- #1246
- #1247
- #1248
- #1249
- #1250
- #1251
- #1252
- #1253
- #1254
- #1255
- #1256
- #1257
- #1258
- #1259
- #1260
- #1261
- #1264
- #1265
- #1267
- #1268
- #1269
- #1270
- #1271
- #1272
- #1273
- #1274
- #1279
- #1280
- #1284
- #1285
- #1286
- #1287
- #1288
- #1289
- #1290
- #1291
- #1292
- #1293
- #1294
- #1295
- #1296
- #1298
- #1299
- #1300
- #1301
- #1302
- #1303
- #1304
- #1305
- #1306
- #1307
- #1308
- #1309
- #1310
- #1311
- #1312
- #1313
- #1314
- #1315
- #1316
- #1317
- #1318
- #1319
- #1320
- #1321
- #1322
- #1323
- #1324
- #1325
- #1326
- #1328
- #1329
- #1331
- #1332
- #1334
- #1338
- #1340
- #1341
- #1342
- #1343
- #1344
- #1345
- #1348
- #1349
- #1351
- #1352
- #1353
- #1354
- #1355
- #1356
- #1357
- #1358
- #1359
- #1360
- #1361
- #1362
- #1363
- #1365
- #1366
- #1367
- #1368
- #1371
- #1372
- #1374
- #1375
- #1376
- #1379
- #1380
- #1381
- #1383
- #1384
- #1385
- #1386
- #1387
- #1388
- #1389
- #1391
- #1392
- #1394
- #1395
- #1397
- #1398
- #1399
- #1400
- #1403
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants