[geary/wip/713006-better-error-reporting] Use the ConnectivityManager to detect connection problems up front.
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/713006-better-error-reporting] Use the ConnectivityManager to detect connection problems up front.
- Date: Mon, 13 Nov 2017 13:47:08 +0000 (UTC)
commit 0d1efc2fe08406e0109e8bc68605731b19c3d1e4
Author: Michael James Gratton <mike vee net>
Date: Tue Nov 14 00:46:52 2017 +1100
Use the ConnectivityManager to detect connection problems up front.
* src/engine/util/util-connectivity-manager.vala (ConnectivityManager):
Work with a NetworkAddress rather than an Endpoint to simplify a
bit. Use trillian for is-reachable property, since that's more
realistic Add new is-valid property and address_error_reported signal,
update and fire these when the reachability check reports an error
rather than just not reachable. Don't treat transient DNS errors as an
error, however. Update call sites.
* src/engine/imap-db/outbox/smtp-outbox-folder.vala (SmtpOutboxFolder),
src/engine/imap/transport/imap-client-session-manager.vala
(ClientSessionManager): Look up to new address_error_reported signal
and clean up then report the problem when it is fired.
src/engine/api/geary-endpoint.vala | 2 +-
src/engine/imap-db/outbox/smtp-outbox-folder.vala | 16 +++-
.../transport/imap-client-session-manager.vala | 19 +++-
src/engine/util/util-connectivity-manager.vala | 93 +++++++++++++++-----
4 files changed, 97 insertions(+), 33 deletions(-)
---
diff --git a/src/engine/api/geary-endpoint.vala b/src/engine/api/geary-endpoint.vala
index 596754e..14121dc 100644
--- a/src/engine/api/geary-endpoint.vala
+++ b/src/engine/api/geary-endpoint.vala
@@ -127,7 +127,7 @@ public class Geary.Endpoint : BaseObject {
this.remote_address = new NetworkAddress(host_specifier, default_port);
this.flags = flags;
this.timeout_sec = timeout_sec;
- this.connectivity = new ConnectivityManager(this);
+ this.connectivity = new ConnectivityManager(this.remote_address);
}
private SocketClient get_socket_client() {
diff --git a/src/engine/imap-db/outbox/smtp-outbox-folder.vala
b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
index 569c06b..4211392 100644
--- a/src/engine/imap-db/outbox/smtp-outbox-folder.vala
+++ b/src/engine/imap-db/outbox/smtp-outbox-folder.vala
@@ -884,7 +884,8 @@ private class Geary.SmtpOutboxFolder :
private void on_account_opened() {
this.fill_outbox_queue.begin();
this.smtp_endpoint.connectivity.notify["is-reachable"].connect(on_reachable_changed);
- if (this.smtp_endpoint.connectivity.is_reachable) {
+ this.smtp_endpoint.connectivity.address_error_reported.connect(on_connectivity_error);
+ if (this.smtp_endpoint.connectivity.is_reachable.is_certain()) {
this.start_timer.start();
} else {
this.smtp_endpoint.connectivity.check_reachable.begin();
@@ -894,18 +895,23 @@ private class Geary.SmtpOutboxFolder :
private void on_account_closed() {
this.stop_postman();
this.smtp_endpoint.connectivity.notify["is-reachable"].disconnect(on_reachable_changed);
+ this.smtp_endpoint.connectivity.address_error_reported.disconnect(on_connectivity_error);
}
private void on_reachable_changed() {
- if (this.smtp_endpoint.connectivity.is_reachable) {
+ if (this.smtp_endpoint.connectivity.is_reachable.is_certain()) {
if (this.queue_cancellable == null) {
this.start_timer.start();
}
} else {
this.start_timer.reset();
- if (this.queue_cancellable != null) {
- stop_postman();
- }
+ stop_postman();
}
}
+
+ private void on_connectivity_error(Error error) {
+ stop_postman();
+ notify_report_problem(ProblemType.CONNECTION_ERROR, error);
+ }
+
}
diff --git a/src/engine/imap/transport/imap-client-session-manager.vala
b/src/engine/imap/transport/imap-client-session-manager.vala
index a02b6d5..302b914 100644
--- a/src/engine/imap/transport/imap-client-session-manager.vala
+++ b/src/engine/imap/transport/imap-client-session-manager.vala
@@ -123,7 +123,8 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
this.authentication_failed = false;
this.endpoint.connectivity.notify["is-reachable"].connect(on_connectivity_change);
- if (this.endpoint.connectivity.is_reachable) {
+ this.endpoint.connectivity.address_error_reported.connect(on_connectivity_error);
+ if (this.endpoint.connectivity.is_reachable.is_certain()) {
this.adjust_session_pool.begin();
} else {
this.endpoint.connectivity.check_reachable.begin();
@@ -141,6 +142,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
this.pool_stop.reset();
this.endpoint.connectivity.notify["is-reachable"].disconnect(on_connectivity_change);
+ this.endpoint.connectivity.address_error_reported.disconnect(on_connectivity_error);
// to avoid locking down the sessions table while scheduling disconnects, make a copy
// and work off of that
@@ -204,7 +206,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
&& this.is_open
&& !this.authentication_failed
&& !this.untrusted_host
- && this.endpoint.connectivity.is_reachable) {
+ && this.endpoint.connectivity.is_reachable.is_certain()) {
this.pending_sessions++;
create_new_authorized_session.begin(
null,
@@ -232,9 +234,9 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
if (untrusted_host)
throw new ImapError.UNAVAILABLE("Untrusted host %s", endpoint.to_string());
- if (!this.endpoint.connectivity.is_reachable)
+ if (!this.endpoint.connectivity.is_reachable.is_certain())
throw new ImapError.UNAVAILABLE("Host at %s is unreachable", endpoint.to_string());
-
+
ClientSession new_session = new ClientSession(endpoint);
// add session to pool before launching all the connect activity so error cases can properly
@@ -546,7 +548,7 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
}
private void on_connectivity_change() {
- bool is_reachable = this.endpoint.connectivity.is_reachable;
+ bool is_reachable = this.endpoint.connectivity.is_reachable.is_certain();
if (is_reachable) {
this.pool_start.start();
this.pool_stop.reset();
@@ -558,6 +560,13 @@ public class Geary.Imap.ClientSessionManager : BaseObject {
}
}
+ private void on_connectivity_error(Error error) {
+ this.is_ready = false;
+ this.pool_start.reset();
+ this.pool_stop.start();
+ connection_failed(error);
+ }
+
/**
* Use only for debugging and logging.
*/
diff --git a/src/engine/util/util-connectivity-manager.vala b/src/engine/util/util-connectivity-manager.vala
index 92f6767..226f1de 100644
--- a/src/engine/util/util-connectivity-manager.vala
+++ b/src/engine/util/util-connectivity-manager.vala
@@ -6,13 +6,13 @@
*/
/**
- * Keeps track of network connectivity changes for an endpoint.
+ * Keeps track of network connectivity changes for a network address.
*
* This class is a convenience API for the GIO NetworkMonitor. Since
* when connecting and disconnecting from a network, multiple
* network-changed signals may be sent, this class coalesces these as
* best as possible so the rest of the engine is only notified once
- * when an endpoint becomes reachable, and once when it becomes
+ * when an address becomes reachable, and once when it becomes
* unreachable.
*
* Note this class is not thread safe and should only be invoked from
@@ -20,11 +20,19 @@
*/
public class Geary.ConnectivityManager : BaseObject {
- /** Determines if the managed endpoint is currently reachable. */
- public bool is_reachable { get; private set; default = false; }
+ /** The address being monitored. */
+ public NetworkAddress address { get; private set; default = null; }
- // Weak to avoid a circular ref with the endpoint
- private weak Endpoint endpoint;
+ /** Determines if the managed address is currently reachable. */
+ public Trillian is_reachable { get; private set; default = Geary.Trillian.UNKNOWN; }
+
+ /**
+ * Determines if a the address's network address name is valid.
+ *
+ * This will become certain if the address becomes reachable, and
+ * will become impossible if a fatal address error is reported.
+ */
+ public Trillian is_valid { get; private set; default = Geary.Trillian.UNKNOWN; }
private NetworkMonitor monitor;
@@ -32,10 +40,21 @@ public class Geary.ConnectivityManager : BaseObject {
/**
- * Constructs a new manager for a specific endpoint.
+ * Fired when a fatal error was reported checking the address.
+ *
+ * This is typically caused by an an authoritative DNS name not
+ * found error, but may be anything else that indicates that the
+ * address will be unusable as-is without some kind of user or
+ * server administrator intervention.
+ */
+ public signal void address_error_reported(Error error);
+
+
+ /**
+ * Constructs a new manager for a specific address.
*/
- public ConnectivityManager(Endpoint endpoint) {
- this.endpoint = endpoint;
+ public ConnectivityManager(NetworkAddress address) {
+ this.address = address;
this.monitor = NetworkMonitor.get_default();
this.monitor.network_changed.connect(on_network_changed);
@@ -46,7 +65,7 @@ public class Geary.ConnectivityManager : BaseObject {
}
/**
- * Starts checking if the manager's endpoint is reachable.
+ * Starts checking if the manager's address is reachable.
*
* This will cancel any existing check, and start a new one
* running, updating the `is_reachable` property on completion.
@@ -63,14 +82,11 @@ public class Geary.ConnectivityManager : BaseObject {
Cancellable cancellable = new Cancellable();
this.existing_check = cancellable;
- string endpoint = this.endpoint.to_string();
- bool is_reachable = this.is_reachable;
+ string endpoint = to_address_string();
+ bool is_reachable = false;
try {
debug("Checking if %s reachable...", endpoint);
- is_reachable = yield this.monitor.can_reach_async(
- this.endpoint.remote_address,
- cancellable
- );
+ is_reachable = yield this.monitor.can_reach_async(this.address, cancellable);
} catch (Error err) {
if (err is IOError.NETWORK_UNREACHABLE &&
this.monitor.network_available) {
@@ -81,11 +97,19 @@ public class Geary.ConnectivityManager : BaseObject {
is_reachable = true;
debug("Assuming %s is reachable, despite network unavailability",
endpoint);
+ } else if (err is ResolverError.TEMPORARY_FAILURE) {
+ // This often happens when networking is coming back
+ // online, may because the interface is up but has not
+ // been assigned an address yet? Since we should get
+ // another network change when the interface is
+ // configured, just ignore it.
+ debug("Ignoring: %s", err.message);
} else if (!(err is IOError.CANCELLED)) {
// Service is unreachable
debug("Error checking %s reachable, treating as unreachable: %s",
- endpoint, err.message);
- is_reachable = false;
+ endpoint, err.message);
+ set_invalid();
+ address_error_reported(err);
}
} finally {
if (!cancellable.is_cancelled()) {
@@ -123,13 +147,38 @@ public class Geary.ConnectivityManager : BaseObject {
private inline void set_reachable(bool reachable) {
// Coalesce changes to is_reachable, since Vala <= 0.34 always
// fires notify signals on set, even if the value doesn't
- // change. 0.36 fixes that, so pull this out when we can
+ // change. 0.36 fixes that, so pull this test out when we can
// depend on that as a minimum.
- if (this.is_reachable != reachable) {
+ if ((reachable && !this.is_reachable.is_certain()) ||
+ (!reachable && !this.is_reachable.is_impossible())) {
debug("Host %s became %s",
- this.endpoint.to_string(), reachable ? "reachable" : "unreachable");
- this.is_reachable = reachable;
+ this.address.to_string(), reachable ? "reachable" : "unreachable");
+ this.is_reachable = reachable ? Trillian.TRUE : Trillian.FALSE;
}
+
+ // We only work out if the name is valid (or becomes valid
+ // again) if the address becomes reachable.
+ if (reachable && this.is_valid.is_uncertain()) {
+ this.is_valid = Trillian.TRUE;
+ }
+
}
+ private inline void set_invalid() {
+ // Coalesce changes to is_reachable, since Vala <= 0.34 always
+ // fires notify signals on set, even if the value doesn't
+ // change. 0.36 fixes that, so pull this method out when we can
+ // depend on that as a minimum.
+ if (this.is_valid != Trillian.FALSE) {
+ this.is_valid = Trillian.FALSE;
+ }
+ }
+
+ private inline string to_address_string() {
+ // Unlikely to be the case, but if IPv6 format it nicely
+ return (this.address.hostname.index_of(":") == -1)
+ ? "%s:%u".printf(this.address.hostname, this.address.port)
+ : "[%s]:%u".printf(this.address.hostname, this.address.port);
+ }
+
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
