Merge pull request #806 from mwhudson/add-subiquity.common

add subiqity.common package and more error report handling to it

subiquity/common/__init__.py

@@ -0,0 +1,14 @@
+# Copyright 2020 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.

subiquity/common/errorreport.py

@@ -0,0 +1,412 @@
+# Copyright 2020 Canonical, Ltd.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as
+# published by the Free Software Foundation, either version 3 of the
+# License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import asyncio
+import enum
+import fcntl
+import json
+import logging
+import os
+import sys
+import time
+import traceback
+
+import apport
+import apport.crashdb
+import apport.hookutils
+
+import attr
+
+import bson
+
+import requests
+
+import urwid
+
+from subiquitycore.async_helpers import (
+    run_in_thread,
+    schedule_task,
+    )
+
+log = logging.getLogger('subiquitycore.common.errorreport')
+
+
+class ErrorReportState(enum.Enum):
+    INCOMPLETE = enum.auto()
+    LOADING = enum.auto()
+    DONE = enum.auto()
+    ERROR_GENERATING = enum.auto()
+    ERROR_LOADING = enum.auto()
+
+
+class ErrorReportKind(enum.Enum):
+    BLOCK_PROBE_FAIL = _("Block device probe failure")
+    DISK_PROBE_FAIL = _("Disk probe failure")
+    INSTALL_FAIL = _("Install failure")
+    UI = _("Installer crash")
+    NETWORK_FAIL = _("Network error")
+    UNKNOWN = _("Unknown error")
+
+
+@attr.s(cmp=False)
+class Upload(metaclass=urwid.MetaSignals):
+    signals = ['progress']
+
+    bytes_to_send = attr.ib()
+    bytes_sent = attr.ib(default=0)
+    pipe_r = attr.ib(default=None)
+    pipe_w = attr.ib(default=None)
+    cancelled = attr.ib(default=False)
+
+    def start(self):
+        self.pipe_r, self.pipe_w = os.pipe()
+        fcntl.fcntl(self.pipe_r, fcntl.F_SETFL, os.O_NONBLOCK)
+        asyncio.get_event_loop().add_reader(self.pipe_r, self._progress)
+
+    def _progress(self):
+        os.read(self.pipe_w, 4096)
+        urwid.emit_signal(self, 'progress')
+
+    def _bg_update(self, sent, to_send=None):
+        self.bytes_sent = sent
+        if to_send is not None:
+            self.bytes_to_send = to_send
+        os.write(self.pipe_w, b'x')
+
+    def stop(self):
+        asyncio.get_event_loop().remove_reader(self.pipe_r)
+        os.close(self.pipe_w)
+        os.close(self.pipe_r)
+
+
+@attr.s(cmp=False)
+class ErrorReport(metaclass=urwid.MetaSignals):
+
+    signals = ["changed"]
+
+    reporter = attr.ib()
+    base = attr.ib()
+    pr = attr.ib()
+    state = attr.ib()
+    _file = attr.ib()
+    _context = attr.ib()
+
+    meta = attr.ib(default=attr.Factory(dict))
+    uploader = attr.ib(default=None)
+
+    @classmethod
+    def new(cls, reporter, kind):
+        base = "{:.9f}.{}".format(time.time(), kind.name.lower())
+        crash_file = open(
+            os.path.join(reporter.crash_directory, base + ".crash"),
+            'wb')
+
+        pr = apport.Report('Bug')
+        pr['CrashDB'] = repr(reporter.crashdb_spec)
+
+        r = cls(
+            reporter=reporter, base=base, pr=pr, file=crash_file,
+            state=ErrorReportState.INCOMPLETE,
+            context=reporter.context.child(base))
+        r.set_meta("kind", kind.name)
+        return r
+
+    @classmethod
+    def from_file(cls, reporter, fpath):
+        base = os.path.splitext(os.path.basename(fpath))[0]
+        report = cls(
+            reporter, base, pr=apport.Report(date='???'),
+            state=ErrorReportState.LOADING, file=open(fpath, 'rb'),
+            context=reporter.context.child(base))
+        try:
+            fp = open(report.meta_path, 'r')
+        except FileNotFoundError:
+            pass
+        else:
+            with fp:
+                report.meta = json.load(fp)
+        return report
+
+    def add_info(self, _bg_attach_hook, wait=False):
+        def _bg_add_info():
+            _bg_attach_hook()
+            # Add basic info to report.
+            self.pr.add_proc_info()
+            self.pr.add_os_info()
+            self.pr.add_hooks_info(None)
+            apport.hookutils.attach_hardware(self.pr)
+            # Because apport-cli will in general be run on a different
+            # machine, we make some slightly obscure alterations to the report
+            # to make this go better.
+
+            # apport-cli gets upset if neither of these are present.
+            self.pr['Package'] = 'subiquity ' + os.environ.get(
+                "SNAP_REVISION", "SNAP_REVISION")
+            self.pr['SourcePackage'] = 'subiquity'
+
+            # If ExecutableTimestamp is present, apport-cli will try to check
+            # that ExecutablePath hasn't changed. But it won't be there.
+            del self.pr['ExecutableTimestamp']
+            # apport-cli gets upset at the probert C extensions it sees in
+            # here.  /proc/maps is very unlikely to be interesting for us
+            # anyway.
+            del self.pr['ProcMaps']
+            self.pr.write(self._file)
+
+        async def add_info():
+            with self._context.child("add_info") as context:
+                try:
+                    await run_in_thread(_bg_add_info)
+                except Exception:
+                    self.state = ErrorReportState.ERROR_GENERATING
+                    log.exception("adding info to problem report failed")
+                else:
+                    context.description = "written to " + self.path
+                    self.state = ErrorReportState.DONE
+                self._file.close()
+                self._file = None
+                urwid.emit_signal(self, "changed")
+        if wait:
+            with self._context.child("add_info") as context:
+                _bg_add_info()
+                context.description = "written to " + self.path
+        else:
+            schedule_task(add_info())
+
+    async def load(self):
+        with self._context.child("load"):
+            # Load report from disk in background.
+            try:
+                await run_in_thread(self.pr.load, self._file)
+            except Exception:
+                log.exception("loading problem report failed")
+                self.state = ErrorReportState.ERROR_LOADING
+            else:
+                self.state = ErrorReportState.DONE
+        self._file.close()
+        self._file = None
+        urwid.emit_signal(self, "changed")
+
+    def upload(self):
+        uploader = self.uploader = Upload(bytes_to_send=1)
+
+        url = "https://daisy.ubuntu.com"
+        if self.reporter.dry_run:
+            url = "https://daisy.staging.ubuntu.com"
+
+        chunk_size = 1024
+
+        def chunk(data):
+            for i in range(0, len(data), chunk_size):
+                if uploader.cancelled:
+                    log.debug("upload for %s cancelled", self.base)
+                    return
+                yield data[i:i+chunk_size]
+                uploader._bg_update(uploader.bytes_sent + chunk_size)
+
+        def _bg_upload():
+            for_upload = {
+                "Kind": self.kind.value
+                }
+            for k, v in self.pr.items():
+                if len(v) < 1024 or k in {
+                        "InstallerLogInfo",
+                        "Traceback",
+                        "ProcCpuinfoMinimal",
+                        }:
+                    for_upload[k] = v
+                else:
+                    log.debug("dropping %s of length %s", k, len(v))
+            if "CurtinLog" in self.pr:
+                logtail = []
+                for line in self.pr["CurtinLog"].splitlines():
+                    logtail.append(line.strip())
+                    while sum(map(len, logtail)) > 2048:
+                        logtail.pop(0)
+                for_upload["CurtinLogTail"] = "\n".join(logtail)
+            data = bson.BSON().encode(for_upload)
+            self.uploader._bg_update(0, len(data))
+            headers = {
+                'user-agent': 'subiquity/{}'.format(
+                    os.environ.get("SNAP_VERSION", "SNAP_VERSION")),
+                }
+            response = requests.post(url, data=chunk(data), headers=headers)
+            response.raise_for_status()
+            return response.text.split()[0]
+
+        async def upload():
+            with self._context.child("upload") as context:
+                try:
+                    oops_id = await run_in_thread(_bg_upload)
+                except requests.exceptions.RequestException:
+                    log.exception("upload for %s failed", self.base)
+                else:
+                    self.set_meta("oops-id", oops_id)
+                    context.description = oops_id
+                uploader.stop()
+                self.uploader = None
+                urwid.emit_signal(self, 'changed')
+
+        urwid.emit_signal(self, 'changed')
+        uploader.start()
+
+        schedule_task(upload())
+
+    def _path_with_ext(self, ext):
+        return os.path.join(
+            self.reporter.crash_directory, self.base + '.' + ext)
+
+    @property
+    def meta_path(self):
+        return self._path_with_ext('meta')
+
+    @property
+    def path(self):
+        return self._path_with_ext('crash')
+
+    def set_meta(self, key, value):
+        self.meta[key] = value
+        with open(self.meta_path, 'w') as fp:
+            json.dump(self.meta, fp, indent=4)
+
+    def mark_seen(self):
+        self.set_meta("seen", True)
+        urwid.emit_signal(self, "changed")
+
+    @property
+    def kind(self):
+        k = self.meta.get("kind", "UNKNOWN")
+        return getattr(ErrorReportKind, k, ErrorReportKind.UNKNOWN)
+
+    @property
+    def seen(self):
+        return self.meta.get("seen", False)
+
+    @property
+    def oops_id(self):
+        return self.meta.get("oops-id")
+
+    @property
+    def persistent_details(self):
+        """Return fs-label, path-on-fs to report."""
+        # Not sure if this is more or less sane than shelling out to
+        # findmnt(1).
+        looking_for = os.path.abspath(
+            os.path.normpath(self.reporter.crash_directory))
+        for line in open('/proc/self/mountinfo').readlines():
+            parts = line.strip().split()
+            if os.path.normpath(parts[4]) == looking_for:
+                devname = parts[9]
+                root = parts[3]
+                break
+        else:
+            if self.reporter.dry_run:
+                path = ('install-logs/2019-11-06.0/crash/' +
+                        self.base +
+                        '.crash')
+                return "casper-rw", path
+            return None, None
+        import pyudev
+        c = pyudev.Context()
+        devs = list(c.list_devices(
+            subsystem='block', DEVNAME=os.path.realpath(devname)))
+        if not devs:
+            return None, None
+        label = devs[0].get('ID_FS_LABEL_ENC', '')
+        return label, root[1:] + '/' + self.base + '.crash'
+
+
+class ErrorReporter(object):
+
+    def __init__(self, context, dry_run, root):
+        self.context = context
+        self.dry_run = dry_run
+        self.reports = []
+        if dry_run:
+            self.crash_directory = os.path.join(root, 'var/crash')
+        self.crashdb_spec = {
+            'impl': 'launchpad',
+            'project': 'subiquity',
+            }
+        if dry_run:
+            self.crashdb_spec['launchpad_instance'] = 'staging'
+        self._apport_data = []
+        self._apport_files = []
+
+    def start_loading_reports(self):
+        os.makedirs(self.crash_directory, exist_ok=True)
+        filenames = os.listdir(self.crash_directory)
+        to_load = []
+        for filename in sorted(filenames, reverse=True):
+            base, ext = os.path.splitext(filename)
+            if ext != ".crash":
+                continue
+            path = os.path.join(self.crash_directory, filename)
+            r = ErrorReport.from_file(self, path)
+            self.reports.append(r)
+            to_load.append(r)
+        schedule_task(self._load_reports(to_load))
+
+    async def _load_reports(self, to_load):
+        for report in to_load:
+            await report.load()
+
+    def note_file_for_apport(self, key, path):
+        self._apport_files.append((key, path))
+
+    def note_data_for_apport(self, key, value):
+        self._apport_data.append((key, value))
+
+    def make_apport_report(self, kind, thing, *, wait=False, **kw):
+        if not self.dry_run and not os.path.exists('/cdrom/.disk/info'):
+            return None
+
+        log.debug("generating crash report")
+
+        try:
+            report = ErrorReport.new(self, kind)
+            self.reports.insert(0, report)
+        except Exception:
+            log.exception("creating crash report failed")
+            return
+
+        etype = sys.exc_info()[0]
+        if etype is not None:
+            report.pr["Title"] = "{} crashed with {}".format(
+                thing, etype.__name__)
+            report.pr['Traceback'] = traceback.format_exc()
+        else:
+            report.pr["Title"] = thing
+
+        log.info(
+            "saving crash report %r to %s", report.pr["Title"], report.path)
+
+        apport_files = self._apport_files[:]
+        apport_data = self._apport_data.copy()
+
+        def _bg_attach_hook():
+            # Attach any stuff other parts of the code think we should know
+            # about.
+            for key, path in apport_files:
+                apport.hookutils.attach_file_if_exists(report.pr, path, key)
+            for key, value in apport_data:
+                report.pr[key] = value
+            for key, value in kw.items():
+                report.pr[key] = value
+
+        report.add_info(_bg_attach_hook, wait)
+
+        # In the fullness of time we should do the signature thing here.
+        return report

