[geary/wip/17-noisy-problem-reports] Add Geary.ErrorContext class for passing rich error context info around
- From: Michael Gratton <mjog src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [geary/wip/17-noisy-problem-reports] Add Geary.ErrorContext class for passing rich error context info around
- Date: Mon, 7 Jan 2019 12:16:51 +0000 (UTC)
commit 92353bf0c92498af357871bb91e30863d61c1120
Author: Michael Gratton <mike vee net>
Date: Mon Jan 7 22:04:03 2019 +1100
Add Geary.ErrorContext class for passing rich error context info around
Require ClientService implementations and ProblemReport use contexts
rather than bare errors. Move code for generating backtraces and and
formatting errors from ProblemReport to ErrorContext.
po/POTFILES.in | 1 +
src/client/components/main-window-info-bar.vala | 13 +--
src/engine/api/geary-client-service.vala | 21 +++--
src/engine/api/geary-problem-report.vala | 94 ++------------------
src/engine/imap/api/imap-client-service.vala | 4 +-
src/engine/meson.build | 1 +
src/engine/smtp/smtp-client-service.vala | 6 +-
src/engine/util/util-error-context.vala | 111 ++++++++++++++++++++++++
8 files changed, 149 insertions(+), 102 deletions(-)
---
diff --git a/po/POTFILES.in b/po/POTFILES.in
index d99c8727..8e1e600a 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -380,6 +380,7 @@ src/engine/util/util-ascii.vala
src/engine/util/util-collection.vala
src/engine/util/util-config-file.vala
src/engine/util/util-connectivity-manager.vala
+src/engine/util/util-error-context.vala
src/engine/util/util-files.vala
src/engine/util/util-generic-capabilities.vala
src/engine/util/util-html.vala
diff --git a/src/client/components/main-window-info-bar.vala b/src/client/components/main-window-info-bar.vala
index 17902e36..7c489064 100644
--- a/src/client/components/main-window-info-bar.vala
+++ b/src/client/components/main-window-info-bar.vala
@@ -222,12 +222,15 @@ public class MainWindowInfoBar : Gtk.InfoBar {
if (this.report.error == null) {
details.append("No error reported");
} else {
- details.append_printf("Error type: %s\n", this.report.format_error_type());
- details.append_printf("Message: %s\n", this.report.error.message);
- }
- if (this.report.backtrace != null) {
+ details.append_printf(
+ "Error type: %s\n", this.report.error.format_error_type()
+ );
+ details.append_printf(
+ "Message: %s\n", this.report.error.thrown.message
+ );
details.append("Back trace:\n");
- foreach (Geary.ProblemReport.StackFrame frame in this.report.backtrace) {
+ foreach (Geary.ErrorContext.StackFrame frame in
+ this.report.error.backtrace) {
details.append_printf(" - %s\n", frame.to_string());
}
}
diff --git a/src/engine/api/geary-client-service.vala b/src/engine/api/geary-client-service.vala
index e0e76941..a2d92417 100644
--- a/src/engine/api/geary-client-service.vala
+++ b/src/engine/api/geary-client-service.vala
@@ -151,14 +151,14 @@ public abstract class Geary.ClientService : BaseObject {
*
* @see Status.CONNECTION_FAILED
*/
- public signal void connection_error(GLib.Error? err);
+ public signal void connection_error(ErrorContext err);
/**
* Fired when the service encounters an unrecoverable error.
*
* @see Status.UNRECOVERABLE_ERROR
*/
- public signal void unrecoverable_error(GLib.Error? err);
+ public signal void unrecoverable_error(ErrorContext err);
/** The service's account. */
@@ -194,6 +194,9 @@ public abstract class Geary.ClientService : BaseObject {
private TimeoutManager became_reachable_timer;
private TimeoutManager became_unreachable_timer;
+ /** The last reported error, if any. */
+ public ErrorContext? last_error { get; private set; default = null; }
+
protected ClientService(AccountInformation account,
ServiceInformation configuration,
@@ -344,9 +347,12 @@ public abstract class Geary.ClientService : BaseObject {
* network service encountered some network error other than a
* login failure or TLS certificate validation error.
*/
- protected void notify_connection_failed(GLib.Error? err) {
+ protected void notify_connection_failed(ErrorContext? error) {
+ // Set the error first so it is up to date when any
+ // current-status notify handlers fire
+ this.last_error = error;
this.current_status = CONNECTION_FAILED;
- connection_error(err);
+ connection_error(error);
}
/**
@@ -370,9 +376,12 @@ public abstract class Geary.ClientService : BaseObject {
* 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) {
+ protected void notify_unrecoverable_error(ErrorContext error) {
+ // Set the error first so it is up to date when any
+ // current-status notify handlers fire
+ this.last_error = error;
this.current_status = UNRECOVERABLE_ERROR;
- unrecoverable_error(err);
+ unrecoverable_error(error);
}
private void connect_handlers() {
diff --git a/src/engine/api/geary-problem-report.vala b/src/engine/api/geary-problem-report.vala
index 2e550560..41272929 100644
--- a/src/engine/api/geary-problem-report.vala
+++ b/src/engine/api/geary-problem-report.vala
@@ -57,59 +57,17 @@ public enum Geary.ProblemType {
public class Geary.ProblemReport : Object {
- /**
- * Represents an individual stack frame in a call back-trace.
- */
- public class StackFrame {
-
-
- /** Name of the function being called. */
- public string name = "unknown";
-
-
- internal StackFrame(Unwind.Cursor frame) {
- uint8 proc_name[256];
- int ret = -frame.get_proc_name(proc_name);
- if (ret == Unwind.Error.SUCCESS ||
- ret == Unwind.Error.NOMEM) {
- this.name = (string) proc_name;
- }
- }
-
- public string to_string() {
- return this.name;
- }
-
- }
-
-
/** Describes the type of being reported. */
public ProblemType problem_type { get; private set; }
/** The exception caused the problem, if any. */
- public Error? error { get; private set; default = null; }
-
- /** A back trace from when the problem report was constructed. */
- public Gee.List<StackFrame>? backtrace = null;
+ public ErrorContext? error { get; private set; default = null; }
public ProblemReport(ProblemType type, Error? error) {
this.problem_type = type;
- this.error = error;
-
if (error != null) {
- // Some kind of exception occurred, so build a trace. This
- // is far from perfect, but at least we will know where it
- // was getting caught.
- this.backtrace = new Gee.LinkedList<StackFrame>();
- Unwind.Context trace = Unwind.Context();
- Unwind.Cursor cursor = Unwind.Cursor.local(trace);
-
- // This misses the first frame, but that's this
- // constructor call, so we don't really care.
- while (cursor.step() != 0) {
- this.backtrace.add(new StackFrame(cursor));
- }
+ this.error = new ErrorContext(error);
}
}
@@ -117,50 +75,12 @@ public class Geary.ProblemReport : Object {
public string to_string() {
return "%s: %s".printf(
this.problem_type.to_string(),
- format_full_error() ?? "no error reported"
+ this.error != null
+ ? this.error.format_full_error()
+ : "no error reported"
);
}
- /** Returns a string representation of the error type, for debugging. */
- public string? format_error_type() {
- string type = null;
- if (this.error != null) {
- const string QUARK_SUFFIX = "-quark";
- string ugly_domain = this.error.domain.to_string();
- if (ugly_domain.has_suffix(QUARK_SUFFIX)) {
- ugly_domain = ugly_domain.substring(
- 0, ugly_domain.length - QUARK_SUFFIX.length
- );
- }
- StringBuilder nice_domain = new StringBuilder();
- string separator = (ugly_domain.index_of("_") != -1) ? "_" : "-";
- foreach (string part in ugly_domain.split(separator)) {
- if (part.length > 0) {
- if (part == "io") {
- nice_domain.append("IO");
- } else {
- nice_domain.append(part.up(1));
- nice_domain.append(part.substring(1));
- }
- }
- }
-
- type = "%s %i".printf(nice_domain.str, this.error.code);
- }
- return type;
- }
-
- /** Returns a string representation of the complete error, for debugging. */
- public string? format_full_error() {
- string error = null;
- if (this.error != null) {
- error = String.is_empty(this.error.message)
- ? "%s: no message specified".printf(format_error_type())
- : "%s: \"%s\"".printf(format_error_type(), this.error.message);
- }
- return error;
- }
-
}
/**
@@ -209,7 +129,9 @@ public class Geary.ServiceProblemReport : AccountProblemReport {
this.account.id,
this.service.protocol.to_string(),
this.problem_type.to_string(),
- format_full_error() ?? "no error reported"
+ this.error != null
+ ? this.error.format_full_error()
+ : "no error reported"
);
}
diff --git a/src/engine/imap/api/imap-client-service.vala b/src/engine/imap/api/imap-client-service.vala
index 5b2c315d..ea18782b 100644
--- a/src/engine/imap/api/imap-client-service.vala
+++ b/src/engine/imap/api/imap-client-service.vala
@@ -296,7 +296,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
if (!(err is IOError.CANCELLED)) {
debug("[%s] Error adding new session to the pool: %s",
this.account.id, err.message);
- notify_connection_failed(err);
+ notify_connection_failed(new ErrorContext(err));
}
this.close_pool.begin();
}
@@ -370,7 +370,7 @@ internal class Geary.Imap.ClientService : Geary.ClientService {
yield new_session.connect_async(cancellable);
} catch (GLib.Error err) {
if (!(err is IOError.CANCELLED)) {
- notify_connection_failed(err);
+ notify_connection_failed(new ErrorContext(err));
}
throw err;
}
diff --git a/src/engine/meson.build b/src/engine/meson.build
index 2fd212b8..796e18d1 100644
--- a/src/engine/meson.build
+++ b/src/engine/meson.build
@@ -303,6 +303,7 @@ geary_engine_vala_sources = files(
'util/util-collection.vala',
'util/util-config-file.vala',
'util/util-connectivity-manager.vala',
+ 'util/util-error-context.vala',
'util/util-files.vala',
'util/util-generic-capabilities.vala',
'util/util-html.vala',
diff --git a/src/engine/smtp/smtp-client-service.vala b/src/engine/smtp/smtp-client-service.vala
index 58b93634..bbead2ad 100644
--- a/src/engine/smtp/smtp-client-service.vala
+++ b/src/engine/smtp/smtp-client-service.vala
@@ -128,17 +128,17 @@ internal class Geary.Smtp.ClientService : Geary.ClientService {
notify_authentication_failed();
} else if (err is SmtpError.STARTTLS_FAILED ||
err is SmtpError.NOT_CONNECTED) {
- notify_connection_failed(err);
+ notify_connection_failed(new ErrorContext(err));
} else if (err is SmtpError.PARSE_ERROR ||
err is SmtpError.SERVER_ERROR ||
err is SmtpError.NOT_SUPPORTED) {
- notify_unrecoverable_error(err);
+ notify_unrecoverable_error(new ErrorContext(err));
}
cancellable.cancel();
} catch (GLib.IOError.CANCELLED err) {
// Nothing to do here — we're already cancelled.
} catch (GLib.Error err) {
- notify_connection_failed(err);
+ notify_connection_failed(new ErrorContext(err));
cancellable.cancel();
}
diff --git a/src/engine/util/util-error-context.vala b/src/engine/util/util-error-context.vala
new file mode 100644
index 00000000..81cfcb8b
--- /dev/null
+++ b/src/engine/util/util-error-context.vala
@@ -0,0 +1,111 @@
+/*
+ * Copyright 2019 Michael Gratton <mike vee net>
+ *
+ * This software is licensed under the GNU Lesser General Public License
+ * (version 2.1 or later). See the COPYING file in this distribution.
+ */
+
+/**
+ * Provides additional context information for an error when thrown.
+ *
+ * This class allows the engine to provide additional context
+ * information such as stack traces, the cause of this error, or
+ * engine state when an error is thrown. A stack trace will be
+ * generated for the context when instantiated, so context instances
+ * should be constructed as close to when the error was thrown as
+ * possible.
+ *
+ * This works around the GLib error system's lack of extensibility.
+ */
+public class Geary.ErrorContext : BaseObject {
+
+
+ /** Represents an individual stack frame in a call back-trace. */
+ public class StackFrame {
+
+
+ /** Name of the function being called. */
+ public string name = "unknown";
+
+
+ internal StackFrame(Unwind.Cursor frame) {
+ uint8 proc_name[256];
+ int ret = -frame.get_proc_name(proc_name);
+ if (ret == Unwind.Error.SUCCESS ||
+ ret == Unwind.Error.NOMEM) {
+ this.name = (string) proc_name;
+ }
+ }
+
+ public string to_string() {
+ return this.name;
+ }
+
+ }
+
+
+ /** The error thrown that this context is describing. */
+ public GLib.Error thrown { get; private set; }
+
+ /** A back trace from when the context was constructed. */
+ public Gee.List<StackFrame>? backtrace {
+ get; private set; default = new Gee.LinkedList<StackFrame>();
+ }
+
+
+ public ErrorContext(GLib.Error thrown) {
+ this.thrown = thrown;
+
+ Unwind.Context trace = Unwind.Context();
+ Unwind.Cursor cursor = Unwind.Cursor.local(trace);
+
+ // This misses the first frame, but that's this
+ // constructor call, so we don't really care.
+ while (cursor.step() != 0) {
+ this.backtrace.add(new StackFrame(cursor));
+ }
+ }
+
+ /** Returns a string representation of the error type, for debugging. */
+ public string? format_error_type() {
+ string type = null;
+ if (this.thrown != null) {
+ const string QUARK_SUFFIX = "-quark";
+ string ugly_domain = this.thrown.domain.to_string();
+ if (ugly_domain.has_suffix(QUARK_SUFFIX)) {
+ ugly_domain = ugly_domain.substring(
+ 0, ugly_domain.length - QUARK_SUFFIX.length
+ );
+ }
+ StringBuilder nice_domain = new StringBuilder();
+ string separator = (ugly_domain.index_of("_") != -1) ? "_" : "-";
+ foreach (string part in ugly_domain.split(separator)) {
+ if (part.length > 0) {
+ if (part == "io") {
+ nice_domain.append("IO");
+ } else {
+ nice_domain.append(part.up(1));
+ nice_domain.append(part.substring(1));
+ }
+ }
+ }
+
+ type = "%s %i".printf(nice_domain.str, this.thrown.code);
+ }
+ return type;
+ }
+
+ /** Returns a string representation of the complete error, for debugging. */
+ public string? format_full_error() {
+ string error = null;
+ if (this.thrown != null) {
+ error = String.is_empty(this.thrown.message)
+ ? "%s: no message specified".printf(format_error_type())
+ : "%s: \"%s\"".printf(
+ format_error_type(), this.thrown.message
+ );
+ }
+ return error;
+ }
+
+}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
