diff --git a/cr-core/src/main/java/org/terasology/crashreporter/GlobalProperties.java b/cr-core/src/main/java/org/terasology/crashreporter/GlobalProperties.java index 7c7cade..a0f8c3a 100644 --- a/cr-core/src/main/java/org/terasology/crashreporter/GlobalProperties.java +++ b/cr-core/src/main/java/org/terasology/crashreporter/GlobalProperties.java @@ -18,6 +18,7 @@ public enum KEY { SUPPORT_FORUM_LINK, JOIN_DISCORD_LINK, REPORT_ISSUE_LINK, + REPORT_ISSUE_TEMPLATE, RES_BANNER_IMAGE, RES_SERVER_ICON, diff --git a/cr-core/src/main/java/org/terasology/crashreporter/RootPanel.java b/cr-core/src/main/java/org/terasology/crashreporter/RootPanel.java index 21de16e..19a9904 100644 --- a/cr-core/src/main/java/org/terasology/crashreporter/RootPanel.java +++ b/cr-core/src/main/java/org/terasology/crashreporter/RootPanel.java @@ -74,7 +74,13 @@ public String get() { } }); pages.add(uploadPanel); - pages.add(new FinalActionsPanel(properties, new Supplier() { + pages.add(new FinalActionsPanel(properties, exception, new Supplier() { + + @Override + public String get() { + return errorMessagePanel.getLog(); + } + }, new Supplier() { @Override public URL get() { diff --git a/cr-core/src/main/java/org/terasology/crashreporter/pages/CrashSummary.java b/cr-core/src/main/java/org/terasology/crashreporter/pages/CrashSummary.java new file mode 100644 index 0000000..64192fb --- /dev/null +++ b/cr-core/src/main/java/org/terasology/crashreporter/pages/CrashSummary.java @@ -0,0 +1,380 @@ +// Copyright 2026 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.crashreporter.pages; + +import java.io.PrintWriter; +import java.io.StringWriter; +import java.net.URL; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * Builds a pre-filled GitHub issue title/body from a crash: the exception itself, plus every other + * exception found across the crashed process' own log tabs, and the engine version/active module + * list - the reporter runs in its own JVM (see #52, subprocess isolation) and has no other way to + * reach any of the log-only information. + *

+ * On macOS, that subprocess isolation means the in-process {@code Throwable} passed to + * {@link #extract} is a best-effort reconstruction from just its class name and message (see + * {@code CrashReporter#reconstructThrowable}) - its own stack trace points into the reporter's own + * relaunch machinery, not the real crash site. Whenever the crash was also logged in one of the log + * tabs (the normal case for an engine-level crash handler), the trace captured from that log text is + * the real one and is used instead; the reconstructed exception's own trace is only a fallback for + * when nothing better is available. + *

+ * The version/module regexes here mirror two fixed, narrow log lines the engine emits at startup - + * see {@code TerasologyEngine#logEnvironmentInfo} ({@code TerasologyVersion#toString}'s + * {@code [buildNumber=..., ..., engineVersion=X, displayVersion=Y]} format) and + * {@code RegisterMods} ({@code "Activating module: :"}, once per active module). + * Log formatting is not a published API and can drift; a change there degrades this to a blank + * "unknown"/empty-list extract rather than failing the report itself. + */ +public final class CrashSummary { + + private static final int MAX_STACK_LINES = 15; + private static final int MAX_MODULES_LISTED = 30; + private static final int MAX_TITLE_MESSAGE_LENGTH = 80; + private static final int MAX_EXCEPTIONS_LISTED = 10; + private static final int CONTEXT_LINES_BEFORE = 5; + private static final String NO_TAB_LABEL = "this crash"; + + private static final Pattern ENGINE_VERSION_PATTERN = Pattern.compile("engineVersion=([^,\\]]*)"); + private static final Pattern DISPLAY_VERSION_PATTERN = Pattern.compile("displayVersion=([^,\\]]*)"); + private static final Pattern ACTIVE_MODULE_PATTERN = Pattern.compile("Activating module: (\\S+:\\S+)"); + private static final Pattern LOG_TAB_PATTERN = Pattern.compile("(?m)^=== (.*) ===$"); + // A log-formatted stack trace: a "some.FullyQualified.NameException[: message]" header line + // immediately followed by one or more "at ..."/"Caused by: ..." frame lines - the shape every + // JVM logging framework prints a Throwable in (Logback's %ex, java.util.logging, a raw + // printStackTrace()), regardless of which class emits it. + private static final Pattern STACK_TRACE_HEADER_PATTERN = Pattern.compile( + "(?m)^([\\w$]+(?:\\.[\\w$]+)+(?:Exception|Error))(:[^\\n]*)?\\n((?:[ \\t]*(?:at |Caused by:)[^\\n]*\\n?)+)"); + + private final Throwable exception; + private final List exceptionBlocks; + private final int moreExceptionsCount; + private final String engineVersion; + private final String displayVersion; + private final List activeModules; + + private CrashSummary(Throwable exception, List exceptionBlocks, int moreExceptionsCount, + String engineVersion, String displayVersion, List activeModules) { + this.exception = exception; + this.exceptionBlocks = exceptionBlocks; + this.moreExceptionsCount = moreExceptionsCount; + this.engineVersion = engineVersion; + this.displayVersion = displayVersion; + this.activeModules = activeModules; + } + + public static CrashSummary extract(Throwable exception, String combinedLogText) { + String text = combinedLogText != null ? combinedLogText : ""; + + List blocks = buildExceptionBlocks(exception, text); + int moreCount = 0; + if (blocks.size() > MAX_EXCEPTIONS_LISTED) { + moreCount = blocks.size() - MAX_EXCEPTIONS_LISTED; + blocks = new ArrayList<>(blocks.subList(0, MAX_EXCEPTIONS_LISTED)); + } + + return new CrashSummary(exception, blocks, moreCount, + firstGroup(ENGINE_VERSION_PATTERN, text), firstGroup(DISPLAY_VERSION_PATTERN, text), + extractActiveModules(text)); + } + + /** + * Builds one Markdown block per distinct exception found - the one that triggered this report + * first, then every other exception found across the log tabs - so a crash whose real cause is + * an earlier exception logged in a different tab (e.g. during init) isn't left out of the + * pre-filled issue just because it wasn't the in-process {@code exception} the reporter happened + * to be invoked with. + */ + private static List buildExceptionBlocks(Throwable exception, String combinedLogText) { + String primaryHeader = exception.toString().trim(); + List found = findAllExceptions(combinedLogText); + + ExceptionEntry primary = null; + for (ExceptionEntry entry : found) { + if (entry.header.equals(primaryHeader)) { + primary = entry; + break; + } + } + // Not logged anywhere - fall back to the exception object's own trace. On macOS that trace + // is a best-effort reconstruction (see the class javadoc) rather than the real crash site, + // but it's all that's available. There's no log text to pull leading context from either. + if (primary == null) { + primary = new ExceptionEntry(null, primaryHeader, framesFromThrowable(exception), ""); + } + + List blocks = new ArrayList<>(); + blocks.add(formatBlock(primary)); + for (ExceptionEntry entry : found) { + if (entry.header.equals(primaryHeader)) { + continue; + } + String block = formatBlock(entry); + if (!blocks.contains(block)) { + blocks.add(block); + } + } + return blocks; + } + + private static String formatBlock(ExceptionEntry entry) { + String label = entry.tabName != null ? entry.tabName : NO_TAB_LABEL; + String combined = entry.frames.isEmpty() ? entry.header : entry.header + "\n" + entry.frames; + String trace = truncateTrace(combined); + // Context isn't part of the trace itself, so it's not subject to truncateTrace()'s + // MAX_STACK_LINES cap - a few lines of what led up to the crash shouldn't cost trace detail. + String content = entry.context.isEmpty() ? trace : entry.context + "\n" + trace; + return "**" + label + "**\n\n```\n" + content + "\n```"; + } + + private static String truncateTrace(String combined) { + String[] lines = combined.split("\r?\n"); + StringBuilder builder = new StringBuilder(); + int limit = Math.min(lines.length, MAX_STACK_LINES); + for (int i = 0; i < limit; i++) { + builder.append(lines[i]).append('\n'); + } + if (lines.length > limit) { + builder.append("... ").append(lines.length - limit).append(" more line(s) - see the full log\n"); + } + return builder.toString().trim(); + } + + private static String framesFromThrowable(Throwable exception) { + StringWriter sink = new StringWriter(); + exception.printStackTrace(new PrintWriter(sink)); + String full = sink.toString(); + // printStackTrace()'s first line is exception.toString() - already the header - so only the + // "at ..."/"Caused by: ..." frames after it are needed here. + int newlineIndex = full.indexOf('\n'); + return newlineIndex >= 0 ? stripTrailingWhitespace(full.substring(newlineIndex + 1)) : ""; + } + + /** + * Like {@link String#trim()} but only at the end - frame lines are indented with a leading tab + * ({@code "\tat ..."}), which a plain {@code trim()} would strip from the first line along with + * the trailing newline it's actually meant to remove. + */ + private static String stripTrailingWhitespace(String s) { + int end = s.length(); + while (end > 0 && Character.isWhitespace(s.charAt(end - 1))) { + end--; + } + return s.substring(0, end); + } + + private static final class ExceptionEntry { + private final String tabName; + private final String header; + private final String frames; + private final String context; + + private ExceptionEntry(String tabName, String header, String frames, String context) { + this.tabName = tabName; + this.header = header; + this.frames = frames; + this.context = context; + } + } + + private static List findAllExceptions(String combinedLogText) { + List found = new ArrayList<>(); + + Matcher tabMatcher = LOG_TAB_PATTERN.matcher(combinedLogText); + int tabStart = -1; + String tabName = null; + while (true) { + boolean hasNext = tabMatcher.find(); + int nextStart = hasNext ? tabMatcher.start() : combinedLogText.length(); + if (tabName != null) { + collectExceptionHeaders(combinedLogText.substring(tabStart, nextStart), tabName, found); + } + if (!hasNext) { + break; + } + tabName = tabMatcher.group(1); + // tabMatcher.end() lands right after "===", before that line's own terminator - skip it + // too, so each tab's text starts at its real first content line instead of with a blank + // artifact line (which precedingLines() would otherwise count as logged context). + tabStart = tabMatcher.end(); + if (tabStart < combinedLogText.length() && combinedLogText.charAt(tabStart) == '\r') { + tabStart++; + } + if (tabStart < combinedLogText.length() && combinedLogText.charAt(tabStart) == '\n') { + tabStart++; + } + } + // No "=== tab ===" headers at all - a single combined-log caller (e.g. a direct test) rather + // than ErrorMessagePanel#getLog(); scan the whole text with no tab attribution. + if (tabName == null) { + collectExceptionHeaders(combinedLogText, null, found); + } + return found; + } + + private static void collectExceptionHeaders(String tabText, String tabName, List found) { + Matcher matcher = STACK_TRACE_HEADER_PATTERN.matcher(tabText); + while (matcher.find()) { + String header = (matcher.group(1) + (matcher.group(2) != null ? matcher.group(2) : "")).trim(); + String frames = stripTrailingWhitespace(matcher.group(3)); + String context = precedingLines(tabText, matcher.start(), CONTEXT_LINES_BEFORE); + found.add(new ExceptionEntry(tabName, header, frames, context)); + } + } + + /** + * @return up to {@code maxLines} lines of whatever was logged right before {@code beforeIndex} in + * {@code text} - what led up to a crash is often as useful for diagnosing it as the trace + * itself, and it's only available here (the reporter's own {@link #exception} carries no + * log context of its own). + */ + private static String precedingLines(String text, int beforeIndex, int maxLines) { + String[] lines = text.substring(0, beforeIndex).split("\r?\n", -1); + int end = lines.length; + // A trailing empty element only ever means the substring ended in a newline - i.e. the line + // right before the match, not a real blank log line - so it isn't context to show. + if (end > 0 && lines[end - 1].isEmpty()) { + end--; + } + int start = Math.max(0, end - maxLines); + StringBuilder builder = new StringBuilder(); + for (int i = start; i < end; i++) { + builder.append(lines[i]).append('\n'); + } + return stripTrailingWhitespace(builder.toString()); + } + + private static String firstGroup(Pattern pattern, String text) { + Matcher matcher = pattern.matcher(text); + return matcher.find() ? matcher.group(1).trim() : ""; + } + + private static List extractActiveModules(String text) { + List modules = new ArrayList<>(); + Matcher matcher = ACTIVE_MODULE_PATTERN.matcher(text); + while (matcher.find()) { + String module = matcher.group(1); + if (!modules.contains(module)) { + modules.add(module); + } + } + return modules; + } + + /** + * @return a short, single-line issue title: the exception's simple class name, plus a + * truncated message if it has one. + */ + public String buildTitle() { + StringBuilder title = new StringBuilder("Crash: ").append(exception.getClass().getSimpleName()); + String message = exception.getLocalizedMessage(); + if (message != null && !message.trim().isEmpty()) { + String trimmed = message.length() > MAX_TITLE_MESSAGE_LENGTH + ? message.substring(0, MAX_TITLE_MESSAGE_LENGTH - 3) + "..." + : message; + title.append(": ").append(trimmed); + } + return title.toString(); + } + + /** + * @param pastebinLink the uploaded log link, or {@code null} if the user skipped upload + * @return a Markdown issue body: every exception found, one labeled code block each (naming the + * log tab it was found in), then environment info, then a link to the full logs + */ + public String buildBody(URL pastebinLink) { + StringBuilder body = new StringBuilder(); + + body.append("### Exceptions\n\n"); + for (String block : exceptionBlocks) { + body.append(block).append("\n\n"); + } + if (moreExceptionsCount > 0) { + body.append("... ").append(moreExceptionsCount).append(" more - see the full log\n\n"); + } + + body.append("### Environment\n\n"); + body.append("- Terasology version: ").append(engineVersion.isEmpty() ? "unknown" : engineVersion); + if (!displayVersion.isEmpty()) { + body.append(" (").append(displayVersion).append(')'); + } + body.append('\n'); + body.append("- OS: ").append(System.getProperty("os.name")).append(' ') + .append(System.getProperty("os.version")).append(" (").append(System.getProperty("os.arch")).append(")\n"); + body.append("- Active modules:"); + if (activeModules.isEmpty()) { + body.append(" none found in logs\n"); + } else { + body.append('\n'); + int shown = Math.min(activeModules.size(), MAX_MODULES_LISTED); + for (int i = 0; i < shown; i++) { + body.append(" - ").append(activeModules.get(i)).append('\n'); + } + if (activeModules.size() > shown) { + body.append(" - ... ").append(activeModules.size() - shown).append(" more\n"); + } + } + + body.append("\n### Full logs\n\n"); + body.append(pastebinLink != null ? "[PasteBin](" + pastebinLink + ")\n" : "(not uploaded)\n"); + + return body.toString(); + } + + /** + * @param pastebinLink the uploaded log link, or {@code null} if the user skipped upload + * @return field ID to value for the issue form named by + * {@link org.terasology.crashreporter.GlobalProperties.KEY#REPORT_ISSUE_TEMPLATE} - only + * meaningful when a downstream app has configured one (see + * {@link GitHubIssueLinkBuilder#build(String, String, String, Map)}); the IDs here match + * Terasology's own {@code crash-bug-report.yml}. Fields with nothing extractable are + * omitted so the user's own blank field is left for them to fill in, rather than being + * pre-filled with something misleading like "unknown". + */ + public Map buildIssueFormFields(URL pastebinLink) { + Map fields = new LinkedHashMap<>(); + + if (!engineVersion.isEmpty()) { + String version = displayVersion.isEmpty() ? engineVersion : engineVersion + " (" + displayVersion + ")"; + fields.put("terasology_version", version); + } + fields.put("operating_system", System.getProperty("os.name") + " " + System.getProperty("os.version") + + " (" + System.getProperty("os.arch") + ")"); + fields.put("java_version", System.getProperty("java.version")); + + StringBuilder actual = new StringBuilder(); + for (String block : exceptionBlocks) { + actual.append(block).append("\n\n"); + } + if (moreExceptionsCount > 0) { + actual.append("... ").append(moreExceptionsCount).append(" more - see the full log\n"); + } + fields.put("actual_behavior", actual.toString().trim()); + + if (pastebinLink != null) { + fields.put("log_details", "[PasteBin](" + pastebinLink + ")"); + } + + if (!activeModules.isEmpty()) { + StringBuilder modules = new StringBuilder("Active modules:\n"); + int shown = Math.min(activeModules.size(), MAX_MODULES_LISTED); + for (int i = 0; i < shown; i++) { + modules.append("- ").append(activeModules.get(i)).append('\n'); + } + if (activeModules.size() > shown) { + modules.append("- ... ").append(activeModules.size() - shown).append(" more\n"); + } + fields.put("additional_context", modules.toString().trim()); + } + + return fields; + } +} diff --git a/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java b/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java index ebacd35..4e33e9d 100644 --- a/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java +++ b/cr-core/src/main/java/org/terasology/crashreporter/pages/FinalActionsPanel.java @@ -39,6 +39,10 @@ public class FinalActionsPanel extends JPanel { private static final long serialVersionUID = 2639334979749507943L; + private final Throwable exception; + + private final Supplier logTextSupplier; + private final Supplier uploadedFile; private final JTextArea linkText; @@ -47,8 +51,11 @@ public class FinalActionsPanel extends JPanel { private boolean pageComplete; - public FinalActionsPanel(GlobalProperties properties, Supplier uploadedFile) { + public FinalActionsPanel(GlobalProperties properties, Throwable exception, Supplier logTextSupplier, + Supplier uploadedFile) { + this.exception = exception; + this.logTextSupplier = logTextSupplier; this.uploadedFile = uploadedFile; setLayout(new BorderLayout(0, 10)); @@ -89,7 +96,20 @@ public void actionPerformed(ActionEvent e) { @Override public void actionPerformed(ActionEvent e) { - openInBrowser(properties.get(KEY.REPORT_ISSUE_LINK)); + CrashSummary summary = CrashSummary.extract(exception, logTextSupplier.get()); + String baseUrl = properties.get(KEY.REPORT_ISSUE_LINK); + String template = properties.get(KEY.REPORT_ISSUE_TEMPLATE); + String link; + if (template != null && !template.isEmpty()) { + // The downstream app has its own issue *form* - land the summary in its real + // fields instead of overwriting the whole thing with a bespoke body. + link = GitHubIssueLinkBuilder.build(baseUrl, template, summary.buildTitle(), + summary.buildIssueFormFields(uploadedFile.get())); + } else { + link = GitHubIssueLinkBuilder.build(baseUrl, summary.buildTitle(), + summary.buildBody(uploadedFile.get())); + } + openInBrowser(link); pageComplete = true; firePropertyChange("pageComplete", !pageComplete, pageComplete); } diff --git a/cr-core/src/main/java/org/terasology/crashreporter/pages/GitHubIssueLinkBuilder.java b/cr-core/src/main/java/org/terasology/crashreporter/pages/GitHubIssueLinkBuilder.java new file mode 100644 index 0000000..cff644b --- /dev/null +++ b/cr-core/src/main/java/org/terasology/crashreporter/pages/GitHubIssueLinkBuilder.java @@ -0,0 +1,61 @@ +// Copyright 2026 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.crashreporter.pages; + +import java.io.UnsupportedEncodingException; +import java.net.URLEncoder; +import java.util.Map; + +/** + * Builds a GitHub new-issue URL, pre-filled either via the classic {@code title}/{@code body} query + * parameters (works against any repo, regardless of what issue templates it has - the safe default), + * or, when a downstream app has one, via an issue *form*'s own field IDs (see + * {@link org.terasology.crashreporter.GlobalProperties.KEY#REPORT_ISSUE_TEMPLATE}) - a + * {@code template=} query parameter plus one parameter per field ID, landing the crash summary in + * the repo's own real template instead of overwriting it with a bespoke body. + */ +public final class GitHubIssueLinkBuilder { + + private GitHubIssueLinkBuilder() { + } + + public static String build(String baseUrl, String title, String body) { + if (baseUrl == null) { + return null; + } + String query = "title=" + encode(title) + "&body=" + encode(body); + return baseUrl + (baseUrl.contains("?") ? "&" : "?") + query; + } + + /** + * @param template the issue form's filename (e.g. {@code "crash-bug-report.yml"}), as it appears + * under {@code .github/ISSUE_TEMPLATE/} in the target repo + * @param fields field ID to value - entries with a {@code null}/empty value are omitted, leaving + * that field for the user to fill in themselves rather than pre-filling it blank + */ + public static String build(String baseUrl, String template, String title, Map fields) { + if (baseUrl == null) { + return null; + } + StringBuilder query = new StringBuilder("template=").append(encode(template)) + .append("&title=").append(encode(title)); + for (Map.Entry field : fields.entrySet()) { + String value = field.getValue(); + if (value == null || value.isEmpty()) { + continue; + } + query.append('&').append(field.getKey()).append('=').append(encode(value)); + } + return baseUrl + (baseUrl.contains("?") ? "&" : "?") + query; + } + + private static String encode(String value) { + try { + return URLEncoder.encode(value, "UTF-8"); + } catch (UnsupportedEncodingException e) { + // UTF-8 is a standard charset every JVM implementation is required to support. + throw new AssertionError(e); + } + } +} diff --git a/cr-core/src/test/java/org/terasology/crashreporter/pages/CrashSummaryTest.java b/cr-core/src/test/java/org/terasology/crashreporter/pages/CrashSummaryTest.java new file mode 100644 index 0000000..afb982b --- /dev/null +++ b/cr-core/src/test/java/org/terasology/crashreporter/pages/CrashSummaryTest.java @@ -0,0 +1,232 @@ +// Copyright 2026 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.crashreporter.pages; + +import org.junit.jupiter.api.Test; + +import java.net.MalformedURLException; +import java.net.URL; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Regression tests for #53 item 3: "Report Issue" opened a blank GitHub form instead of one + * pre-filled with the crash summary. + */ +class CrashSummaryTest { + + private static final String LOG_TEXT = + "10:00:00.000 [main] INFO o.t.e.version.TerasologyVersion - " + + "[buildNumber=42, buildId=42, buildTag=Terasology-42, buildUrl=, jobName=Terasology/engine/develop, " + + "dateTime=2026-08-20, displayVersion=Aeternum, engineVersion=5.4.0-SNAPSHOT]\n" + + "10:00:00.100 [main] INFO o.t.e.core.TerasologyEngine - OS: Linux, arch: amd64, version: 6.12.85\n" + + "10:00:01.000 [main] INFO o.t.e.core.modes.loadProcesses.RegisterMods - Activating module: engine:5.4.0-SNAPSHOT\n" + + "10:00:01.010 [main] INFO o.t.e.core.modes.loadProcesses.RegisterMods - Activating module: CoreAssets:2.4.0\n" + + "10:00:01.020 [main] INFO o.t.e.core.modes.loadProcesses.RegisterMods - Activating module: CoreAssets:2.4.0\n"; + + @Test + void extractsEngineAndDisplayVersion() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + String body = summary.buildBody(null); + + assertTrue(body.contains("5.4.0-SNAPSHOT"), "Expected the engine version in the body, got: " + body); + assertTrue(body.contains("Aeternum"), "Expected the display version in the body, got: " + body); + } + + @Test + void extractsActiveModulesAndDeduplicates() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + String body = summary.buildBody(null); + + assertTrue(body.contains("engine:5.4.0-SNAPSHOT"), "Expected engine module, got: " + body); + // Occurs twice in the log (duplicate "Activating module" line) - should appear once in the body. + int occurrences = body.split("CoreAssets:2\\.4\\.0", -1).length - 1; + assertEquals(1, occurrences, "Expected the duplicate module line deduplicated, got: " + body); + } + + @Test + void missingVersionAndModulesDegradeGracefullyInsteadOfFailing() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), "no relevant lines here"); + String body = summary.buildBody(null); + + assertTrue(body.contains("unknown"), "Expected a fallback for a missing version, got: " + body); + assertTrue(body.contains("none found in logs"), "Expected a fallback for an empty module list, got: " + body); + } + + @Test + void titleUsesExceptionClassAndMessage() { + CrashSummary summary = CrashSummary.extract(new IllegalStateException("world was null"), LOG_TEXT); + + assertEquals("Crash: IllegalStateException: world was null", summary.buildTitle()); + } + + @Test + void bodyIncludesTheExceptionExtract() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("kaboom"), LOG_TEXT); + String body = summary.buildBody(null); + + assertTrue(body.contains("kaboom"), "Expected the exception message in the body, got: " + body); + assertTrue(body.contains("RuntimeException"), "Expected the exception type in the body, got: " + body); + } + + @Test + void bodyIncludesThePastebinLinkWhenUploaded() throws MalformedURLException { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + URL link = new URL("https://pastebin.com/abc123"); + + String body = summary.buildBody(link); + + assertTrue(body.contains("https://pastebin.com/abc123"), "Expected the PasteBin link in the body, got: " + body); + } + + @Test + void bodyNotesWhenUploadWasSkipped() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + + String body = summary.buildBody(null); + + assertFalse(body.contains("null"), "A skipped upload must not leak the literal string \"null\" into the body: " + body); + assertTrue(body.contains("not uploaded"), "Expected a note that upload was skipped, got: " + body); + } + + // Regression: ErrorMessagePanel#getLog() combines every log tab, not just the one that + // triggered the report, but only the in-process exception ever made it into the pre-filled + // issue - an exception logged in a different tab (e.g. an earlier init-time failure) was + // silently left out even though it was right there in the combined text. All exceptions - the + // primary one and every other one found - are listed together, one labeled code block each + // (full trace, not just a one-line header - a real fix needs the actual trace), before + // "### Environment", not split across two separate sections. + @Test + void bodyListsExceptionsFoundInOtherLogTabsWithTheirFullTraceNamingTheTab() { + String combinedLog = "=== Terasology-init.log ===\n" + LOG_TEXT + + "\n=== Terasology-game.log ===\n" + + "10:10:05.123 [main] ERROR o.t.e.core.TerasologyEngine - Uncaught exception in main loop\n" + + "java.lang.NullPointerException: world was null\n" + + "\tat org.terasology.engine.core.TerasologyEngine.run(TerasologyEngine.java:200)\n"; + + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), combinedLog); + String body = summary.buildBody(null); + + assertTrue(body.contains("### Exceptions"), "Expected a single unified exceptions section, got: " + body); + assertTrue(body.contains("**Terasology-game.log**\n\n```\n" + + "10:10:05.123 [main] ERROR o.t.e.core.TerasologyEngine - Uncaught exception in main loop\n" + + "java.lang.NullPointerException: world was null\n" + + "\tat org.terasology.engine.core.TerasologyEngine.run(TerasologyEngine.java:200)\n```"), + "Expected the other tab's exception as its own labeled block, with the line logged right before it and the full " + + "trace, got: " + body); + int exceptionsIndex = body.indexOf("### Exceptions"); + int environmentIndex = body.indexOf("### Environment"); + assertTrue(exceptionsIndex >= 0 && environmentIndex > exceptionsIndex, + "Expected \"### Exceptions\" before \"### Environment\", got: " + body); + } + + @Test + void bodyAttributesThePrimaryExceptionToItsOwnTabInsteadOfListingItTwice() { + RuntimeException primary = new RuntimeException("boom"); + // The crash is very often also logged (by the crashed process itself) in one of its own + // log tabs - that's the same exception, not another one, and must not be listed twice. It's + // also the *real* trace (see the class javadoc on macOS reconstruction), so it must be used + // in preference to the exception object's own (possibly fake) trace. + String combinedLog = "=== Terasology-game.log ===\n" + + primary + "\n" + + "\tat org.terasology.engine.core.TerasologyEngine.run(TerasologyEngine.java:200)\n"; + + CrashSummary summary = CrashSummary.extract(primary, combinedLog); + String body = summary.buildBody(null); + + assertTrue(body.contains("**Terasology-game.log**\n\n```\njava.lang.RuntimeException: boom\n" + + "\tat org.terasology.engine.core.TerasologyEngine.run(TerasologyEngine.java:200)\n```"), + "Expected the primary exception's block attributed to the tab it was found in, using that tab's real trace, got: " + + body); + int blocks = body.split("\\*\\*Terasology-game\\.log\\*\\*", -1).length - 1; + assertEquals(1, blocks, "Expected exactly one block - the primary exception must not also be listed as an \"other\" one: " + + body); + } + + @Test + void bodyIncludesOnlyTheLastFiveLinesLoggedBeforeTheException() { + StringBuilder combinedLog = new StringBuilder("=== Terasology-game.log ===\n"); + for (int i = 1; i <= 8; i++) { + combinedLog.append("log line ").append(i).append('\n'); + } + combinedLog.append("java.lang.NullPointerException: world was null\n") + .append("\tat org.terasology.engine.core.TerasologyEngine.run(TerasologyEngine.java:200)\n"); + + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), combinedLog.toString()); + String body = summary.buildBody(null); + + assertFalse(body.contains("log line 1\n") || body.contains("log line 2\n") || body.contains("log line 3\n"), + "Expected only the last 5 lines of context, not all 8, got: " + body); + assertTrue(body.contains("log line 4\nlog line 5\nlog line 6\nlog line 7\nlog line 8\n" + + "java.lang.NullPointerException: world was null"), + "Expected the last 5 lines directly before the exception's header, got: " + body); + } + + @Test + void bodyFallsBackToTheExceptionsOwnTraceWhenNotFoundInAnyTab() { + RuntimeException primary = new RuntimeException("boom"); + CrashSummary summary = CrashSummary.extract(primary, LOG_TEXT); + String body = summary.buildBody(null); + + assertTrue(body.contains("**this crash**\n\n```\njava.lang.RuntimeException: boom\n"), + "Expected a fallback label and the exception's own trace when it isn't found in any log tab, got: " + body); + assertTrue(body.contains(CrashSummaryTest.class.getName()), + "Expected this test's own stack frame in the fallback trace, got: " + body); + } + + // buildIssueFormFields() feeds GitHubIssueLinkBuilder's template-based overload (see + // GitHubIssueLinkBuilderTest) - used instead of buildBody() when a downstream app has configured + // REPORT_ISSUE_TEMPLATE, landing the summary in that issue form's own fields. + @Test + void issueFormFieldsIncludeVersionOsAndTheExceptionBlocks() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + Map fields = summary.buildIssueFormFields(null); + + assertEquals("5.4.0-SNAPSHOT (Aeternum)", fields.get("terasology_version")); + assertTrue(fields.get("operating_system").contains(System.getProperty("os.name")), fields.get("operating_system")); + assertEquals(System.getProperty("java.version"), fields.get("java_version")); + assertTrue(fields.get("actual_behavior").contains("RuntimeException: boom"), fields.get("actual_behavior")); + } + + @Test + void issueFormFieldsOmitVersionWhenNotFoundInsteadOfSayingUnknown() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), "no relevant lines here"); + Map fields = summary.buildIssueFormFields(null); + + // Unlike buildBody()'s "unknown" fallback, an omitted field is left blank in the actual issue + // form for the user to fill in themselves - "unknown" pre-filled into a real form field would + // read as if the reporter deliberately couldn't tell, not as an untouched field. + assertNull(fields.get("terasology_version"), "Expected no terasology_version entry, got: " + fields); + } + + @Test + void issueFormFieldsOmitLogDetailsWhenUploadWasSkipped() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + Map fields = summary.buildIssueFormFields(null); + + assertNull(fields.get("log_details"), "Expected no log_details entry when nothing was uploaded, got: " + fields); + } + + @Test + void issueFormFieldsIncludeThePastebinLinkWhenUploaded() throws MalformedURLException { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + URL link = new URL("https://pastebin.com/abc123"); + + Map fields = summary.buildIssueFormFields(link); + + assertTrue(fields.get("log_details").contains("https://pastebin.com/abc123"), fields.get("log_details")); + } + + @Test + void issueFormFieldsIncludeActiveModulesUnderAdditionalContext() { + CrashSummary summary = CrashSummary.extract(new RuntimeException("boom"), LOG_TEXT); + Map fields = summary.buildIssueFormFields(null); + + assertTrue(fields.get("additional_context").contains("engine:5.4.0-SNAPSHOT"), fields.get("additional_context")); + } +} diff --git a/cr-core/src/test/java/org/terasology/crashreporter/pages/GitHubIssueLinkBuilderTest.java b/cr-core/src/test/java/org/terasology/crashreporter/pages/GitHubIssueLinkBuilderTest.java new file mode 100644 index 0000000..bbcb421 --- /dev/null +++ b/cr-core/src/test/java/org/terasology/crashreporter/pages/GitHubIssueLinkBuilderTest.java @@ -0,0 +1,79 @@ +// Copyright 2026 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 + +package org.terasology.crashreporter.pages; + +import org.junit.jupiter.api.Test; + +import java.util.LinkedHashMap; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class GitHubIssueLinkBuilderTest { + + @Test + void returnsNullWhenBaseUrlIsNotConfigured() { + // REPORT_ISSUE_LINK is only set by downstream apps (cr-terasology, cr-destsol, ...), not by + // cr-core's own defaults - build() must not silently produce a broken "null?title=..." link. + assertNull(GitHubIssueLinkBuilder.build(null, "t", "b")); + assertNull(GitHubIssueLinkBuilder.build(null, "template.yml", "t", new LinkedHashMap<>())); + } + + @Test + void appendsTitleAndBodyAsQueryParameters() { + String link = GitHubIssueLinkBuilder.build("https://github.com/MovingBlocks/Terasology/issues/new", + "Crash: NullPointerException", "some body text"); + + assertTrue(link.startsWith("https://github.com/MovingBlocks/Terasology/issues/new?")); + assertTrue(link.contains("title=Crash%3A+NullPointerException"), link); + assertTrue(link.contains("body=some+body+text"), link); + } + + @Test + void encodesSpecialCharactersInTitleAndBody() { + String link = GitHubIssueLinkBuilder.build("https://example.com/issues/new", "a & b", "line1\nline2"); + + assertTrue(link.contains("title=a+%26+b"), link); + assertTrue(link.contains("body=line1%0Aline2"), link); + } + + @Test + void usesAmpersandWhenBaseUrlAlreadyHasAQueryString() { + String link = GitHubIssueLinkBuilder.build("https://example.com/issues/new?template=bug", "t", "b"); + + assertEquals("https://example.com/issues/new?template=bug&title=t&body=b", link); + } + + @Test + void formBuildAppendsTemplateTitleAndEachFieldAsItsOwnQueryParameter() { + Map fields = new LinkedHashMap<>(); + fields.put("terasology_version", "5.4.0-SNAPSHOT"); + fields.put("operating_system", "Mac OS X"); + + String link = GitHubIssueLinkBuilder.build("https://github.com/MovingBlocks/Terasology/issues/new", + "crash-bug-report.yml", "Crash: NullPointerException", fields); + + assertTrue(link.contains("template=crash-bug-report.yml"), link); + assertTrue(link.contains("title=Crash%3A+NullPointerException"), link); + assertTrue(link.contains("terasology_version=5.4.0-SNAPSHOT"), link); + assertTrue(link.contains("operating_system=Mac+OS+X"), link); + } + + @Test + void formBuildOmitsFieldsWithNoValueInsteadOfPreFillingThemBlank() { + Map fields = new LinkedHashMap<>(); + fields.put("terasology_version", ""); + fields.put("java_version", null); + fields.put("operating_system", "Linux"); + + String link = GitHubIssueLinkBuilder.build("https://example.com/issues/new", "crash-bug-report.yml", "t", fields); + + assertFalse(link.contains("terasology_version="), link); + assertFalse(link.contains("java_version="), link); + assertTrue(link.contains("operating_system=Linux"), link); + } +} diff --git a/cr-terasology/src/main/resources/crashreporter.properties b/cr-terasology/src/main/resources/crashreporter.properties index 47a7a74..45c4030 100644 --- a/cr-terasology/src/main/resources/crashreporter.properties +++ b/cr-terasology/src/main/resources/crashreporter.properties @@ -1,5 +1,6 @@ SUPPORT_FORUM_LINK=http://forum.terasology.org/forum/support.20/ REPORT_ISSUE_LINK=https://github.com/MovingBlocks/Terasology/issues/new +REPORT_ISSUE_TEMPLATE=crash-bug-report.yml JOIN_DISCORD_LINK=https://discord.gg/terasology RES_BANNER_IMAGE=icons/banner.jpg