subiquity/controllers/error.py

@@ -13,28 +13,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import enum
-import json
 import logging
-import os
-import time
-
-import apport
-import apport.crashdb
-import apport.hookutils
-
-import bson
-
-import attr
-
-import requests
-
-import urwid
-
-from subiquitycore.async_helpers import (
-    run_in_thread,
-    schedule_task,
-    )
 
 from subiquity.controllers.cmdlist import CmdListController
 
@@ -42,328 +21,7 @@ from subiquity.controllers.cmdlist import CmdListController
 log = logging.getLogger('subiquity.controllers.error')
 
 
-class ErrorReportState(enum.Enum):
-    INCOMPLETE = enum.auto()
-    LOADING = enum.auto()
-    DONE = enum.auto()
-    ERROR_GENERATING = enum.auto()
-    ERROR_LOADING = enum.auto()
-
-
-class ErrorReportKind(enum.Enum):
-    BLOCK_PROBE_FAIL = _("Block device probe failure")
-    DISK_PROBE_FAIL = _("Disk probe failure")
-    INSTALL_FAIL = _("Install failure")
-    UI = _("Installer crash")
-    NETWORK_FAIL = _("Network error")
-    UNKNOWN = _("Unknown error")
-
-
-@attr.s(cmp=False)
-class Upload(metaclass=urwid.MetaSignals):
-    signals = ['progress']
-
-    controller = attr.ib()
-    bytes_to_send = attr.ib()
-    bytes_sent = attr.ib(default=0)
-    pipe_w = attr.ib(default=None)
-    cancelled = attr.ib(default=False)
-
-    def start(self):
-        self.pipe_w = self.controller.app.urwid_loop.watch_pipe(self._progress)
-
-    def _progress(self, x):
-        urwid.emit_signal(self, 'progress')
-
-    def _bg_update(self, sent, to_send=None):
-        self.bytes_sent = sent
-        if to_send is not None:
-            self.bytes_to_send = to_send
-        os.write(self.pipe_w, b'x')
-
-    def stop(self):
-        self.controller.app.urwid_loop.remove_watch_pipe(self.pipe_w)
-        os.close(self.pipe_w)
-
-
-@attr.s(cmp=False)
-class ErrorReport(metaclass=urwid.MetaSignals):
-
-    signals = ["changed"]
-
-    controller = attr.ib()
-    base = attr.ib()
-    pr = attr.ib()
-    state = attr.ib()
-    _file = attr.ib()
-    _context = attr.ib()
-
-    meta = attr.ib(default=attr.Factory(dict))
-    uploader = attr.ib(default=None)
-
-    @classmethod
-    def new(cls, controller, kind):
-        base = "{:.9f}.{}".format(time.time(), kind.name.lower())
-        crash_file = open(
-            os.path.join(controller.crash_directory, base + ".crash"),
-            'wb')
-
-        pr = apport.Report('Bug')
-        pr['CrashDB'] = repr(controller.crashdb_spec)
-
-        r = cls(
-            controller=controller, base=base, pr=pr, file=crash_file,
-            state=ErrorReportState.INCOMPLETE,
-            context=controller.context.child(base))
-        r.set_meta("kind", kind.name)
-        return r
-
-    @classmethod
-    def from_file(cls, controller, fpath):
-        base = os.path.splitext(os.path.basename(fpath))[0]
-        report = cls(
-            controller, base, pr=apport.Report(date='???'),
-            state=ErrorReportState.LOADING, file=open(fpath, 'rb'),
-            context=controller.context.child(base))
-        try:
-            fp = open(report.meta_path, 'r')
-        except FileNotFoundError:
-            pass
-        else:
-            with fp:
-                report.meta = json.load(fp)
-        return report
-
-    def add_info(self, _bg_attach_hook, wait=False):
-        def _bg_add_info():
-            _bg_attach_hook()
-            # Add basic info to report.
-            self.pr.add_proc_info()
-            self.pr.add_os_info()
-            self.pr.add_hooks_info(None)
-            apport.hookutils.attach_hardware(self.pr)
-            # Because apport-cli will in general be run on a different
-            # machine, we make some slightly obscure alterations to the report
-            # to make this go better.
-
-            # apport-cli gets upset if neither of these are present.
-            self.pr['Package'] = 'subiquity ' + os.environ.get(
-                "SNAP_REVISION", "SNAP_REVISION")
-            self.pr['SourcePackage'] = 'subiquity'
-
-            # If ExecutableTimestamp is present, apport-cli will try to check
-            # that ExecutablePath hasn't changed. But it won't be there.
-            del self.pr['ExecutableTimestamp']
-            # apport-cli gets upset at the probert C extensions it sees in
-            # here.  /proc/maps is very unlikely to be interesting for us
-            # anyway.
-            del self.pr['ProcMaps']
-            self.pr.write(self._file)
-
-        async def add_info():
-            with self._context.child("add_info") as context:
-                try:
-                    await run_in_thread(_bg_add_info)
-                except Exception:
-                    self.state = ErrorReportState.ERROR_GENERATING
-                    log.exception("adding info to problem report failed")
-                else:
-                    context.description = "written to " + self.path
-                    self.state = ErrorReportState.DONE
-                self._file.close()
-                self._file = None
-                urwid.emit_signal(self, "changed")
-        if wait:
-            with self._context.child("add_info") as context:
-                _bg_add_info()
-                context.description = "written to " + self.path
-        else:
-            schedule_task(add_info())
-
-    async def load(self):
-        with self._context.child("load"):
-            # Load report from disk in background.
-            try:
-                await run_in_thread(self.pr.load, self._file)
-            except Exception:
-                log.exception("loading problem report failed")
-                self.state = ErrorReportState.ERROR_LOADING
-            else:
-                self.state = ErrorReportState.DONE
-        self._file.close()
-        self._file = None
-        urwid.emit_signal(self, "changed")
-
-    def upload(self):
-        uploader = self.uploader = Upload(
-            controller=self.controller, bytes_to_send=1)
-
-        url = "https://daisy.ubuntu.com"
-        if self.controller.opts.dry_run:
-            url = "https://daisy.staging.ubuntu.com"
-
-        chunk_size = 1024
-
-        def chunk(data):
-            for i in range(0, len(data), chunk_size):
-                if uploader.cancelled:
-                    log.debug("upload for %s cancelled", self.base)
-                    return
-                yield data[i:i+chunk_size]
-                uploader._bg_update(uploader.bytes_sent + chunk_size)
-
-        def _bg_upload():
-            for_upload = {
-                "Kind": self.kind.value
-                }
-            for k, v in self.pr.items():
-                if len(v) < 1024 or k in {
-                        "InstallerLogInfo",
-                        "Traceback",
-                        "ProcCpuinfoMinimal",
-                        }:
-                    for_upload[k] = v
-                else:
-                    log.debug("dropping %s of length %s", k, len(v))
-            if "CurtinLog" in self.pr:
-                logtail = []
-                for line in self.pr["CurtinLog"].splitlines():
-                    logtail.append(line.strip())
-                    while sum(map(len, logtail)) > 2048:
-                        logtail.pop(0)
-                for_upload["CurtinLogTail"] = "\n".join(logtail)
-            data = bson.BSON().encode(for_upload)
-            self.uploader._bg_update(0, len(data))
-            headers = {
-                'user-agent': 'subiquity/{}'.format(
-                    os.environ.get("SNAP_VERSION", "SNAP_VERSION")),
-                }
-            response = requests.post(url, data=chunk(data), headers=headers)
-            response.raise_for_status()
-            return response.text.split()[0]
-
-        async def upload():
-            with self._context.child("upload") as context:
-                try:
-                    oops_id = await run_in_thread(_bg_upload)
-                except requests.exceptions.RequestException:
-                    log.exception("upload for %s failed", self.base)
-                else:
-                    self.set_meta("oops-id", oops_id)
-                    context.description = oops_id
-                uploader.stop()
-                self.uploader = None
-                urwid.emit_signal(self, 'changed')
-
-        urwid.emit_signal(self, 'changed')
-        uploader.start()
-
-        schedule_task(upload())
-
-    def _path_with_ext(self, ext):
-        return os.path.join(
-            self.controller.crash_directory, self.base + '.' + ext)
-
-    @property
-    def meta_path(self):
-        return self._path_with_ext('meta')
-
-    @property
-    def path(self):
-        return self._path_with_ext('crash')
-
-    def set_meta(self, key, value):
-        self.meta[key] = value
-        with open(self.meta_path, 'w') as fp:
-            json.dump(self.meta, fp, indent=4)
-
-    def mark_seen(self):
-        self.set_meta("seen", True)
-        urwid.emit_signal(self, "changed")
-
-    @property
-    def kind(self):
-        k = self.meta.get("kind", "UNKNOWN")
-        return getattr(ErrorReportKind, k, ErrorReportKind.UNKNOWN)
-
-    @property
-    def seen(self):
-        return self.meta.get("seen", False)
-
-    @property
-    def oops_id(self):
-        return self.meta.get("oops-id")
-
-    @property
-    def persistent_details(self):
-        """Return fs-label, path-on-fs to report."""
-        # Not sure if this is more or less sane than shelling out to
-        # findmnt(1).
-        looking_for = os.path.abspath(
-            os.path.normpath(self.controller.crash_directory))
-        for line in open('/proc/self/mountinfo').readlines():
-            parts = line.strip().split()
-            if os.path.normpath(parts[4]) == looking_for:
-                devname = parts[9]
-                root = parts[3]
-                break
-        else:
-            if self.controller.opts.dry_run:
-                path = ('install-logs/2019-11-06.0/crash/' +
-                        self.base +
-                        '.crash')
-                return "casper-rw", path
-            return None, None
-        import pyudev
-        c = pyudev.Context()
-        devs = list(c.list_devices(
-            subsystem='block', DEVNAME=os.path.realpath(devname)))
-        if not devs:
-            return None, None
-        label = devs[0].get('ID_FS_LABEL_ENC', '')
-        return label, root[1:] + '/' + self.base + '.crash'
-
-
 class ErrorController(CmdListController):
 
     autoinstall_key = 'error-commands'
     cmd_check = False
