Improve error handling #53
Conversation
The loop will only stop if f() returned true. Therefore, we can predict its return code and don't need to call it once more. Signed-off-by: Jan Kiszka <[email protected]>
This also catches problems of the native messaging host. If it stalls, the browser will stall as well. 10 seconds should be enough time for it to react. Fail waitFor if that limit is reached and handle the failure at the caller sites. Signed-off-by: Jan Kiszka <[email protected]>
background.js
Outdated
| @@ -377,6 +377,7 @@ async function on_message_native(response) { | |||
| } else { | |||
| ssoLog("lost connection to broker"); | |||
| broker_online = false; | |||
| logout(); | |||
There was a problem hiding this comment.
This breaks on-demand dbus activation and was removed in 04c06aa#diff-bb06783fccd762e4d905ab922491e954a9fbaf708a1acbf53dff92872b0bcd3bL335.
For details, see #33.
There was a problem hiding this comment.
But it's still broken. We need to signal that the broker is offline, also visually.
There was a problem hiding this comment.
Not necessarily. It is fine if the broker is offline as it will get restarted on the next DBus call. If we logout, the next hit of the Entra login page will not even try to start it.
There was a problem hiding this comment.
But visualization is broken - I will fix that instead.
There was a problem hiding this comment.
We might give a slight indication that the broker is offline. But in the end, the user should not need to care and should also not be disturbed by that. For (caller activated) DBus services the caller should not assume anything about the current state of the service. Instead it should just make the call and check the return code - which indicates the error reason.
jan-kiszka
force-pushed
the
jan/error-handling
branch
from
751f620 to
471359c
Compare
Signaling availability makes no sense if the broker is offline as we cannot request or update the PRT in that case. Signed-off-by: Jan Kiszka <[email protected]>
jan-kiszka
force-pushed
the
jan/error-handling
branch
from
471359c to
5005cb9
Compare
|
Now it works perfectly - without logging out. |
|
I already merged the first two commits to master. The last one still needs changes to distinguish between the "mostly irrelevant" broker-currently-offline state and the logged-in state. |
Makes the addon more robust in case of connection losses to the broker or even the native messaging host.