Skip to content

Commit

Permalink
feat: assert that a problem report exists where needed
Browse files Browse the repository at this point in the history
Some UI methods rely on an existing problem report. Assert that they are
present instead of trying to access the report.

Signed-off-by: Benjamin Drung <benjamin.drung@canonical.com>
  • Loading branch information
bdrung authored and schopin-pro committed Nov 7, 2023
1 parent 50a9398 commit a40f5dc
Show file tree
Hide file tree
Showing 5 changed files with 53 additions and 2 deletions.
15 changes: 13 additions & 2 deletions apport/ui.py
Expand Up @@ -406,6 +406,7 @@ def run_crashes(self):
for f in reports:
if not self.load_report(f):
continue
assert self.report

# Skip crashes that happened during logout, which are uninteresting
# and confusing to see at the next login. A crash happened and gets
Expand Down Expand Up @@ -456,6 +457,7 @@ def run_crash(self, report_file):
pass
if not self.report and not self.load_report(report_file):
return
assert self.report

if "Ignore" in self.report:
return
Expand Down Expand Up @@ -1257,6 +1259,7 @@ def format_filesize(size):

def get_complete_size(self):
"""Return the size of the complete report."""
assert self.report
# report wasn't loaded, so count manually
size = 0
for k in self.report:
Expand All @@ -1271,6 +1274,7 @@ def get_complete_size(self):

def get_reduced_size(self):
"""Return the size of the reduced report."""
assert self.report
size = 0
for k in self.report:
if k != "CoreDump":
Expand Down Expand Up @@ -1300,7 +1304,7 @@ def can_examine_locally(self):

def restart(self):
"""Reopen the crashed application."""
assert "ProcCmdline" in self.report
assert self.report and "ProcCmdline" in self.report

if os.fork() == 0:
os.setsid()
Expand All @@ -1314,6 +1318,7 @@ def restart(self):

def examine(self):
"""Locally examine crash report."""
assert self.report_file
response = self.ui_question_choice(
_(
"This will launch apport-retrace in a terminal window"
Expand Down Expand Up @@ -1387,7 +1392,7 @@ def remember_send_report(self, send_report: bool) -> None:

def check_report_crashdb(self):
"""Process reports' CrashDB field, if present."""
if "CrashDB" not in self.report:
if self.report is None or "CrashDB" not in self.report:
return True

# specification?
Expand Down Expand Up @@ -1441,6 +1446,7 @@ def collect_info(
If a symptom script is given, this will be run first (used by
run_symptom()).
"""
assert self.report
self.report["_MarkForUpload"] = "True"

# skip if we already ran (we might load a processed report)
Expand Down Expand Up @@ -1726,6 +1732,7 @@ def collect_info(
on_finished()

def _is_snap(self):
assert self.report
return "SnapSource" in self.report or "Snap" in self.report

def open_url(self, url):
Expand Down Expand Up @@ -1768,6 +1775,7 @@ def open_url(self, url):
def file_report(self):
"""Upload the current report and guide the user to the reporting
web page."""
assert self.report
# FIXME: This behaviour is not really correct, but necessary as
# long as we only support a single crashdb and have whoopsie
# hardcoded. Once we have multiple crash dbs, we need to check
Expand Down Expand Up @@ -1892,6 +1900,7 @@ def check_unreportable(self):
If so, display an info message and return True.
"""
assert self.report
if not self.crashdb.accepts(self.report):
return False
if "UnreportableReason" in self.report:
Expand All @@ -1916,6 +1925,7 @@ def get_desktop_entry(self):
Return None if report cannot be associated to a .desktop file.
"""
assert self.report
if "DesktopFile" in self.report and os.path.exists(self.report["DesktopFile"]):
desktop_file = self.report["DesktopFile"]
else:
Expand Down Expand Up @@ -1948,6 +1958,7 @@ def handle_duplicate(self):
If so, tell the user about it, open the existing bug in a browser, and
return True.
"""
assert self.report
if not self.crashdb.accepts(self.report):
return False
if "_KnownReport" not in self.report:
Expand Down
1 change: 1 addition & 0 deletions bin/apport-cli
Expand Up @@ -154,6 +154,7 @@ class CLIUserInterface(apport.ui.UserInterface):

def _get_details(self):
"""Build report string for display."""
assert self.report

details = ""
max_show = 1000000
Expand Down
3 changes: 3 additions & 0 deletions gtk/apport-gtk
Expand Up @@ -115,6 +115,7 @@ class GTKUserInterface(apport.ui.UserInterface):
def ui_update_view(self, shown_keys=None):
# TODO: Split into smaller functions/methods
# pylint: disable=too-many-branches
assert self.report

# do nothing if the dialog is already destroyed when the data
# collection finishes
Expand Down Expand Up @@ -180,6 +181,7 @@ class GTKUserInterface(apport.ui.UserInterface):
prominent placement. Otherwise, provide a simple explanation for
more novice users.
"""
assert self.report
env = self.report.get("ProcEnviron", "")
from_console = "TERM=" in env and "SHELL=" in env

Expand All @@ -202,6 +204,7 @@ class GTKUserInterface(apport.ui.UserInterface):
def setup_bug_report(self):
# This is a bug generated through `apport-bug $package`, or
# `apport-collect $id`.
assert self.report

# avoid collecting information again, in this mode we already have it
if "DistroRelease" in self.report:
Expand Down
1 change: 1 addition & 0 deletions kde/apport-kde
Expand Up @@ -367,6 +367,7 @@ class MainUserInterface(apport.ui.UserInterface):
#

def ui_update_view(self, dialog, shown_keys=None):
assert self.report
# report contents
details = dialog.findChild(QTreeWidget, "details")
if shown_keys:
Expand Down

0 comments on commit a40f5dc

Please sign in to comment.