-
-    def __init__(self, app):
-        super().__init__(app)
-        self.crash_directory = os.path.join(self.app.root, 'var/crash')
-        self.crashdb_spec = {
-            'impl': 'launchpad',
-            'project': 'subiquity',
-            }
-        if self.app.opts.dry_run:
-            self.crashdb_spec['launchpad_instance'] = 'staging'
-        self.reports = []
-
-    def start(self):
-        os.makedirs(self.crash_directory, exist_ok=True)
-        # scan for pre-existing crash reports and start loading them
-        # in the background
-        self.scan_crash_dir()
-
-    async def _load_reports(self, to_load):
-        for report in to_load:
-            await report.load()
-
-    def scan_crash_dir(self):
-        filenames = os.listdir(self.crash_directory)
-        to_load = []
-        for filename in sorted(filenames, reverse=True):
-            base, ext = os.path.splitext(filename)
-            if ext != ".crash":
-                continue
-            path = os.path.join(self.crash_directory, filename)
-            r = ErrorReport.from_file(self, path)
-            self.reports.append(r)
-            to_load.append(r)
-        schedule_task(self._load_reports(to_load))
-
-    def create_report(self, kind):
-        r = ErrorReport.new(self, kind)
-        self.reports.insert(0, r)
-        return r

subiquity/controllers/filesystem.py

