IEP-1618: Generate Bug Reports #1315
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughCorrects the Linux EIM log directory in BugReportGenerator from "/.local/share/.eim/logs" to "/.local/share/eim/logs" and removes a few extraneous blank lines/spacing in related methods. No public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/FileUtil.java (1)
416-425: Ensure the target directory exists before writingIf
createFileWithContentsInDirectoryis called with a directory path that hasn't been created yet,new FileWriter(...)will fail withFileNotFoundException. Even though the current caller prepares the directory, this helper’s name implies it should be resilient on its own. Please create the directory tree first so future callers don’t trip over this.- java.nio.file.Path filePath = directoryPath.resolve(fileName); + Files.createDirectories(directoryPath); + java.nio.file.Path filePath = directoryPath.resolve(fileName);.github/ISSUE_TEMPLATE/bug_report.md (1)
26-28: Consider reminding users to scrub sensitive data before attaching the ZIPThe generated archive can include machine-specific details and logs. It would help to remind reporters to double-check the contents before uploading.
Apply this diff to add the reminder:
**Bug Report** Please attach the bug report zip file generated. The file is generated in the workspace root directory. Espressif > Generate Bug Report +Please review the generated zip to ensure it does not contain sensitive information before attaching it here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.github/ISSUE_TEMPLATE/bug_report.md(1 hunks)bundles/com.espressif.idf.core/META-INF/MANIFEST.MF(1 hunks)bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java(1 hunks)bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/GithubIssueOpener.java(1 hunks)bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/FileUtil.java(3 hunks)bundles/com.espressif.idf.ui/plugin.xml(2 hunks)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/GenerateBugReportCommandHandler.java(1 hunks)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/Messages.java(1 hunks)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/messages.properties(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (5)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/ProcessBuilderFactory.java (1)
ProcessBuilderFactory(16-163)bundles/com.espressif.idf.core/src/com/espressif/idf/core/SystemExecutableFinder.java (1)
SystemExecutableFinder(19-131)bundles/com.espressif.idf.core/src/com/espressif/idf/core/logging/Logger.java (1)
Logger(21-137)bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/FileUtil.java (1)
FileUtil(48-493)bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/IDFUtil.java (1)
IDFUtil(59-962)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/GenerateBugReportCommandHandler.java (4)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (1)
BugReportGenerator(52-459)bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/GithubIssueOpener.java (1)
GithubIssueOpener(18-34)bundles/com.espressif.idf.core/src/com/espressif/idf/core/logging/Logger.java (1)
Logger(21-137)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/Messages.java (1)
Messages(9-88)
🔇 Additional comments (3)
bundles/com.espressif.idf.core/META-INF/MANIFEST.MF (1)
41-41: Manifest export wiring LGTMThanks for exposing the new
com.espressif.idf.core.bugpackage; this matches the new bug-reporting classes.bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/Messages.java (1)
77-78: Resource key addition looks goodThe new bundle key lines up with the properties file entry; no issues spotted.
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/update/messages.properties (1)
68-70: Message text LGTMThe formatted message (with leading/trailing separators) reads well and matches the new key.
Comment on lines
312
to
344
| List<String> arguments = new ArrayList<String>(); | ||
| IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$ | ||
| Logger.log("GIT path:" + gitPath); //$NON-NLS-1$ | ||
| String gitExecutablePath = gitPath.getDevice(); | ||
| if (StringUtil.isEmpty(gitExecutablePath)) | ||
| { | ||
| gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); | ||
| } | ||
|
|
||
| arguments.add(IDFUtil.getIDFPythonEnvPath()); | ||
| arguments.add(IDFUtil.getIDFToolsScriptFile().getAbsolutePath()); | ||
| arguments.add(IDFConstants.TOOLS_LIST_CMD); | ||
| Logger.log("Executing command: " + String.join(" ", arguments)); //$NON-NLS-1$ //$NON-NLS-2$ | ||
| Map<String, String> environment = new HashMap<>(IDFUtil.getSystemEnv()); | ||
| Logger.log(environment.toString()); | ||
| environment.put("PYTHONUNBUFFERED", "1"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
|
|
||
| environment.put("IDF_GITHUB_ASSETS", //$NON-NLS-1$ | ||
| Platform.getPreferencesService().getString(IDFCorePlugin.PLUGIN_ID, | ||
| IDFCorePreferenceConstants.IDF_GITHUB_ASSETS, | ||
| IDFCorePreferenceConstants.IDF_GITHUB_ASSETS_DEFAULT_GLOBAL, null)); | ||
|
|
||
| environment.put("PIP_EXTRA_INDEX_URL", //$NON-NLS-1$ | ||
| Platform.getPreferencesService().getString(IDFCorePlugin.PLUGIN_ID, | ||
| IDFCorePreferenceConstants.PIP_EXTRA_INDEX_URL, | ||
| IDFCorePreferenceConstants.PIP_EXTRA_INDEX_URL_DEFAULT_GLOBAL, null)); | ||
|
|
||
| if (StringUtil.isEmpty(gitExecutablePath)) | ||
| { | ||
| Logger.log("Git executable path is empty. Please check GIT_PATH environment variable."); //$NON-NLS-1$ | ||
| } | ||
| else | ||
| { | ||
| addGitToEnvironment(environment, gitExecutablePath); | ||
| } | ||
| String output = runCommand(arguments, environment); |
There was a problem hiding this comment.
Prevent NPE when git is unavailable
When SystemExecutableFinder fails to locate git, gitPath is null and gitPath.getDevice() throws, killing bug report generation. Even on Linux/macOS, getDevice() returns null, so we never enrich the PATH with the discovered executable. Guard the lookup and use toOSString() instead.
- IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$
- Logger.log("GIT path:" + gitPath); //$NON-NLS-1$
- String gitExecutablePath = gitPath.getDevice();
+ IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$
+ Logger.log("GIT path:" + gitPath); //$NON-NLS-1$
+ String gitExecutablePath = null;
+ if (gitPath != null)
+ {
+ gitExecutablePath = gitPath.toOSString();
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| List<String> arguments = new ArrayList<String>(); | |
| IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$ | |
| Logger.log("GIT path:" + gitPath); //$NON-NLS-1$ | |
| String gitExecutablePath = gitPath.getDevice(); | |
| if (StringUtil.isEmpty(gitExecutablePath)) | |
| { | |
| gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); | |
| } | |
| arguments.add(IDFUtil.getIDFPythonEnvPath()); | |
| arguments.add(IDFUtil.getIDFToolsScriptFile().getAbsolutePath()); | |
| arguments.add(IDFConstants.TOOLS_LIST_CMD); | |
| Logger.log("Executing command: " + String.join(" ", arguments)); //$NON-NLS-1$ //$NON-NLS-2$ | |
| Map<String, String> environment = new HashMap<>(IDFUtil.getSystemEnv()); | |
| Logger.log(environment.toString()); | |
| environment.put("PYTHONUNBUFFERED", "1"); //$NON-NLS-1$ //$NON-NLS-2$ | |
| environment.put("IDF_GITHUB_ASSETS", //$NON-NLS-1$ | |
| Platform.getPreferencesService().getString(IDFCorePlugin.PLUGIN_ID, | |
| IDFCorePreferenceConstants.IDF_GITHUB_ASSETS, | |
| IDFCorePreferenceConstants.IDF_GITHUB_ASSETS_DEFAULT_GLOBAL, null)); | |
| environment.put("PIP_EXTRA_INDEX_URL", //$NON-NLS-1$ | |
| Platform.getPreferencesService().getString(IDFCorePlugin.PLUGIN_ID, | |
| IDFCorePreferenceConstants.PIP_EXTRA_INDEX_URL, | |
| IDFCorePreferenceConstants.PIP_EXTRA_INDEX_URL_DEFAULT_GLOBAL, null)); | |
| if (StringUtil.isEmpty(gitExecutablePath)) | |
| { | |
| Logger.log("Git executable path is empty. Please check GIT_PATH environment variable."); //$NON-NLS-1$ | |
| } | |
| else | |
| { | |
| addGitToEnvironment(environment, gitExecutablePath); | |
| } | |
| String output = runCommand(arguments, environment); | |
| List<String> arguments = new ArrayList<String>(); | |
| IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$ | |
| Logger.log("GIT path:" + gitPath); //$NON-NLS-1$ | |
| String gitExecutablePath = null; | |
| if (gitPath != null) | |
| { | |
| gitExecutablePath = gitPath.toOSString(); | |
| } | |
| if (StringUtil.isEmpty(gitExecutablePath)) | |
| { | |
| gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); | |
| } | |
| arguments.add(IDFUtil.getIDFPythonEnvPath()); | |
| arguments.add(IDFUtil.getIDFToolsScriptFile().getAbsolutePath()); | |
| arguments.add(IDFConstants.TOOLS_LIST_CMD); | |
| Logger.log("Executing command: " + String.join(" ", arguments)); //$NON-NLS-1$ //$NON-NLS-2$ | |
| // … rest of method unchanged … |
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 312-347, guard against a null SystemExecutableFinder result and
avoid calling getDevice(); first check if gitPath is null and only then read a
path string, using gitPath.toOSString() (which works on Linux/macOS) rather than
getDevice(); if gitPath is null or the resulting string is empty, fall back to
IDFEnvironmentVariables.GIT_PATH as already intended, and only call
addGitToEnvironment when you have a non-empty git path; also update the debug
log to reflect the resolved git path or that none was found.
Comment on lines
22
to
33
| public static void openNewIssue() | ||
| throws Exception | ||
| { | ||
| String q = "&template=" + enc("bug_report.md"); // e.g. "bug_report.md" //$NON-NLS-1$ //$NON-NLS-2$ | ||
| URI uri = new URI(issueUrlBase + "?" + q); //$NON-NLS-1$ | ||
| Desktop.getDesktop().browse(uri); | ||
| } | ||
|
|
||
| private static String enc(String s) throws UnsupportedEncodingException | ||
| { | ||
| return URLEncoder.encode(s, StandardCharsets.UTF_8.toString()); | ||
| } |
There was a problem hiding this comment.
Guard against unsupported desktop browsing
On several Linux setups (minimal desktop environments, remote/headless sessions) Desktop.isDesktopSupported() can be false and Desktop.getDesktop() will throw UnsupportedOperationException, causing the command to fail instead of falling back gracefully. Please check that the browse action is supported before invoking it (and surface a clear message when it isn’t).
Apply this diff to make the handling robust:
public static void openNewIssue()
throws Exception
{
- String q = "&template=" + enc("bug_report.md"); // e.g. "bug_report.md" //$NON-NLS-1$ //$NON-NLS-2$
- URI uri = new URI(issueUrlBase + "?" + q); //$NON-NLS-1$
- Desktop.getDesktop().browse(uri);
+ if (!Desktop.isDesktopSupported())
+ {
+ throw new UnsupportedOperationException("Desktop browsing is not supported on this platform."); //$NON-NLS-1$
+ }
+
+ Desktop desktop = Desktop.getDesktop();
+ if (!desktop.isSupported(Desktop.Action.BROWSE))
+ {
+ throw new UnsupportedOperationException("Desktop browsing is not supported on this platform."); //$NON-NLS-1$
+ }
+
+ String q = "template=" + enc("bug_report.md"); //$NON-NLS-1$ //$NON-NLS-2$
+ URI uri = new URI(issueUrlBase + "?" + q); //$NON-NLS-1$
+ desktop.browse(uri);
}🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/GithubIssueOpener.java
around lines 22 to 33, the code calls Desktop.getDesktop().browse(...) without
checking whether desktop browsing is supported; update it to first verify
Desktop.isDesktopSupported() and
Desktop.getDesktop().isSupported(Desktop.Action.BROWSE) before calling browse,
handle the case where support is missing by surfacing a clear, user-facing
message (e.g., throw a descriptive Exception or log and return), and also catch
and handle UnsupportedOperationException/IOException from getDesktop()/browse so
the method fails gracefully with an informative error rather than crashing.
Comment on lines
479
to
481
| defaultHandler="com.espressif.idf.ui.handlers.GenerateBugReportCommandHandler" | ||
| id="com.espressif.idf.ui.command.generateBugReport" | ||
| name="Gennerate Bug Report"> |
There was a problem hiding this comment.
Fix the “Generate” command label typo
The command name is rendered as “Gennerate Bug Report” (double “n”), which leaks into the Key Bindings/preferences UI. Please correct the spelling.
- name="Gennerate Bug Report">
+ name="Generate Bug Report">🤖 Prompt for AI Agents
bundles/com.espressif.idf.ui/plugin.xml around lines 479 to 481: the command
name contains a typo "Gennerate Bug Report"; update the name attribute value to
the correct spelling "Generate Bug Report" so the UI and key binding/preferences
display the proper label.
Comment on lines
39
to
54
| BugReportGenerator generator = new BugReportGenerator(); | ||
| String report = generator.generateBugReport(); | ||
|
|
||
| msgConsole.println(String.format(Messages.BugReportHandler_CompletedBugReportMsg, report)); //$NON-NLS-1$ | ||
|
|
||
| msgConsole.println(); | ||
| msgConsole.println("Opening browser to create a new issue..."); //$NON-NLS-1$ | ||
| try | ||
| { | ||
| GithubIssueOpener.openNewIssue(); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| Logger.log("Failed to open browser to create a new issue"); //$NON-NLS-1$ | ||
| Logger.log(e); | ||
| } |
There was a problem hiding this comment.
Run bug-report generation off the UI thread
execute runs on the UI thread, yet BugReportGenerator.generateBugReport() performs substantial file I/O, zipping, and external tool invocations. This will freeze the workbench for several seconds, which is a poor UX. Please move the heavy lifting into a background Job (or IRunnableWithProgress) and marshal console/log updates back onto the UI thread when needed.
- BugReportGenerator generator = new BugReportGenerator();
- String report = generator.generateBugReport();
-
- msgConsole.println(String.format(Messages.BugReportHandler_CompletedBugReportMsg, report)); //$NON-NLS-1$
-
- msgConsole.println();
- msgConsole.println("Opening browser to create a new issue..."); //$NON-NLS-1$
- try
- {
- GithubIssueOpener.openNewIssue();
- }
- catch (Exception e)
- {
- Logger.log("Failed to open browser to create a new issue"); //$NON-NLS-1$
- Logger.log(e);
- }
- return null;
+ Job.create("Generate ESP-IDF bug report", monitor -> {
+ BugReportGenerator generator = new BugReportGenerator();
+ String report = generator.generateBugReport();
+
+ Display.getDefault().asyncExec(() -> {
+ try (MessageConsoleStream stream = console.newMessageStream())
+ {
+ stream.println(String.format(Messages.BugReportHandler_CompletedBugReportMsg, report));
+ stream.println();
+ stream.println("Opening browser to create a new issue..."); //$NON-NLS-1$
+ }
+ catch (IOException ioe)
+ {
+ Logger.log(ioe);
+ }
+ try
+ {
+ GithubIssueOpener.openNewIssue();
+ }
+ catch (Exception e)
+ {
+ Logger.log("Failed to open browser to create a new issue"); //$NON-NLS-1$
+ Logger.log(e);
+ }
+ });
+ }).schedule();
+ return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| BugReportGenerator generator = new BugReportGenerator(); | |
| String report = generator.generateBugReport(); | |
| msgConsole.println(String.format(Messages.BugReportHandler_CompletedBugReportMsg, report)); //$NON-NLS-1$ | |
| msgConsole.println(); | |
| msgConsole.println("Opening browser to create a new issue..."); //$NON-NLS-1$ | |
| try | |
| { | |
| GithubIssueOpener.openNewIssue(); | |
| } | |
| catch (Exception e) | |
| { | |
| Logger.log("Failed to open browser to create a new issue"); //$NON-NLS-1$ | |
| Logger.log(e); | |
| } | |
| Job.create("Generate ESP-IDF bug report", monitor -> { | |
| BugReportGenerator generator = new BugReportGenerator(); | |
| String report = generator.generateBugReport(); | |
| Display.getDefault().asyncExec(() -> { | |
| try (MessageConsoleStream stream = console.newMessageStream()) | |
| { | |
| stream.println(String.format(Messages.BugReportHandler_CompletedBugReportMsg, report)); | |
| stream.println(); | |
| stream.println("Opening browser to create a new issue..."); //$NON-NLS-1$ | |
| } | |
| catch (IOException ioe) | |
| { | |
| Logger.log(ioe); | |
| } | |
| try | |
| { | |
| GithubIssueOpener.openNewIssue(); | |
| } | |
| catch (Exception e) | |
| { | |
| Logger.log("Failed to open browser to create a new issue"); //$NON-NLS-1$ | |
| Logger.log(e); | |
| } | |
| }); | |
| }).schedule(); | |
| return null; |
Collaborator
|
@alirana01 hi! Tested under: LGTM 👍 |
AndriiFilippov
approved these changes
Sep 26, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A small change in logs path for linux
Summary by CodeRabbit