[geary/wip/17-noisy-problem-reports: 1/12] Provide common service status tracking via ClientService
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/17-noisy-problem-reports: 1/12] Provide common service status tracking via ClientService
- Date: Tue, 1 Jan 2019 11:29:48 +0000 (UTC)
commit 827db39fe2a802986b01ed8b08b7e8737bd48cf3
Author: Michael Gratton <mike vee net>
Date: Sat Dec 29 16:18:39 2018 +1100
Provide common service status tracking via ClientService
This adds a `current_status` property to ClientService to track the
service's operational state, wich can be monitored by both the engine
and clients for changes. Hook up subclasses to keep that update, and
move connectivity management in to ClientService to provide a common
implementation.
src/engine/api/geary-client-service.vala | 321 +++++++++++++++++++--
.../imap-engine/imap-engine-generic-account.vala | 42 ++-
.../imap-engine/imap-engine-minimal-folder.vala | 16 +-
src/engine/imap/api/imap-client-service.vala | 206 ++++---------
src/engine/smtp/smtp-client-service.vala | 105 ++-----
test/engine/api/geary-account-mock.vala | 8 +
6 files changed, 414 insertions(+), 284 deletions(-)
---
diff --git a/src/engine/api/geary-client-service.vala b/src/engine/api/geary-client-service.vala
index c0617205..16b07c61 100644
--- a/src/engine/api/geary-client-service.vala
+++ b/src/engine/api/geary-client-service.vala
@@ -8,32 +8,156 @@
/**
* Manages client connections to a specific network service.
*
- * Derived classes are used by accounts to manage client connections
- * to a specific network service, such as IMAP or SMTP. This class
- * does connect to the service itself, rather manages the
- * configuration and life-cycle of client sessions that do connect to
- * the service.
+ * Client service object are used by accounts to manage client
+ * connections to a specific network service, such as IMAP or SMTP
+ * services. This abstract class does not connect to the service
+ * itself, rather manages the configuration, status tracking, and
+ * life-cycle of concrete implementations.
*/
public abstract class Geary.ClientService : BaseObject {
+ private const int BECAME_REACHABLE_TIMEOUT_SEC = 1;
+ private const int BECAME_UNREACHABLE_TIMEOUT_SEC = 3;
+
+
/**
- * The service's account.
+ * Denotes the service's current status.
+ *
+ * @see ClientService.current_status
*/
- public AccountInformation account { get; private set; }
+ public enum Status {
+
+ /**
+ * The service is currently offline.
+ *
+ * This is the initial state, and will only change after
+ * having successfully connected to the remote service. An
+ * attempt to connect will be made when the connectivity
+ * manager indicates the network has changed.
+ */
+ OFFLINE,
+
+ /** A connection has been established and is operating normally. */
+ CONNECTED,
+
+ /**
+ * A network problem occurred connecting to the service.
+ *
+ * This is caused by DNS lookup failures, connectivity
+ * failures, the service rejecting the connection due to
+ * connection limits, or the service configuration being out
+ * of date.
+ *
+ * The {@link connection_error} signal will be fired with an
+ * error (if any), and an attempt to re-connect will be made
+ * when the connectivity manager indicates the network has
+ * changed. It may also require manual intervention to update
+ * the service's configuration to successfully re-connect,
+ * however.
+ */
+ CONNECTION_FAILED,
+
+ /**
+ * The service's credentials were rejected by the remote service.
+ *
+ * The {@link AccountInformation.authentication_failure}
+ * signal on the service's account configuration will be fired
+ * and no more connection attempts will be made until the
+ * service is restarted.
+ */
+ AUTHENTICATION_FAILED,
+
+ /**
+ * The remote service's TLS certificate was rejected.
+ *
+ * The {@link AccountInformation.untrusted_host} signal on the
+ * service's account configuration will be fired and no more
+ * connection attempts will be made until the service is
+ * restarted.
+ */
+ TLS_VALIDATION_FAILED,
+
+ /**
+ * A general problem occurred with the remote service.
+ *
+ * A network connection was successfully established, but some
+ * problem other than authentication or TLS certificate
+ * validation has prevented a successful connection. This may
+ * be because of an unsupported protocol version or other
+ * general incompatibility.
+ *
+ * The {@link unrecoverable_error} signal will be fired with
+ * an error (if any), and no more connection attempts will be
+ * made until the service is restarted.
+ */
+ UNRECOVERABLE_ERROR;
+
+
+ /**
+ * Determines if re-connection should be attempted from this state.
+ *
+ * If the service is in this state, it will automatically
+ * attempt to reconnect when connectivity changes have been
+ * detected.
+ */
+ public bool automatically_reconnect() {
+ return (
+ this == OFFLINE ||
+ this == CONNECTED ||
+ this == CONNECTION_FAILED
+ );
+ }
+
+ public string to_value() {
+ return ObjectUtils.to_enum_nick<Status>(typeof(Status), this);
+ }
+
+ }
+
/**
- * The configuration for the service.
+ * Fired when the service encounters a connection error.
+ *
+ * @see Status.CONNECTION_FAILED
+ */
+ public signal void connection_error(GLib.Error? err);
+
+ /**
+ * Fired when the service encounters an unrecoverable error.
+ *
+ * @see Status.UNRECOVERABLE_ERROR
*/
+ public signal void unrecoverable_error(GLib.Error? err);
+
+
+ /** The service's account. */
+ public AccountInformation account { get; private set; }
+
+ /** The configuration for the service. */
public ServiceInformation configuration { get; private set; }
/**
- * The network endpoint the service will connect to.
+ * The service's current status.
+ *
+ * The current state of certain aspects of the service
+ * (e.g. online/offline state may not be fully known, and hence
+ * the value of this property reflects the engine's current
+ * understanding of the service's status, not necessarily reality.
*/
+ public Status current_status { get; protected set; default = OFFLINE; }
+
+ /** The network endpoint the service will connect to. */
public Endpoint remote { get; private set; }
- /** Determines if this manager has been started. */
- public bool is_running { get; protected set; default = false; }
+ /** Determines if this service has been started. */
+ public bool is_running { get; private set; default = false; }
+
+ // Since the connectivity manager can flip-flop rapidly, introduce
+ // some hysteresis on connectivity changes to smooth out the
+ // transitions.
+ private TimeoutManager became_reachable_timer;
+ private TimeoutManager became_unreachable_timer;
protected ClientService(AccountInformation account,
@@ -42,6 +166,22 @@ public abstract class Geary.ClientService : BaseObject {
this.account = account;
this.configuration = configuration;
this.remote = remote;
+
+ this.became_reachable_timer = new TimeoutManager.seconds(
+ BECAME_REACHABLE_TIMEOUT_SEC, became_reachable
+ );
+ this.became_unreachable_timer = new TimeoutManager.seconds(
+ BECAME_UNREACHABLE_TIMEOUT_SEC, became_unreachable
+ );
+
+ connect_handlers();
+
+ this.notify["running"].connect(on_running_notify);
+ this.notify["current-status"].connect(on_current_status_notify);
+ }
+
+ ~ClientService() {
+ disconnect_handlers();
}
/**
@@ -55,9 +195,7 @@ public abstract class Geary.ClientService : BaseObject {
Endpoint remote,
GLib.Cancellable? cancellable)
throws GLib.Error {
- if (this.remote != null) {
- this.remote.untrusted_host.disconnect(on_untrusted_host);
- }
+ disconnect_handlers();
bool do_restart = this.is_running;
if (do_restart) {
@@ -66,7 +204,7 @@ public abstract class Geary.ClientService : BaseObject {
this.configuration = configuration;
this.remote = remote;
- this.remote.untrusted_host.connect(on_untrusted_host);
+ connect_handlers();
if (do_restart) {
yield start(cancellable);
@@ -90,10 +228,159 @@ public abstract class Geary.ClientService : BaseObject {
public abstract async void stop(GLib.Cancellable? cancellable = null)
throws GLib.Error;
+ /**
+ * Called when the network service has become reachable.
+ *
+ * Derived classes may wish to attempt to establish a network
+ * connection to the remote service when this is called.
+ */
+ protected abstract void became_reachable();
- private void on_untrusted_host(TlsNegotiationMethod method,
+ /**
+ * Called when the network service has become unreachable.
+ *
+ * Derived classes should close any network connections that are
+ * being established, or have been established with remote
+ * service.
+ */
+ protected abstract void became_unreachable();
+
+ /**
+ * Notifies the network service has been started.
+ *
+ * Derived classes must call this when they consider the service
+ * to has been successfully started, to update service status and
+ * start reachable checking.
+ */
+ protected void notify_started() {
+ this.is_running = true;
+ if (this.remote.connectivity.is_reachable.is_certain()) {
+ became_reachable();
+ } else {
+ this.remote.connectivity.check_reachable.begin();
+ }
+ }
+
+ /**
+ * Notifies when the network service has been stopped.
+ *
+ * Derived classes must call this before stopping the service to
+ * update service status and cancel any pending reachable checks.
+ */
+ protected void notify_stopped() {
+ this.is_running = false;
+ this.current_status = OFFLINE;
+ this.became_reachable_timer.reset();
+ this.became_unreachable_timer.reset();
+ }
+
+ /**
+ * Notifies that the service has successfully connected.
+ *
+ * Derived classes should call this when a connection to the
+ * network service has been successfully negotiated and appears to
+ * be operating normally.
+ */
+ protected void notify_connected() {
+ this.current_status = CONNECTED;
+ }
+
+ /**
+ * Notifies that a connection error occurred.
+ *
+ * Derived classes should call this when a connection to the
+ * network service encountered some network error other than a
+ * login failure or TLS certificate validation error.
+ */
+ protected void notify_connection_failed(GLib.Error? err) {
+ this.current_status = CONNECTION_FAILED;
+ connection_error(err);
+ }
+
+ /**
+ * Notifies that an authentication failure has occurred.
+ *
+ * Derived classes should call this when they have detected that
+ * authentication has failed because the service rejected the
+ * supplied credentials, but not when login failed for other
+ * reasons (for example, connection limits being reached, service
+ * temporarily unavailable, etc).
+ */
+ protected void notify_authentication_failed() {
+ this.current_status = AUTHENTICATION_FAILED;
+ }
+
+ /**
+ * Notifies that an unrecoverable error has occurred.
+ *
+ * Derived classes should call this when they have detected that
+ * some unrecoverable error has occurred when connecting to the
+ * service, such as an unsupported protocol or version.
+ */
+ protected void notify_unrecoverable_error(GLib.Error? err) {
+ this.current_status = UNRECOVERABLE_ERROR;
+ unrecoverable_error(err);
+ }
+
+ private void connect_handlers() {
+ this.remote.connectivity.notify["is-reachable"].connect(
+ on_connectivity_change
+ );
+ this.remote.connectivity.address_error_reported.connect(
+ on_connectivity_error
+ );
+ this.remote.untrusted_host.connect(on_untrusted_host);
+ }
+
+ private void disconnect_handlers() {
+ this.remote.connectivity.notify["is-reachable"].disconnect(
+ on_connectivity_change
+ );
+ this.remote.connectivity.address_error_reported.disconnect(
+ on_connectivity_error
+ );
+ this.remote.untrusted_host.disconnect(on_untrusted_host);
+ }
+
+ private void on_running_notify() {
+ debug(this.is_running ? "started" : "stopped");
+ }
+
+ private void on_current_status_notify() {
+ debug(this.current_status.to_value());
+ }
+
+ private void on_connectivity_change() {
+ if (this.is_running && this.current_status.automatically_reconnect()) {
+ if (this.remote.connectivity.is_reachable.is_certain()) {
+ this.became_reachable_timer.start();
+ this.became_unreachable_timer.reset();
+ } else {
+ this.current_status = OFFLINE;
+ this.became_unreachable_timer.start();
+ this.became_reachable_timer.reset();
+ }
+ }
+ }
+
+ private void on_connectivity_error(Error error) {
+ if (this.is_running) {
+ this.current_status = CONNECTION_FAILED;
+ this.became_reachable_timer.reset();
+ this.became_unreachable_timer.reset();
+ became_unreachable();
+ }
+ }
+
+ private void on_untrusted_host(Geary.TlsNegotiationMethod method,
GLib.TlsConnection cx) {
- this.account.untrusted_host(this.configuration, method, cx);
+ if (this.is_running) {
+ this.current_status = TLS_VALIDATION_FAILED;
+ this.became_reachable_timer.reset();
+ this.became_unreachable_timer.reset();
+ became_unreachable();
+ this.account.untrusted_host(this.configuration, method, cx);
+ }
}
}
diff --git a/src/engine/imap-engine/imap-engine-generic-account.vala
b/src/engine/imap-engine/imap-engine-generic-account.vala
index 2bde62d3..d5c5eab0 100644
--- a/src/engine/imap-engine/imap-engine-generic-account.vala
+++ b/src/engine/imap-engine/imap-engine-generic-account.vala
@@ -74,9 +74,10 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
config, config.incoming, incoming_remote
);
this.imap.min_pool_size = IMAP_MIN_POOL_SIZE;
- this.imap.ready.connect(on_pool_session_ready);
- this.imap.connection_failed.connect(on_pool_connection_failed);
this.imap.login_failed.connect(on_pool_login_failed);
+ this.imap.notify["current-status"].connect(
+ on_remote_status_notify
+ );
this.smtp = new Smtp.ClientService(
config,
@@ -1016,29 +1017,6 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
}
}
- private void on_pool_session_ready(bool is_ready) {
- this.is_online = is_ready;
- if (is_ready) {
- // Now have a valid session, so credentials must be good
- this.authentication_failures = 0;
- this.remote_ready_lock.blind_notify();
- update_remote_folders();
- } else {
- this.remote_ready_lock.reset();
- this.refresh_folder_timer.reset();
- }
- }
-
- private void on_pool_connection_failed(Error error) {
- this.remote_ready_lock.reset();
- if (error is ImapError.UNAUTHENTICATED) {
- // This is effectively a login failure
- on_pool_login_failed(null);
- } else {
- notify_imap_problem(ProblemType.CONNECTION_ERROR, error);
- }
- }
-
private void on_pool_login_failed(Geary.Imap.StatusResponse? response) {
this.remote_ready_lock.reset();
this.authentication_failures++;
@@ -1092,6 +1070,20 @@ private abstract class Geary.ImapEngine.GenericAccount : Geary.Account {
}
}
+ private void on_remote_status_notify() {
+ if (this.open) {
+ if (this.imap.current_status == CONNECTED) {
+ this.is_online = true;
+ this.remote_ready_lock.blind_notify();
+ update_remote_folders();
+ } else {
+ this.is_online = false;
+ this.remote_ready_lock.reset();
+ this.refresh_folder_timer.reset();
+ }
+ }
+ }
+
}
diff --git a/src/engine/imap-engine/imap-engine-minimal-folder.vala
b/src/engine/imap-engine/imap-engine-minimal-folder.vala
index 5baf1cb4..ed3d9f09 100644
--- a/src/engine/imap-engine/imap-engine-minimal-folder.vala
+++ b/src/engine/imap-engine/imap-engine-minimal-folder.vala
@@ -781,7 +781,9 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// manipulate), no remote connection will ever be made,
// meaning that folder normalization never happens and
// unsolicited notifications never arrive
- this._account.imap.ready.connect(on_remote_ready);
+ this._account.imap.notify["current-status"].connect(
+ on_remote_status_notify
+ );
if (open_flags.is_all_set(OpenFlags.NO_DELAY)) {
this.open_remote_session.begin();
} else {
@@ -822,7 +824,9 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// Ensure we don't attempt to start opening a remote while
// closing
- this._account.imap.ready.disconnect(on_remote_ready);
+ this._account.imap.notify["current-status"].disconnect(
+ on_remote_status_notify
+ );
this.remote_open_timer.reset();
// Stop any internal tasks from running
@@ -906,7 +910,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// Ensure we are open already and guard against someone
// else having called this just before we did.
if (this.open_count > 0 &&
- this._account.imap.is_ready &&
+ this._account.imap.current_status == CONNECTED &&
this.remote_session == null) {
this.opening_monitor.notify_start();
@@ -1532,8 +1536,8 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
);
}
- private void on_remote_ready(bool is_ready) {
- if (is_ready) {
+ private void on_remote_status_notify() {
+ if (this._account.imap.current_status == CONNECTED) {
this.open_remote_session.begin();
}
}
@@ -1554,7 +1558,7 @@ private class Geary.ImapEngine.MinimalFolder : Geary.Folder, Geary.FolderSupport
// occurred, but the folder is still open and so is
// the pool, try re-establishing the connection.
if (is_error &&
- this._account.imap.is_ready &&
+ this._account.imap.current_status == CONNECTED &&
!this.open_cancellable.is_cancelled()) {
this.open_remote_session.begin();
}
diff --git a/src/engine/imap/api/imap-client-service.vala b/src/engine/imap/api/imap-client-service.vala
index e37b55eb..9227a7d9 100644
--- a/src/engine/imap/api/imap-client-service.vala
+++ b/src/engine/imap/api/imap-client-service.vala
@@ -24,19 +24,8 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
private const int DEFAULT_MIN_POOL_SIZE = 1;
private const int DEFAULT_MAX_FREE_SIZE = 1;
- private const int POOL_START_TIMEOUT_SEC = 1;
- private const int POOL_STOP_TIMEOUT_SEC = 3;
private const int CHECK_NOOP_THRESHOLD_SEC = 5;
- /**
- * Determines if the manager has a working connection.
- *
- * This will be true once at least one connection has been
- * established, and after the server has become reachable again
- * after being unreachable.
- */
- public bool is_ready { get; private set; default = false; }
-
/**
* Set to zero or negative value if keepalives should be disabled when a connection has not
* selected a mailbox. (This is not recommended.)
@@ -100,25 +89,9 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
private Nonblocking.Queue<ClientSession> free_queue =
new Nonblocking.Queue<ClientSession>.fifo();
- private TimeoutManager pool_start;
- private TimeoutManager pool_stop;
private Cancellable? pool_cancellable = null;
private bool authentication_failed = false;
- private bool untrusted_host = false;
-
- /**
- * Fired when the manager's ready state changes.
- *
- * This will be fired after opening if online and once at least
- * one connection has been established, after the server has
- * become reachable again after being unreachable, and if the
- * server becomes unreachable.
- */
- public signal void ready(bool is_ready);
-
- /** Fired when a network or non-auth error occurs opening a session. */
- public signal void connection_failed(Error err);
/** Fired when an authentication error occurs opening a session. */
public signal void login_failed(StatusResponse? response);
@@ -128,16 +101,6 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
ServiceInformation service,
Endpoint remote) {
base(account, service, remote);
-
- this.pool_start = new TimeoutManager.seconds(
- POOL_START_TIMEOUT_SEC,
- () => { this.check_pool.begin(); }
- );
-
- this.pool_stop = new TimeoutManager.seconds(
- POOL_STOP_TIMEOUT_SEC,
- () => { this.close_pool.begin(); }
- );
}
/**
@@ -151,25 +114,9 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
);
}
- this.is_running = true;
this.authentication_failed = false;
this.pool_cancellable = new Cancellable();
-
- this.remote.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].connect(
- on_imap_trust_untrusted_host
- );
- this.remote.untrusted_host.connect(on_imap_untrusted_host);
- this.remote.connectivity.notify["is-reachable"].connect(
- on_connectivity_change
- );
- this.remote.connectivity.address_error_reported.connect(
- on_connectivity_error
- );
- if (this.remote.connectivity.is_reachable.is_certain()) {
- this.check_pool.begin();
- } else {
- this.remote.connectivity.check_reachable.begin();
- }
+ notify_started();
}
/**
@@ -181,20 +128,9 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
return;
}
- this.is_running = false;
- this.pool_cancellable.cancel();
-
- this.remote.notify[Endpoint.PROP_TRUST_UNTRUSTED_HOST].disconnect(
- on_imap_trust_untrusted_host
- );
- this.remote.untrusted_host.disconnect(on_imap_untrusted_host);
- this.remote.connectivity.notify["is-reachable"].disconnect(
- on_connectivity_change
- );
- this.remote.connectivity.address_error_reported.disconnect(
- on_connectivity_error
- );
+ notify_stopped();
+ this.pool_cancellable.cancel();
yield close_pool();
// TODO: This isn't the best (deterministic) way to deal with
@@ -229,20 +165,27 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
* @throws ImapError.UNAVAILABLE if the IMAP endpoint is not
* trusted or is not reachable.
*/
- public async ClientSession claim_authorized_session_async(Cancellable? cancellable)
- throws Error {
- check_open();
- debug("[%s] Claiming session with %d of %d free",
- this.account.id, this.free_queue.size, this.all_sessions.size);
+ public async ClientSession
+ claim_authorized_session_async(GLib.Cancellable? cancellable)
+ throws GLib.Error {
+ if (!this.is_running) {
+ throw new EngineError.OPEN_REQUIRED(
+ "IMAP client service is not running"
+ );
+ }
- if (this.authentication_failed)
- throw new ImapError.UNAUTHENTICATED("Invalid ClientSessionManager credentials");
+ debug("Claiming session with %d of %d free",
+ this.free_queue.size, this.all_sessions.size);
- if (this.untrusted_host)
- throw new ImapError.UNAVAILABLE("Untrusted host %s", remote.to_string());
+ if (this.current_status == AUTHENTICATION_FAILED) {
+ throw new ImapError.UNAUTHENTICATED("Invalid credentials");
+ }
- if (!this.remote.connectivity.is_reachable.is_certain())
- throw new ImapError.UNAVAILABLE("Host at %s is unreachable", remote.to_string());
+ if (this.current_status == TLS_VALIDATION_FAILED) {
+ throw new ImapError.UNAVAILABLE(
+ "Untrusted host %s", this.remote.to_string()
+ );
+ }
ClientSession? claimed = null;
while (claimed == null) {
@@ -314,35 +257,29 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
}
}
- private void check_open() throws Error {
- if (!this.is_running) {
- throw new EngineError.OPEN_REQUIRED(
- "IMAP client service is not running"
- );
- }
+ /** Restarts the client session pool. */
+ protected override void became_reachable() {
+ this.check_pool.begin(false);
}
- private async void check_pool(bool is_claiming = false) {
- debug("[%s] Checking session pool with %d of %d free",
- this.account.id, this.free_queue.size, this.all_sessions.size);
-
- this.pool_start.reset();
+ /** Closes the client session pool. */
+ protected override void became_unreachable() {
+ this.close_pool.begin();
+ }
- if (this.is_running &&
- !this.authentication_failed &&
- !this.untrusted_host &&
- this.remote.connectivity.is_reachable.is_certain()) {
+ private async void check_pool(bool is_claiming) {
+ debug("Checking session pool with %d of %d free",
+ this.free_queue.size, this.all_sessions.size);
- int needed = this.min_pool_size - this.all_sessions.size;
- if (needed <= 0 && is_claiming) {
- needed = 1;
- }
+ int needed = this.min_pool_size - this.all_sessions.size;
+ if (needed <= 0 && is_claiming) {
+ needed = 1;
+ }
- // Open as many as needed in parallel
- while (needed > 0) {
- add_pool_session.begin();
- needed--;
- }
+ // Open as many as needed in parallel
+ while (needed > 0) {
+ add_pool_session.begin();
+ needed--;
}
}
@@ -434,9 +371,9 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
try {
yield new_session.connect_async(cancellable);
- } catch (Error err) {
+ } catch (GLib.Error err) {
if (!(err is IOError.CANCELLED)) {
- connection_failed(err);
+ notify_connection_failed(err);
}
throw err;
}
@@ -445,9 +382,12 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
yield new_session.initiate_session_async(
this.configuration.credentials, cancellable
);
+ } catch (ImapError.UNAUTHENTICATED err) {
+ notify_authentication_failed();
+ throw err;
} catch (Error err) {
if (!(err is IOError.CANCELLED)) {
- connection_failed(err);
+ notify_connection_failed(err);
}
// need to disconnect before throwing error ... don't honor Cancellable here, it's
@@ -469,23 +409,13 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
unselected_keepalive_sec,
selected_with_idle_keepalive_sec);
- // We now have a good connection, so signal us as ready if not
- // already done so.
- if (!this.is_ready) {
- debug("[%s] Became ready", this.account.id);
- notify_ready(true);
- }
-
+ notify_connected();
return new_session;
}
private async void close_pool() {
- debug("[%s] Closing the pool, disconnecting %d sessions",
- this.account.id, this.all_sessions.size);
-
- this.pool_start.reset();
- this.pool_stop.reset();
- notify_ready(false);
+ debug("Closing the pool, disconnecting %d sessions",
+ this.all_sessions.size);
// Take a copy and work off that while scheduling disconnects,
// since as they disconnect they'll remove themselves from the
@@ -539,11 +469,6 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
return removed;
}
- private void notify_ready(bool is_ready) {
- this.is_ready = is_ready;
- ready(is_ready);
- }
-
private void on_disconnected(ClientSession session, ClientSession.DisconnectReason reason) {
this.remove_session_async.begin(
session,
@@ -559,42 +484,9 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
}
private void on_login_failed(ClientSession session, StatusResponse? response) {
- this.authentication_failed = true;
login_failed(response);
+ notify_authentication_failed();
this.close_pool.begin();
}
- private void on_imap_untrusted_host() {
- this.untrusted_host = true;
- this.close_pool.begin();
- }
-
- private void on_imap_trust_untrusted_host() {
- // fired when the trust_untrusted_host property changes, indicating if the user has agreed
- // to ignore the trust problems and continue connecting
- if (untrusted_host && remote.trust_untrusted_host == Trillian.TRUE) {
- untrusted_host = false;
-
- if (this.is_running) {
- check_pool.begin();
- }
- }
- }
-
- private void on_connectivity_change() {
- bool is_reachable = this.remote.connectivity.is_reachable.is_certain();
- if (is_reachable) {
- this.pool_start.start();
- this.pool_stop.reset();
- } else {
- this.pool_start.reset();
- this.pool_stop.start();
- }
- }
-
- private void on_connectivity_error(Error error) {
- connection_failed(error);
- this.close_pool.begin();
- }
-
}
diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala
index d7eb1f17..859dcf8f 100644
--- a/src/engine/smtp/smtp-client-service.vala
+++ b/src/engine/smtp/smtp-client-service.vala
@@ -14,11 +14,6 @@
*/
internal class Geary.Smtp.ClientService : Geary.ClientService {
- // Min and max times between attempting to re-send after a
- // connection failure.
- private const uint MIN_SEND_RETRY_INTERVAL_SEC = 4;
- private const uint MAX_SEND_RETRY_INTERVAL_SEC = 64;
-
// Used solely for debugging, hence "(no subject)" not marked for translation
private static string message_subject(RFC822.Message message) {
@@ -63,16 +58,9 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
*/
public override async void start(GLib.Cancellable? cancellable = null)
throws GLib.Error {
- this.is_running = true;
yield this.outbox.open_async(Folder.OpenFlags.NONE, cancellable);
yield this.fill_outbox_queue(cancellable);
- this.remote.connectivity.notify["is-reachable"].connect(
- on_reachable_changed
- );
- this.remote.connectivity.address_error_reported.connect(
- on_connectivity_error
- );
- this.remote.connectivity.check_reachable.begin();
+ notify_started();
}
/**
@@ -80,12 +68,7 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
*/
public override async void stop(GLib.Cancellable? cancellable = null)
throws GLib.Error {
- this.remote.connectivity.notify["is-reachable"].disconnect(
- on_reachable_changed
- );
- this.remote.connectivity.address_error_reported.disconnect(
- on_connectivity_error
- );
+ notify_stopped();
this.stop_postie();
// Wait for the postie to actually stop before closing the
// folder so w don't interrupt e.g. sending/saving/deleting
@@ -95,7 +78,6 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
yield;
}
yield this.outbox.close_async(cancellable);
- this.is_running = false;
}
/**
@@ -112,6 +94,16 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
this.outbox_queue.send(id);
}
+ /** Starts the postie delivering messages. */
+ protected override void became_reachable() {
+ this.start_postie.begin();
+ }
+
+ /** Stops the postie delivering. */
+ protected override void became_unreachable() {
+ this.stop_postie();
+ }
+
/**
* Starts delivery of messages in the queue.
*/
@@ -123,7 +115,6 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
Cancellable cancellable = this.queue_cancellable =
new GLib.Cancellable();
- uint send_retry_seconds = MIN_SEND_RETRY_INTERVAL_SEC;
// Start the send queue.
while (!cancellable.is_cancelled()) {
@@ -132,54 +123,30 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
bool email_handled = false;
try {
id = yield this.outbox_queue.receive(cancellable);
- email_handled = yield process_email(id, cancellable);
+ yield process_email(id, cancellable);
+ email_handled = true;
} catch (SmtpError err) {
- ProblemType problem = ProblemType.GENERIC_ERROR;
if (err is SmtpError.AUTHENTICATION_FAILED) {
- problem = ProblemType.LOGIN_FAILED;
- } else if (err is SmtpError.STARTTLS_FAILED) {
- problem = ProblemType.CONNECTION_ERROR;
- } else if (err is SmtpError.NOT_CONNECTED) {
- problem = ProblemType.NETWORK_ERROR;
+ notify_authentication_failed();
+ } else if (err is SmtpError.STARTTLS_FAILED ||
+ err is SmtpError.NOT_CONNECTED) {
+ notify_connection_failed(err);
} else if (err is SmtpError.PARSE_ERROR ||
err is SmtpError.SERVER_ERROR ||
err is SmtpError.NOT_SUPPORTED) {
- problem = ProblemType.SERVER_ERROR;
+ notify_unrecoverable_error(err);
}
- notify_report_problem(problem, err);
- cancellable.cancel();
- } catch (IOError.CANCELLED err) {
- // Nothing to do here — we're already cancelled. In
- // particular we don't want to report the cancelled
- // error as a problem since this is the normal
- // shutdown method.
- } catch (IOError err) {
- notify_report_problem(ProblemType.for_ioerror(err), err);
cancellable.cancel();
- } catch (Error err) {
- notify_report_problem(ProblemType.GENERIC_ERROR, err);
+ } catch (GLib.IOError.CANCELLED err) {
+ // Nothing to do here — we're already cancelled.
+ } catch (GLib.Error err) {
+ notify_connection_failed(err);
cancellable.cancel();
}
- if (email_handled) {
- // send was good, reset nap length
- send_retry_seconds = MIN_SEND_RETRY_INTERVAL_SEC;
- } else {
- // send was bad, try sending again later
- if (id != null) {
- this.outbox_queue.send(id);
- }
-
- if (!cancellable.is_cancelled()) {
- debug("Outbox napping for %u seconds...", send_retry_seconds);
- // Take a brief nap before continuing to allow
- // connection problems to resolve.
- yield Geary.Scheduler.sleep_async(send_retry_seconds);
- send_retry_seconds = Geary.Numeric.uint_ceiling(
- send_retry_seconds * 2,
- MAX_SEND_RETRY_INTERVAL_SEC
- );
- }
+ if (!email_handled && id != null) {
+ // Send was bad, try sending again later
+ this.outbox_queue.send(id);
}
}
@@ -297,7 +264,6 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
yield save_sent_mail_async(email, cancellable);
} catch (Error err) {
debug("Outbox postie: Error saving sent mail: %s", err.message);
- notify_report_problem(ProblemType.SEND_EMAIL_SAVE_FAILED, err);
return false;
}
}
@@ -407,23 +373,4 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
}
}
- private void notify_report_problem(ProblemType problem, Error? err) {
- report_problem(
- new ServiceProblemReport(problem, this.account, this.configuration, err)
- );
- }
-
- private void on_reachable_changed() {
- if (this.remote.connectivity.is_reachable.is_certain()) {
- start_postie.begin();
- } else {
- stop_postie();
- }
- }
-
- private void on_connectivity_error(Error error) {
- stop_postie();
- notify_report_problem(ProblemType.CONNECTION_ERROR, error);
- }
-
}
diff --git a/test/engine/api/geary-account-mock.vala b/test/engine/api/geary-account-mock.vala
index 8c978f5f..b9274e9e 100644
--- a/test/engine/api/geary-account-mock.vala
+++ b/test/engine/api/geary-account-mock.vala
@@ -49,6 +49,14 @@ public class Geary.MockAccount : Account, MockObject {
throw new EngineError.UNSUPPORTED("Mock method");
}
+ public override void became_reachable() {
+
+ }
+
+ public override void became_unreachable() {
+
+ }
+
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