@@ -33,8 +33,8 @@ from subiquitycore.utils import (
     )
 
 
+from subiquity.common.errorreport import ErrorReportKind
 from subiquity.controller import SubiquityController
-from subiquity.controllers.error import ErrorReportKind
 from subiquity.models.filesystem import (
     align_up,
     Bootloader,

subiquity/controllers/installprogress.py

@@ -43,8 +43,8 @@ from subiquitycore.utils import (
     astart_command,
     )
 
+from subiquity.common.errorreport import ErrorReportKind
 from subiquity.controller import SubiquityController
-from subiquity.controllers.error import ErrorReportKind
 from subiquity.journald import journald_listener
 from subiquity.ui.views.installprogress import ProgressView
 

subiquity/controllers/network.py

@@ -20,8 +20,8 @@ from subiquitycore.async_helpers import schedule_task
 from subiquitycore.context import with_context
 from subiquitycore.controllers.network import NetworkController
 
+from subiquity.common.errorreport import ErrorReportKind
 from subiquity.controller import SubiquityController
-from subiquity.controllers.error import ErrorReportKind
 
 
 log = logging.getLogger("subiquity.controllers.network")

subiquity/core.py

@@ -23,8 +23,6 @@ import traceback
 import time
 import urwid
 
-import apport.hookutils
-
 import jsonschema
 
 import yaml
@@ -42,7 +40,8 @@ from subiquitycore.snapd import (
     )
 from subiquitycore.view import BaseView
 
-from subiquity.controllers.error import (
+from subiquity.common.errorreport import (
+    ErrorReporter,
     ErrorReportKind,
     )
 from subiquity.journald import journald_listener
@@ -147,14 +146,15 @@ class Subiquity(Application):
             ('network-proxy-set', lambda: schedule_task(self._proxy_set())),
             ('network-change', self._network_change),
             ])
