When the IPC service gets terminated gracefully, the user must have
initiated some kind of action, be it an upgrade or an explicit "Stop the
service". In that case, there is no point in displaying an alert with an
info / error message as the user already knows that they are stopping
Firezone. In order to not fatigue the user with alerts, we exit the GUI
with a toast notification when the IPC service shuts down gracefully.
Toast notifications do not grab the users attention, allowing them to
continue what they are doing while still being notified that their
Firezone client is now disconnected.
Fixes: #6232.
At present, the GUI client uses a monolithic `Error` enum that
represents all kinds of errors. Some of them are unused (see #7956).
Others are only used during startup, like the `deep_link` and
`WebViewNotInstalled` variants. This makes it difficult to write correct
error handling code.
In addition to remove certain variants in #7965, this PR refactors the
`run::gui` function to not depend on this `Error` at all. Instead, we
use `anyhow::Result` and probe for particular errors that we want to
special-case. This is a bit less type-safe because there is no source
code-level connection between the source site that emits an error and
the error handling code.
In the worst case, any regression here is "just" a slight degradation in
UX: We will show a generic error dialog instead of a tailored message.
This risk is deemed acceptable in exchange for an easier to understand
control flow.
At present, the GUI client uses a separate task for reading messages
from the IPC connection and forwards them to another channel. The other
end of this channel is then used within the controller to actually react
to IPC messages.
We can simplify this by removing the intermediary task and processing
the messages from the IPC connection directly.
Currently, some errors are double-logged when we show them to the user
because of the `tracing::error!` statements within the generation of the
user-friendly error message for the error dialog.
To get rid of these, we generalise the `show_error_dialog` function to
take just the message and move the generation of the message to a
function on the `Error` itself. This also allows us to split out a
separate error type that is only used for the elevation check, thereby
reducing the complexity of the other error enum.
When `connlib` fails to establish a session, the GUI client currently
only captures the top-level error within `connect_to_firezone` because
it uses `.to_string()` for all errors. Unfortunately, that doesn't print
any of the sources of an error.
To conveniently capture all sources, we can use `anyhow` and its
alternate formatting using `format!("{e:#}")` (notice the `#`). Not all
errors within `connect_to_firezone` should be captured like this
however. Certain IO errors, in particular when trying to resolve the
domain of the portal, need to be captured separately because they may
resolve by themselves if we gain connectivity again. This is important,
otherwise we discard the users token when they boot-up a machine without
internet access yet Firezone is auto-starting.
To make this more ergonomic, we trim down `IpcServiceError` to two
variants: The IO variant we need to special-case and everything else.
This allows us to create `From` impls which "do the right thing" by
capturing more error information using `anyhow`'s alternate formatting.
The `expect` attribute is similar to `allow` in that it will silence a
particular lint. In addition to `allow` however, `expect` will fail as
soon as the lint is no longer emitted. This ensures we don't end up with
stale `allow` attributes in our codebase. Additionally, it provides a
way of adding a `reason` to document, why the lint is being suppressed.
This moves about 2/3rds of the code from `firezone-gui-client` to
`firezone-gui-client-common`.
I tested it in aarch64 Windows and cycled through sign-in and sign-out
and closing and re-opening the GUI process while the IPC service stays
running. IPC and updates each get their own MPSC channel in this, so I
wanted to be sure it didn't break.
---------
Signed-off-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Co-authored-by: Thomas Eizinger <thomas@eizinger.io>
