Surface the real failure cause in the error dialog (roadmap A3)
On failure the error window showed a generic message with the whole log buried in an expander. Lead instead with a focused summary: which step failed (the process label), its command, and the last stderr lines, while keeping the full log below. The runner passes the failing process to error_window, which reads the executor's bounded stderr_data. build_summary/_stderr_tail are static and unit-tested (tests/ test_error_summary.py): tail extraction, CR handling, blank-line skipping, stdout fallback, long-line truncation, and Pango-markup escaping of paths/ messages containing & < ' so set_markup can't choke. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
00fdbd50df
commit
8cb6e1aa52
3 changed files with 127 additions and 3 deletions
|
|
@ -14,14 +14,14 @@
|
||||||
# You should have received a copy of the GNU General Public License
|
# You should have received a copy of the GNU General Public License
|
||||||
# along with this program. If not, see <http://www.gnu.org/licenses/>
|
# along with this program. If not, see <http://www.gnu.org/licenses/>
|
||||||
|
|
||||||
from gi.repository import Gtk, Gdk
|
from gi.repository import Gtk, Gdk, GLib
|
||||||
import os
|
import os
|
||||||
import devedeng.configuration_data
|
import devedeng.configuration_data
|
||||||
|
|
||||||
|
|
||||||
class error_window:
|
class error_window:
|
||||||
|
|
||||||
def __init__(self):
|
def __init__(self, process=None):
|
||||||
|
|
||||||
self.config = devedeng.configuration_data.configuration.get_config()
|
self.config = devedeng.configuration_data.configuration.get_config()
|
||||||
|
|
||||||
|
|
@ -34,10 +34,60 @@ class error_window:
|
||||||
wdebug_buffer = builder.get_object("debug_buffer")
|
wdebug_buffer = builder.get_object("debug_buffer")
|
||||||
wdebug_buffer.set_text(self.config.get_log())
|
wdebug_buffer.set_text(self.config.get_log())
|
||||||
|
|
||||||
|
# lead with a focused summary of what actually failed, so the cause is
|
||||||
|
# visible without digging through the full log in the expander
|
||||||
|
wlabel = builder.get_object("label1")
|
||||||
|
if (wlabel is not None) and (process is not None):
|
||||||
|
wlabel.set_line_wrap(True)
|
||||||
|
wlabel.set_max_width_chars(80)
|
||||||
|
wlabel.set_selectable(True)
|
||||||
|
wlabel.set_markup(error_window.build_summary(process))
|
||||||
|
|
||||||
werror_window.show_all()
|
werror_window.show_all()
|
||||||
werror_window.run()
|
werror_window.run()
|
||||||
werror_window.destroy()
|
werror_window.destroy()
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def build_summary(process):
|
||||||
|
esc = GLib.markup_escape_text
|
||||||
|
parts = ["<b>" + esc(_("The disc could not be created.")) + "</b>"]
|
||||||
|
|
||||||
|
step = getattr(process, "text", "") or ""
|
||||||
|
if step:
|
||||||
|
parts.append("")
|
||||||
|
parts.append(esc(_("Failed step: %s") % step))
|
||||||
|
|
||||||
|
command = getattr(process, "command_var", None)
|
||||||
|
if command:
|
||||||
|
cmd_str = " ".join(str(c) for c in command)
|
||||||
|
if len(cmd_str) > 400:
|
||||||
|
cmd_str = cmd_str[:400] + " ..."
|
||||||
|
parts.append(esc(_("Command: %s") % cmd_str))
|
||||||
|
|
||||||
|
tail = error_window._stderr_tail(process)
|
||||||
|
if tail:
|
||||||
|
parts.append("")
|
||||||
|
parts.append(esc(_("Last messages:")))
|
||||||
|
parts.append("<tt>" + esc(tail) + "</tt>")
|
||||||
|
|
||||||
|
parts.append("")
|
||||||
|
parts.append(esc(_("See the full log below for details.")))
|
||||||
|
return "\n".join(parts)
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _stderr_tail(process, max_lines=12, max_line_len=200):
|
||||||
|
raw = getattr(process, "stderr_data", "") or ""
|
||||||
|
if not raw:
|
||||||
|
raw = getattr(process, "stdout_data", "") or ""
|
||||||
|
if not raw:
|
||||||
|
return ""
|
||||||
|
lines = [l.strip() for l in raw.replace("\r", "\n").split("\n")]
|
||||||
|
lines = [l for l in lines if l]
|
||||||
|
tail = lines[-max_lines:]
|
||||||
|
tail = [(l[:max_line_len] + " ...") if len(l) > max_line_len else l
|
||||||
|
for l in tail]
|
||||||
|
return "\n".join(tail)
|
||||||
|
|
||||||
def on_copy_clicked(self, b):
|
def on_copy_clicked(self, b):
|
||||||
|
|
||||||
clipboard = Gtk.Clipboard.get(Gdk.SELECTION_CLIPBOARD)
|
clipboard = Gtk.Clipboard.get(Gdk.SELECTION_CLIPBOARD)
|
||||||
|
|
|
||||||
|
|
@ -132,7 +132,7 @@ class runner(GObject.GObject):
|
||||||
for element in self.proc_list:
|
for element in self.proc_list:
|
||||||
element.cancel()
|
element.cancel()
|
||||||
self.wprogress.destroy()
|
self.wprogress.destroy()
|
||||||
devedeng.error.error_window()
|
devedeng.error.error_window(process)
|
||||||
self.emit("done", 1) # there was an error
|
self.emit("done", 1) # there was an error
|
||||||
return
|
return
|
||||||
|
|
||||||
|
|
|
||||||
74
tests/test_error_summary.py
Normal file
74
tests/test_error_summary.py
Normal file
|
|
@ -0,0 +1,74 @@
|
||||||
|
"""Tests for the focused failure summary shown in the error window."""
|
||||||
|
|
||||||
|
from devedeng.error import error_window
|
||||||
|
|
||||||
|
|
||||||
|
class FakeProc:
|
||||||
|
def __init__(self, text="", command_var=None, stderr_data="", stdout_data=""):
|
||||||
|
self.text = text
|
||||||
|
self.command_var = command_var or []
|
||||||
|
self.stderr_data = stderr_data
|
||||||
|
self.stdout_data = stdout_data
|
||||||
|
|
||||||
|
|
||||||
|
def test_stderr_tail_takes_last_lines():
|
||||||
|
raw = "\n".join("line %d" % i for i in range(100))
|
||||||
|
tail = error_window._stderr_tail(FakeProc(stderr_data=raw), max_lines=5)
|
||||||
|
assert tail.splitlines() == ["line 95", "line 96", "line 97",
|
||||||
|
"line 98", "line 99"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_stderr_tail_handles_carriage_returns():
|
||||||
|
raw = "p 1\rp 2\rp 3\ndone\n"
|
||||||
|
tail = error_window._stderr_tail(FakeProc(stderr_data=raw))
|
||||||
|
assert tail.splitlines()[-2:] == ["p 3", "done"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_stderr_tail_skips_blank_lines():
|
||||||
|
raw = "real\n\n \n\nlast\n"
|
||||||
|
assert error_window._stderr_tail(FakeProc(stderr_data=raw)) == "real\nlast"
|
||||||
|
|
||||||
|
|
||||||
|
def test_stderr_tail_falls_back_to_stdout():
|
||||||
|
p = FakeProc(stderr_data="", stdout_data="only on stdout\n")
|
||||||
|
assert error_window._stderr_tail(p) == "only on stdout"
|
||||||
|
|
||||||
|
|
||||||
|
def test_stderr_tail_truncates_long_lines():
|
||||||
|
raw = "x" * 500
|
||||||
|
tail = error_window._stderr_tail(FakeProc(stderr_data=raw), max_line_len=200)
|
||||||
|
assert tail.endswith(" ...")
|
||||||
|
assert len(tail) <= 210
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_summary_includes_step_command_and_tail():
|
||||||
|
p = FakeProc(
|
||||||
|
text="Converting My Movie",
|
||||||
|
command_var=["nice", "-n", "10", "ffmpeg", "-i", "in.mkv", "out.mpg"],
|
||||||
|
stderr_data="some warning\nfatal: invalid data found\n",
|
||||||
|
)
|
||||||
|
s = error_window.build_summary(p)
|
||||||
|
assert "Converting My Movie" in s
|
||||||
|
assert "ffmpeg" in s
|
||||||
|
assert "invalid data found" in s
|
||||||
|
# headline is present and markup is well-formed (bold tag closed)
|
||||||
|
assert "<b>" in s and "</b>" in s
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_summary_escapes_markup_special_chars():
|
||||||
|
# a path with & and < must be escaped so set_markup doesn't choke
|
||||||
|
p = FakeProc(text="Converting A & B <x>",
|
||||||
|
command_var=["ffmpeg", "-i", "a&b<x>.mkv"],
|
||||||
|
stderr_data="error: <bad> & worse\n")
|
||||||
|
s = error_window.build_summary(p)
|
||||||
|
assert "&" in s
|
||||||
|
assert "<" in s
|
||||||
|
# the raw unescaped sequences should not appear in dynamic content
|
||||||
|
assert "a&b<x>.mkv" not in s
|
||||||
|
|
||||||
|
|
||||||
|
def test_build_summary_without_stderr_still_works():
|
||||||
|
p = FakeProc(text="Creating ISO image", command_var=["mkisofs", "-o", "x"])
|
||||||
|
s = error_window.build_summary(p)
|
||||||
|
assert "Creating ISO image" in s
|
||||||
|
assert "mkisofs" in s
|
||||||
Loading…
Add table
Add a link
Reference in a new issue