-        self._apport_data = []
-        self._apport_files = []
 
         self.autoinstall_config = {}
         self.report_to_show = None
         self.show_progress_handle = None
         self.progress_shown_time = self.aio_loop.time()
         self.progress_showing = False
+        self.error_reporter = ErrorReporter(
+            self.context.child("ErrorReporter"), self.opts.dry_run, self.root)
+
         self.note_data_for_apport("SnapUpdated", str(self.updated))
         self.note_data_for_apport("UsingAnswers", str(bool(self.answers)))
 
@@ -363,7 +363,8 @@ class Subiquity(Application):
             self.ui.body.remove_overlay(overlay)
 
     def select_initial_screen(self, index):
-        for report in self.controllers.Error.reports:
+        self.error_reporter.start_loading_reports()
+        for report in self.error_reporter.reports:
             if report.kind == ErrorReportKind.UI and not report.seen:
                 self.show_error_report(report)
                 break
@@ -456,53 +457,18 @@ class Subiquity(Application):
             ["bash"], before_hook=_before, after_hook=after_hook, cwd='/')
 
     def note_file_for_apport(self, key, path):
-        self._apport_files.append((key, path))
+        self.error_reporter.note_file_for_apport(key, path)
 
     def note_data_for_apport(self, key, value):
-        self._apport_data.append((key, value))
+        self.error_reporter.note_data_for_apport(key, value)
 
     def make_apport_report(self, kind, thing, *, interrupt, wait=False, **kw):
-        if not self.opts.dry_run and not os.path.exists('/cdrom/.disk/info'):
+        report = self.error_reporter.make_apport_report(
-            return None
+            kind, thing, wait=wait, **kw)
 
-        log.debug("generating crash report")
+        if report is not None and interrupt and self.interactive():
-
-        try:
-            report = self.controllers.Error.create_report(kind)
-        except Exception:
-            log.exception("creating crash report failed")
-            return
-
-        etype = sys.exc_info()[0]
-        if etype is not None:
-            report.pr["Title"] = "{} crashed with {}".format(
-                thing, etype.__name__)
-            report.pr['Traceback'] = traceback.format_exc()
-        else:
-            report.pr["Title"] = thing
-
-        log.info(
-            "saving crash report %r to %s", report.pr["Title"], report.path)
-
-        apport_files = self._apport_files[:]
-        apport_data = self._apport_data.copy()
-
-        def _bg_attach_hook():
-            # Attach any stuff other parts of the code think we should know
-            # about.
-            for key, path in apport_files:
-                apport.hookutils.attach_file_if_exists(report.pr, path, key)
-            for key, value in apport_data:
-                report.pr[key] = value
-            for key, value in kw.items():
-                report.pr[key] = value
-
-        report.add_info(_bg_attach_hook, wait)
-
-        if interrupt and self.interactive():
             self.show_error_report(report)
 
-        # In the fullness of time we should do the signature thing here.
         return report
 
     def show_error_report(self, report):

subiquity/ui/views/error.py

@@ -45,7 +45,7 @@ from subiquitycore.ui.width import (
     widget_width,
     )
 
-from subiquity.controllers.error import (
+from subiquity.common.errorreport import (
     ErrorReportKind,
     ErrorReportState,
     )
@@ -285,7 +285,7 @@ class ErrorReportListStretchy(Stretchy):
                 Text(""),
             ])]
         self.report_to_row = {}
-        for report in self.app.controllers.Error.reports:
+        for report in self.app.error_reporter.reports:
             connect_signal(report, "changed", self._report_changed, report)
             r = self.report_to_row[report] = self.row_for_report(report)
             rows.append(r)

subiquity/ui/views/help.py

@@ -302,7 +302,7 @@ class OpenHelpMenu(WidgetWrap):
             local = Text(
                 ('info_minor header', " " + _("Help on this screen") + " "))
 
-        if self.parent.app.controllers.Error.reports:
+        if self.parent.app.error_reporter.reports:
             view_errors = menu_item(
                 _("View error reports").format(local_title),
                 on_press=self.parent.show_errors)