IEP-1618: Generate Bug Report Feature #1298
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| */ | ||
| public class BugReportGenerator | ||
| { | ||
| private static final String bugReportDirectoryPrefix = "bug_report_"; //$NON-NLS-1$ |
There was a problem hiding this comment.
nit: the naming convention for a final static field is all uppercase letters with underscores separating words.
| String osName = System.getProperty("os.name", "Unknown"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
| String osVersion = System.getProperty("os.version", "Unknown"); //$NON-NLS-1$ //$NON-NLS-2$ | ||
| String arch = System.getProperty("os.arch", "Unknown"); //$NON-NLS-1$//$NON-NLS-2$ | ||
|
|
There was a problem hiding this comment.
nit: we can use a variable here for "Unknown"
| String r1 = base.relativize(p1).toString().replace(File.separatorChar, '/'); | ||
| String r2 = base.relativize(p2).toString().replace(File.separatorChar, '/'); | ||
| return r1.compareTo(r2); | ||
| }).collect(Collectors.toList()); |
There was a problem hiding this comment.
nit: we can use Stream.toList()
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/FileUtil.java (1)
416-425: Fix param typo, ensure parent dirs exist, and write with UTF‑8 (no platform default).The method uses a misspelled parameter, doesn’t ensure directories exist, and writes with platform encoding. This can fail on first run and corrupt non‑ASCII content.
- public static File createFileWithContentsInDirectory(String fileName, String content, String direcotry) throws IOException - { - java.nio.file.Path directoryPath = java.nio.file.Paths.get(direcotry); - java.nio.file.Path filePath = directoryPath.resolve(fileName); - try (FileWriter writer = new FileWriter(filePath.toFile())) - { - writer.write(content); - } - return filePath.toFile(); - } + public static File createFileWithContentsInDirectory(String fileName, String content, String directory) throws IOException + { + java.nio.file.Path directoryPath = java.nio.file.Paths.get(directory); + java.nio.file.Path filePath = directoryPath.resolve(fileName); + // Ensure all parent directories exist (handles fileName with subdirs too) + java.nio.file.Files.createDirectories(filePath.getParent()); + try (java.io.BufferedWriter writer = + java.nio.file.Files.newBufferedWriter( + filePath, + java.nio.charset.StandardCharsets.UTF_8, + java.nio.file.StandardOpenOption.CREATE, + java.nio.file.StandardOpenOption.TRUNCATE_EXISTING)) + { + writer.write(content != null ? content : ""); + } + return filePath.toFile(); + }Add imports if not already present (see earlier import diff).
🧹 Nitpick comments (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/FileUtil.java (2)
11-25: Switch to NIO writer + UTF-8; adjust imports and drop FileWriter.Moving
createFileWithContentsInDirectorytoFiles.newBufferedWriter(..., StandardCharsets.UTF_8, ...)avoids platform-default encodings and aligns with the rest of the class’ NIO usage. RemoveFileWriterand add the needed imports.-import java.io.FileWriter; +import java.io.BufferedWriter; +import java.nio.charset.StandardCharsets; +import java.nio.file.StandardOpenOption;
352-361: Close Scanner with try‑with‑resources; prefer System.lineSeparator().Prevents a resource leak if an exception is thrown mid‑loop and clarifies the line separator.
- try - { - Scanner scanner = new Scanner(new File(absoluteFilePath)); - while (scanner.hasNext()) - { - fileContents.append(scanner.nextLine()); - fileContents.append(System.getProperty("line.separator")); //$NON-NLS-1$ - } - scanner.close(); - } + try (Scanner scanner = new Scanner(new File(absoluteFilePath), UTF_8)) + { + while (scanner.hasNextLine()) + { + fileContents.append(scanner.nextLine()) + .append(System.lineSeparator()); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
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/util/FileUtil.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
🔇 Additional comments (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/util/FileUtil.java (2)
427-491: LGTM: robust, ordered zipping with preserved timestamps.Good use of
Files.walk, stable ordering (dirs first), explicit root entry, and try‑with‑resources. This underpins the bug‑report feature well.
434-437: Apply directory-guard and force UTF-8 (plugin targets JavaSE-17)
- After
insertPath base = directoryToZip.toPath(); Path zipPath = Paths.get(zipFileName);// Prevent placing the ZIP inside the source directory Path absBase = base.toAbsolutePath(); Path absZip = zipPath.toAbsolutePath(); if (absZip.startsWith(absBase)) { throw new IllegalArgumentException("zipFileName must not be inside directoryToZip"); }- Update the ZipOutputStream ctor at line 457 to
- try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipPath)))
- try (ZipOutputStream zos = new ZipOutputStream(
Files.newOutputStream(zipPath), StandardCharsets.UTF_8))
- Add import:
import java.nio.charset.StandardCharsets;
kolipakakondal
left a comment
kolipakakondal
left a comment
There was a problem hiding this comment.
Hi @alirana01 , thanks for the PR - this is really helpful for troubleshooting purposes.
I noticed that we're uploading the entire .metadata folder from the workspace. This folder is usually large and contains a lot of additional information about plugins. What we probably need is only the .metadata/.log file.
It would be great if you could incorporate this change. Otherwise, it looks good to me.
Hi @kolipakakondal |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (2)
87-93: Fix Windows EIM logs path: use LOCALAPPDATA (not APPDATA) and drop '\Local'.APPDATA points to Roaming; the logs live under Local. Use LOCALAPPDATA and construct "...\eim\logs".
- case Platform.OS_WIN32: - eimPath = System.getenv("APPDATA"); //$NON-NLS-1$ - if (!StringUtil.isEmpty(eimPath)) - { - eimPath = eimPath + "\\Local\\eim\\logs"; //$NON-NLS-1$ - } + case Platform.OS_WIN32: + eimPath = System.getenv("LOCALAPPDATA"); //$NON-NLS-1$ + if (!StringUtil.isEmpty(eimPath)) + { + eimPath = eimPath + "\\eim\\logs"; //$NON-NLS-1$ + }
193-195: Compile fix: use OperatingSystemMXBean physical memory APIs.- freePhys = osBean.getFreeMemorySize(); - totalPhys = osBean.getTotalMemorySize(); + freePhys = osBean.getFreePhysicalMemorySize(); + totalPhys = osBean.getTotalPhysicalMemorySize();
🧹 Nitpick comments (4)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (4)
262-266: Remove unused filesToZip list.Not used; dead code.
- List<File> filesToZip = new LinkedList<>(); - filesToZip.add(installedToolsFile); - filesToZip.add(productInfoFile); - filesToZip.add(basicSysInfoFile);
77-79: Minor: avoid trailing separator in directory path.Cleaner and cross-platform.
- String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd_HHmmss")); //$NON-NLS-1$ - bugReportDirectory = getWorkspaceDirectory().resolve(BUG_REPORT_DIRECTORY_PREFIX + timestamp + File.separator) - .toFile(); + String timestamp = LocalDateTime.now().format(DateTimeFormatter.ofPattern("yyyyMMdd_HHmmss")); //$NON-NLS-1$ + bugReportDirectory = getWorkspaceDirectory().resolve(BUG_REPORT_DIRECTORY_PREFIX + timestamp).toFile();
268-276: Prefer File(parent, child) and tidy NLS comments.Reduces string concat and fixes stray ')' in the comment.
- File ideLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "ide_metadata_logs"); //$NON-NLS-1$ ) + File ideLogDir = new File(bugReportDirectory, "ide_metadata_logs"); //$NON-NLS-1$- File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ ) + File eimLogDir = new File(bugReportDirectory, "eim_logs"); //$NON-NLS-1$Also applies to: 279-281
101-107: Confirm Linux EIM logs location and consider checking both candidates.Path currently uses "
/.local/share/.eim/logs"; some installs use "/.local/share/eim/logs". Recommend probing both and returning null if neither exists.- case Platform.OS_LINUX: - eimPath = System.getProperty("user.home"); //$NON-NLS-1$ - if (!StringUtil.isEmpty(eimPath)) - { - eimPath = eimPath + "/.local/share/.eim/logs"; //$NON-NLS-1$ - } + case Platform.OS_LINUX: + String home = System.getProperty("user.home"); //$NON-NLS-1$ + if (!StringUtil.isEmpty(home)) { + String[] candidates = { + home + "/.local/share/eim/logs", //$NON-NLS-1$ + home + "/.local/share/.eim/logs" //$NON-NLS-1$ + }; + for (String c : candidates) { + if (new File(c).exists()) { eimPath = c; break; } + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (3)
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/StringUtil.java (1)
StringUtil(11-25)
⏰ 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). (1)
- GitHub Check: build_macos
🔇 Additional comments (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (1)
122-179: IDE metadata logs selection looks good.Whitelisting .log (today/prev-day) and version.ini mitigates PII risk vs copying entire .metadata.
| return new File(eimPath); | ||
| } |
There was a problem hiding this comment.
Prevent catastrophic copy when EIM path is missing.
getEimLogPath() returns new File("") on empty, which resolves to the CWD; copyDirectory would then copy the whole working dir. Return null when unresolved and guard the copy.
- return new File(eimPath);
+ if (StringUtil.isEmpty(eimPath)) {
+ return null;
+ }
+ return new File(eimPath);- File eimLogPath = getEimLogPath();
- File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ )
- FileUtil.copyDirectory(eimLogPath, eimLogDir);
+ File eimLogPath = getEimLogPath();
+ if (eimLogPath != null && eimLogPath.exists() && eimLogPath.isDirectory()) {
+ File eimLogDir = new File(bugReportDirectory, "eim_logs"); //$NON-NLS-1$
+ FileUtil.copyDirectory(eimLogPath, eimLogDir);
+ }Also applies to: 279-282
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 112-113 (and likewise around 279-282), getEimLogPath() currently
returns new File("") when the EIM path is empty which resolves to the CWD and
can cause copyDirectory to copy the whole working directory; change
getEimLogPath() to return null when the path is empty/unresolved and update the
callers to guard before copying (skip the copy and log a warning or handle
absence) so copyDirectory is only invoked with a non-null, valid directory File.
| try | ||
| { | ||
| File installedToolsFile = FileUtil.createFileWithContentsInDirectory("installed_tools.txt", //$NON-NLS-1$ | ||
| installedToolsCommandOutput, bugReportDirectory.getAbsolutePath()); | ||
| File productInfoFile = FileUtil.createFileWithContentsInDirectory("product_information.txt", //$NON-NLS-1$ | ||
| productInformationCommandOutput, bugReportDirectory.getAbsolutePath()); | ||
| File basicSysInfoFile = createBasicSystemInfoFile(); |
There was a problem hiding this comment.
Null-safe file writes for command outputs.
Avoid NPE if either output is null.
- File installedToolsFile = FileUtil.createFileWithContentsInDirectory("installed_tools.txt", //$NON-NLS-1$
- installedToolsCommandOutput, bugReportDirectory.getAbsolutePath());
- File productInfoFile = FileUtil.createFileWithContentsInDirectory("product_information.txt", //$NON-NLS-1$
- productInformationCommandOutput, bugReportDirectory.getAbsolutePath());
+ String installedToolsOut = installedToolsCommandOutput == null ? StringUtil.EMPTY : installedToolsCommandOutput;
+ String productInfoOut = productInformationCommandOutput == null ? StringUtil.EMPTY : productInformationCommandOutput;
+ File installedToolsFile = FileUtil.createFileWithContentsInDirectory("installed_tools.txt", //$NON-NLS-1$
+ installedToolsOut, bugReportDirectory.getAbsolutePath());
+ File productInfoFile = FileUtil.createFileWithContentsInDirectory("product_information.txt", //$NON-NLS-1$
+ productInfoOut, bugReportDirectory.getAbsolutePath());📝 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.
| try | |
| { | |
| File installedToolsFile = FileUtil.createFileWithContentsInDirectory("installed_tools.txt", //$NON-NLS-1$ | |
| installedToolsCommandOutput, bugReportDirectory.getAbsolutePath()); | |
| File productInfoFile = FileUtil.createFileWithContentsInDirectory("product_information.txt", //$NON-NLS-1$ | |
| productInformationCommandOutput, bugReportDirectory.getAbsolutePath()); | |
| File basicSysInfoFile = createBasicSystemInfoFile(); | |
| try | |
| { | |
| // Null-safe command outputs to avoid passing null into the file-creation utility | |
| String installedToolsOut = installedToolsCommandOutput == null | |
| ? StringUtil.EMPTY | |
| : installedToolsCommandOutput; | |
| String productInfoOut = productInformationCommandOutput == null | |
| ? StringUtil.EMPTY | |
| : productInformationCommandOutput; | |
| File installedToolsFile = FileUtil.createFileWithContentsInDirectory( | |
| "installed_tools.txt", //$NON-NLS-1$ | |
| installedToolsOut, | |
| bugReportDirectory.getAbsolutePath()); | |
| File productInfoFile = FileUtil.createFileWithContentsInDirectory( | |
| "product_information.txt", //$NON-NLS-1$ | |
| productInfoOut, | |
| bugReportDirectory.getAbsolutePath()); | |
| File basicSysInfoFile = createBasicSystemInfoFile(); |
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 254 to 260, the code calls
FileUtil.createFileWithContentsInDirectory using installedToolsCommandOutput and
productInformationCommandOutput which can be null and cause an NPE; fix by
ensuring null-safe values before calling the method (e.g., replace nulls with
empty strings or only create the file when the output is non-null). Concretely,
compute safe strings like String installedTools = installedToolsCommandOutput ==
null ? "" : installedToolsCommandOutput and String productInfo =
productInformationCommandOutput == null ? "" : productInformationCommandOutput,
then pass those safe values to createFileWithContentsInDirectory (or skip file
creation when you prefer) so no NPE occurs.
|
@alirana01 hi! Tested under: If the problem occurs during the ESP-IDF tools installation, clicking "Generate Bug Report" brings up a pop-up asking to install and configure the IDF. Does this mean that "Generate Bug Report" doesn't provide any details about issues during the tools installation process? |
|
@alirana01 hi! Tested under: Install tools -> execute "Generate Bug Report" :
but error occurs in Error log |
Is it how it's supposed to be? |
and log file just in case: |
… to avoid job wait
Hi @AndriiFilippov |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (4)
64-64: Use uppercase for static final field naming convention.Java naming conventions recommend using uppercase letters with underscores for static final fields.
Apply this fix:
-private static final String UNKNOWN = "Unknown"; //$NON-NLS-1$ +private static final String UNKNOWN_VALUE = "Unknown"; //$NON-NLS-1$Also update usages at lines 196-198:
-String osName = System.getProperty("os.name", UNKNOWN); //$NON-NLS-1$ -String osVersion = System.getProperty("os.version", UNKNOWN); //$NON-NLS-1$ -String arch = System.getProperty("os.arch", UNKNOWN); //$NON-NLS-1$ +String osName = System.getProperty("os.name", UNKNOWN_VALUE); //$NON-NLS-1$ +String osVersion = System.getProperty("os.version", UNKNOWN_VALUE); //$NON-NLS-1$ +String arch = System.getProperty("os.arch", UNKNOWN_VALUE); //$NON-NLS-1$
103-109: Fix incorrect Windows EIM logs path.The code uses
APPDATA(roaming) but then appends\Local\eim\logs. You should useLOCALAPPDATAenvironment variable instead for the local app data directory.Apply this fix:
case Platform.OS_WIN32: - eimPath = System.getenv("APPDATA"); //$NON-NLS-1$ + eimPath = System.getenv("LOCALAPPDATA"); //$NON-NLS-1$ if (!StringUtil.isEmpty(eimPath)) { - eimPath = eimPath + "\\Local\\eim\\logs"; //$NON-NLS-1$ + eimPath = eimPath + "\\eim\\logs"; //$NON-NLS-1$ } break;
206-207: Compilation error: Incorrect method names for memory retrieval.The methods
getFreeMemorySize()andgetTotalMemorySize()don't exist oncom.sun.management.OperatingSystemMXBean. Use the correct method names.Apply this fix:
- freePhys = osBean.getFreeMemorySize(); - totalPhys = osBean.getTotalMemorySize(); + freePhys = osBean.getFreePhysicalMemorySize(); + totalPhys = osBean.getTotalPhysicalMemorySize();
128-129: Prevent catastrophic copy when EIM path is empty.When the EIM path isn't found, the method returns
new File("")which resolves to the current working directory. This could causeFileUtil.copyDirectoryto copy the entire working directory into the bug report.Apply this fix to return
nullwhen path is empty and guard the copy operation:- return new File(eimPath); + if (StringUtil.isEmpty(eimPath)) { + return null; + } + return new File(eimPath);And update the copy operation at lines 290-292:
File eimLogPath = getEimLogPath(); -File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ ) -FileUtil.copyDirectory(eimLogPath, eimLogDir); +if (eimLogPath != null && eimLogPath.exists() && eimLogPath.isDirectory()) { + File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ + FileUtil.copyDirectory(eimLogPath, eimLogDir); +}Also applies to: 290-292
🧹 Nitpick comments (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (1)
262-301: Add error handling for individual file creation failures.The current implementation doesn't handle partial failures gracefully. If any file creation fails early, the method will throw an exception and skip creating the zip file entirely.
Consider catching exceptions for individual operations and continuing with what's available:
public String generateBugReport() { if (!bugReportDirectory.exists()) { bugReportDirectory.mkdirs(); } + List<String> errors = new ArrayList<>(); try { - File installedToolsFile = getInstalledToolsInfoFile(); - File productInfoFile = getProductInfoFile(); - File basicSysInfoFile = createBasicSystemInfoFile(); + File installedToolsFile = null; + File productInfoFile = null; + File basicSysInfoFile = null; + + try { + installedToolsFile = getInstalledToolsInfoFile(); + } catch (IOException e) { + errors.add("Failed to get installed tools info: " + e.getMessage()); + Logger.log(e); + } + + try { + productInfoFile = getProductInfoFile(); + } catch (IOException e) { + errors.add("Failed to get product info: " + e.getMessage()); + Logger.log(e); + } + + try { + basicSysInfoFile = createBasicSystemInfoFile(); + } catch (IOException e) { + errors.add("Failed to create basic system info: " + e.getMessage()); + Logger.log(e); + } List<File> filesToZip = new LinkedList<>(); - filesToZip.add(installedToolsFile); - filesToZip.add(productInfoFile); - filesToZip.add(basicSysInfoFile); + if (installedToolsFile != null) filesToZip.add(installedToolsFile); + if (productInfoFile != null) filesToZip.add(productInfoFile); + if (basicSysInfoFile != null) filesToZip.add(basicSysInfoFile); + + // If there were errors, create an error log file + if (!errors.isEmpty()) { + File errorLog = FileUtil.createFileWithContentsInDirectory("errors.txt", + String.join(System.lineSeparator(), errors), + bugReportDirectory.getAbsolutePath()); + filesToZip.add(errorLog); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java(1 hunks)bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/GenerateBugReportCommandHandler.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
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-455)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)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (8)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/IDFCorePlugin.java (1)
IDFCorePlugin(20-77)bundles/com.espressif.idf.core/src/com/espressif/idf/core/IDFCorePreferenceConstants.java (1)
IDFCorePreferenceConstants(24-79)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.core/src/com/espressif/idf/core/util/StringUtil.java (1)
StringUtil(11-25)
⏰ 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). (2)
- GitHub Check: build
- GitHub Check: build_macos
🔇 Additional comments (1)
bundles/com.espressif.idf.ui/src/com/espressif/idf/ui/handlers/GenerateBugReportCommandHandler.java (1)
1-86: LGTM! The handler implementation is clean and straightforward.The command handler correctly integrates with the bug report generation infrastructure and provides appropriate user feedback through the console.
| Logger.log("GIT path:" + gitPath); //$NON-NLS-1$ | ||
| String gitExecutablePath = gitPath.getDevice(); | ||
| if (StringUtil.isEmpty(gitExecutablePath)) | ||
| { | ||
| gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); | ||
| } |
There was a problem hiding this comment.
Logic error: git path extraction is incorrect.
IPath.getDevice() returns the device identifier (e.g., drive letter on Windows), not the full path. You should use toOSString() to get the complete path.
Apply this fix:
IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$
Logger.log("GIT path:" + gitPath); //$NON-NLS-1$
-String gitExecutablePath = gitPath.getDevice();
+String gitExecutablePath = gitPath != null ? gitPath.toOSString() : null;
if (StringUtil.isEmpty(gitExecutablePath))
{
gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH);
}📝 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.
| Logger.log("GIT path:" + gitPath); //$NON-NLS-1$ | |
| String gitExecutablePath = gitPath.getDevice(); | |
| if (StringUtil.isEmpty(gitExecutablePath)) | |
| { | |
| gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); | |
| } | |
| IPath gitPath = new SystemExecutableFinder().find("git"); //$NON-NLS-1$ | |
| Logger.log("GIT path:" + gitPath); //$NON-NLS-1$ | |
| String gitExecutablePath = gitPath != null ? gitPath.toOSString() : null; | |
| if (StringUtil.isEmpty(gitExecutablePath)) | |
| { | |
| gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); | |
| } |
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 310 to 315, the code incorrectly uses IPath.getDevice() (which
returns only the device/drive) to obtain the git path; change it to use
gitPath.toOSString() to get the full OS path string, update the
gitExecutablePath assignment to use that value, and adjust the logging to print
the full path; keep the existing fallback to IDFEnvironmentVariables.GIT_PATH if
the resulting string is empty or null.
| { | ||
| IPath gitPath = new org.eclipse.core.runtime.Path(gitExecutablePath); | ||
| if (gitPath.toFile().exists()) | ||
| { | ||
| String gitDir = gitPath.removeLastSegments(1).toOSString(); | ||
| String path1 = environment.get("PATH"); //$NON-NLS-1$ | ||
| String path2 = environment.get("Path"); //$NON-NLS-1$ | ||
| if (!StringUtil.isEmpty(path1) && !path1.contains(gitDir)) // Git not found on the PATH environment | ||
| { | ||
| path1 = gitDir.concat(";").concat(path1); //$NON-NLS-1$ | ||
| environment.put("PATH", path1); //$NON-NLS-1$ | ||
| } | ||
| else if (!StringUtil.isEmpty(path2) && !path2.contains(gitDir)) // Git not found on the Path environment | ||
| { | ||
| path2 = gitDir.concat(";").concat(path2); //$NON-NLS-1$ | ||
| environment.put("Path", path2); //$NON-NLS-1$ | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider File.pathSeparator for cross-platform PATH handling.
The code uses ; as the path separator, which is Windows-specific. On Unix-like systems, : is the path separator.
Apply this fix for cross-platform compatibility:
if (!StringUtil.isEmpty(path1) && !path1.contains(gitDir)) // Git not found on the PATH environment
{
- path1 = gitDir.concat(";").concat(path1); //$NON-NLS-1$
+ path1 = gitDir.concat(File.pathSeparator).concat(path1);
environment.put("PATH", path1); //$NON-NLS-1$
}
else if (!StringUtil.isEmpty(path2) && !path2.contains(gitDir)) // Git not found on the Path environment
{
- path2 = gitDir.concat(";").concat(path2); //$NON-NLS-1$
+ path2 = gitDir.concat(File.pathSeparator).concat(path2);
environment.put("Path", path2); //$NON-NLS-1$
}📝 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.
| { | |
| IPath gitPath = new org.eclipse.core.runtime.Path(gitExecutablePath); | |
| if (gitPath.toFile().exists()) | |
| { | |
| String gitDir = gitPath.removeLastSegments(1).toOSString(); | |
| String path1 = environment.get("PATH"); //$NON-NLS-1$ | |
| String path2 = environment.get("Path"); //$NON-NLS-1$ | |
| if (!StringUtil.isEmpty(path1) && !path1.contains(gitDir)) // Git not found on the PATH environment | |
| { | |
| path1 = gitDir.concat(";").concat(path1); //$NON-NLS-1$ | |
| environment.put("PATH", path1); //$NON-NLS-1$ | |
| } | |
| else if (!StringUtil.isEmpty(path2) && !path2.contains(gitDir)) // Git not found on the Path environment | |
| { | |
| path2 = gitDir.concat(";").concat(path2); //$NON-NLS-1$ | |
| environment.put("Path", path2); //$NON-NLS-1$ | |
| } | |
| } | |
| } | |
| { | |
| IPath gitPath = new org.eclipse.core.runtime.Path(gitExecutablePath); | |
| if (gitPath.toFile().exists()) | |
| { | |
| String gitDir = gitPath.removeLastSegments(1).toOSString(); | |
| String path1 = environment.get("PATH"); //$NON-NLS-1$ | |
| String path2 = environment.get("Path"); //$NON-NLS-1$ | |
| if (!StringUtil.isEmpty(path1) && !path1.contains(gitDir)) // Git not found on the PATH environment | |
| { | |
| path1 = gitDir.concat(File.pathSeparator).concat(path1); | |
| environment.put("PATH", path1); //$NON-NLS-1$ | |
| } | |
| else if (!StringUtil.isEmpty(path2) && !path2.contains(gitDir)) // Git not found on the Path environment | |
| { | |
| path2 = gitDir.concat(File.pathSeparator).concat(path2); | |
| environment.put("Path", path2); //$NON-NLS-1$ | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 349 to 367, the code appends a hard-coded ";" when prepending
gitDir to PATH/Path which is Windows-only; replace the literal ";" with the
platform-specific separator by using java.io.File.pathSeparator when
concatenating the new PATH value, and ensure any string concat calls use that
File.pathSeparator so the PATH is corrected on Unix-like systems as well.
| private void showEspIdfVersion() | ||
| { | ||
| if (IDFUtil.getIDFPath() != null && IDFUtil.getIDFPythonEnvPath() != null) | ||
| { | ||
| List<String> commands = new ArrayList<>(); | ||
| commands.add(IDFUtil.getIDFPythonEnvPath()); | ||
| commands.add(IDFUtil.getIDFPythonScriptFile().getAbsolutePath()); | ||
| commands.add("--version"); //$NON-NLS-1$ | ||
| Map<String, String> envMap = new IDFEnvironmentVariables().getSystemEnvMap(); | ||
| Logger.log(runCommand(commands, envMap)); | ||
| } | ||
| else | ||
| { | ||
| Logger.log("ESP-IDF version cannot be checked. IDF_PATH or IDF_PYTHON_ENV_PATH are not set."); //$NON-NLS-1$ | ||
| } | ||
| } |
There was a problem hiding this comment.
ESP-IDF version output is not captured in the report.
The showEspIdfVersion() method only logs the ESP-IDF version to the logger but doesn't append it to the product information file.
Modify the method to return the version string and append it to the StringBuilder:
-private void showEspIdfVersion()
+private String getEspIdfVersion()
{
if (IDFUtil.getIDFPath() != null && IDFUtil.getIDFPythonEnvPath() != null)
{
List<String> commands = new ArrayList<>();
commands.add(IDFUtil.getIDFPythonEnvPath());
commands.add(IDFUtil.getIDFPythonScriptFile().getAbsolutePath());
commands.add("--version"); //$NON-NLS-1$
Map<String, String> envMap = new IDFEnvironmentVariables().getSystemEnvMap();
- Logger.log(runCommand(commands, envMap));
+ String version = runCommand(commands, envMap);
+ Logger.log(version);
+ return version;
}
else
{
Logger.log("ESP-IDF version cannot be checked. IDF_PATH or IDF_PYTHON_ENV_PATH are not set."); //$NON-NLS-1$
+ return NOT_FOUND_MSG;
}
}And update line 399:
-showEspIdfVersion();
+sb.append("ESP-IDF Version: ").append(getEspIdfVersion());
+sb.append(System.lineSeparator());Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 408-423, showEspIdfVersion() currently only logs the version;
change it to return the version string instead: build the commands and env map
as before, call runCommand(...) and store its result in a String (or a
descriptive message if null/error), log it as now and then return that string;
adjust the method signature from void to String. Then at the call site around
line 399, capture the returned String and append it to the product information
StringBuilder (with a label/newline as appropriate) instead of relying solely on
Logger.log.
| IStatus status = processRunner.runInBackground(arguments, org.eclipse.core.runtime.Path.ROOT, env); | ||
| if (status == null) | ||
| { | ||
| Logger.log(IDFCorePlugin.getPlugin(), IDFCorePlugin.errorStatus("Status can't be null", null)); //$NON-NLS-1$ | ||
| } | ||
|
|
||
| // process export command output | ||
| exportCmdOp = status.getMessage(); | ||
| Logger.log(exportCmdOp); |
There was a problem hiding this comment.
Handle null status more gracefully.
The null check for status logs an error but then continues to call status.getMessage() on line 438, which will throw a NullPointerException.
Apply this fix:
IStatus status = processRunner.runInBackground(arguments, org.eclipse.core.runtime.Path.ROOT, env);
if (status == null)
{
Logger.log(IDFCorePlugin.getPlugin(), IDFCorePlugin.errorStatus("Status can't be null", null)); //$NON-NLS-1$
+ return exportCmdOp;
}
// process export command output
exportCmdOp = status.getMessage();📝 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.
| IStatus status = processRunner.runInBackground(arguments, org.eclipse.core.runtime.Path.ROOT, env); | |
| if (status == null) | |
| { | |
| Logger.log(IDFCorePlugin.getPlugin(), IDFCorePlugin.errorStatus("Status can't be null", null)); //$NON-NLS-1$ | |
| } | |
| // process export command output | |
| exportCmdOp = status.getMessage(); | |
| Logger.log(exportCmdOp); | |
| IStatus status = processRunner.runInBackground(arguments, org.eclipse.core.runtime.Path.ROOT, env); | |
| if (status == null) | |
| { | |
| Logger.log(IDFCorePlugin.getPlugin(), IDFCorePlugin.errorStatus("Status can't be null", null)); //$NON-NLS-1$ | |
| return exportCmdOp; | |
| } | |
| // process export command output | |
| exportCmdOp = status.getMessage(); | |
| Logger.log(exportCmdOp); |
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 431 to 439, the code logs an error when
processRunner.runInBackground returns null but then immediately calls
status.getMessage(), causing an NPE; update the control flow so that after
logging the error you either return/throw or assign a safe default and skip
calling status.getMessage() (for example set exportCmdOp to an empty string or
null and bypass further processing), ensuring no methods are invoked on a null
status and preserving existing logging of the failure.
@alirana01 do you think we could convert the path into a clickable link? just for comfortable navigation |
|
@alirana01 Windows logs: |
I can see some ambiguities in the logs probably from the same workspace as the other PR for the system environment and tools could be behaving differently please rerun and share logs from a clean and properly installed idf tools workspace |
|
@alirana01 please, take a look here I was using ESP-IDF from EIM installed. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (4)
400-406: Include ESP‑IDF version in product_information.txt (not just logs).Return the version string and append it to
sb.- showEspIdfVersion(); + sb.append("ESP-IDF Version: ").append(getEspIdfVersion()).append(System.lineSeparator());- private void showEspIdfVersion() + private String getEspIdfVersion() { if (IDFUtil.getIDFPath() != null && IDFUtil.getIDFPythonEnvPath() != null) { List<String> commands = new ArrayList<>(); commands.add(IDFUtil.getIDFPythonEnvPath()); commands.add(IDFUtil.getIDFPythonScriptFile().getAbsolutePath()); commands.add("--version"); //$NON-NLS-1$ Map<String, String> envMap = new IDFEnvironmentVariables().getSystemEnvMap(); - Logger.log(runCommand(commands, envMap)); + String version = runCommand(commands, envMap); + Logger.log(version); + return version; } else { Logger.log("ESP-IDF version cannot be checked. IDF_PATH or IDF_PYTHON_ENV_PATH are not set."); //$NON-NLS-1$ + return NOT_FOUND_MSG; } }Also applies to: 409-424
295-303: Optional: delete the temporary folder after zipping to avoid confusion.Users reported seeing both zipped and unzipped folders. Consider cleanup post-zip (or a preference).
// Zip the bug report directory FileUtil.zipDirectory(bugReportDirectory, bugReportDirectory.getAbsolutePath() + ".zip"); //$NON-NLS-1$ + // Optional: clean up the staging directory after zip + FileUtil.deleteDirectory(bugReportDirectory);
273-277: Remove unusedfilesToZiplist.The list is built but never used.
- List<File> filesToZip = new LinkedList<>(); - filesToZip.add(installedToolsFile); - filesToZip.add(productInfoFile); - filesToZip.add(basicSysInfoFile);
239-241: Optional: add.txtextension for basic system info file.Improves readability on all platforms.
- return FileUtil.createFileWithContentsInDirectory("basic_system_info", sb.toString(), //$NON-NLS-1$ + return FileUtil.createFileWithContentsInDirectory("basic_system_info.txt", sb.toString(), //$NON-NLS-1$ bugReportDirectory.getAbsolutePath());
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (8)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/IDFCorePlugin.java (1)
IDFCorePlugin(20-77)bundles/com.espressif.idf.core/src/com/espressif/idf/core/IDFCorePreferenceConstants.java (1)
IDFCorePreferenceConstants(24-79)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.core/src/com/espressif/idf/core/util/StringUtil.java (1)
StringUtil(11-25)
⏰ 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). (1)
- GitHub Check: build_windows
🔇 Additional comments (8)
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java (8)
138-192: Good: selective IDE metadata logs only.Filtering to
.log(active and recent backups) andversion.inimatches the privacy/perf ask.Please confirm this addresses the request to exclude plugin data and large metadata content.
118-123: Verify EIM log locations on macOS/Linux.Paths may differ across versions/distros; ensure logs are actually present in:
- macOS: ~/Library/Application Support/eim/logs
- Linux: ~/.local/share/eim/logs
Would you like me to add a fallback search (e.g., try both eim/.eim and common distro paths) if the primary path doesn’t exist?
Also applies to: 110-116
98-129: EIM logs path resolution: fix Windows var, Linux subdir, and avoid catastrophic copy on empty path.
- Windows must use LOCALAPPDATA, not APPDATA (Roaming). Also don’t append “\Local” twice.
- Linux path likely “~/.local/share/eim/logs” (not “.eim”).
- Return null when unresolved; returning
new File("")risks copying the CWD.- private File getEimLogPath() + private File getEimLogPath() { - String eimPath = StringUtil.EMPTY; + String eimPath = StringUtil.EMPTY; switch (Platform.getOS()) { case Platform.OS_WIN32: - eimPath = System.getenv("APPDATA"); //$NON-NLS-1$ - if (!StringUtil.isEmpty(eimPath)) - { - eimPath = eimPath + "\\Local\\eim\\logs"; //$NON-NLS-1$ - } + eimPath = System.getenv("LOCALAPPDATA"); //$NON-NLS-1$ + if (!StringUtil.isEmpty(eimPath)) { + eimPath = eimPath + "\\eim\\logs"; //$NON-NLS-1$ + } break; case Platform.OS_MACOSX: eimPath = System.getProperty("user.home"); //$NON-NLS-1$ if (!StringUtil.isEmpty(eimPath)) { eimPath = eimPath + "/Library/Application Support/eim/logs"; //$NON-NLS-1$ } break; case Platform.OS_LINUX: eimPath = System.getProperty("user.home"); //$NON-NLS-1$ if (!StringUtil.isEmpty(eimPath)) { - eimPath = eimPath + "/.local/share/.eim/logs"; //$NON-NLS-1$ + eimPath = eimPath + "/.local/share/eim/logs"; //$NON-NLS-1$ } break; default: break; } - - return new File(eimPath); + if (StringUtil.isEmpty(eimPath)) { + return null; + } + return new File(eimPath); }Please verify actual Linux/macOS EIM log locations across targets and adjust strings if needed.
206-208: Use correct OperatingSystemMXBean methods (compile-time fix).
getFreeMemorySize()/getTotalMemorySize()don’t exist. Use physical variants.- freePhys = osBean.getFreeMemorySize(); - totalPhys = osBean.getTotalMemorySize(); + freePhys = osBean.getFreePhysicalMemorySize(); + totalPhys = osBean.getTotalPhysicalMemorySize();
311-316: Fix Git path extraction and null-safety.
IPath.getDevice()returns only the drive, not path, and NPE ifgitPathis null.- 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 = gitPath != null ? gitPath.toOSString() : null; if (StringUtil.isEmpty(gitExecutablePath)) { gitExecutablePath = new IDFEnvironmentVariables().getEnvValue(IDFEnvironmentVariables.GIT_PATH); }
432-441: Avoid NPE when process status is null.You log the null but still dereference
status.- IStatus status = processRunner.runInBackground(arguments, org.eclipse.core.runtime.Path.ROOT, env); - if (status == null) - { - Logger.log(IDFCorePlugin.getPlugin(), IDFCorePlugin.errorStatus("Status can't be null", null)); //$NON-NLS-1$ - } - - // process export command output - exportCmdOp = status.getMessage(); - Logger.log(exportCmdOp); + IStatus status = processRunner.runInBackground(arguments, org.eclipse.core.runtime.Path.ROOT, env); + if (status == null) { + Logger.log(IDFCorePlugin.getPlugin(), IDFCorePlugin.errorStatus("Status can't be null", null)); //$NON-NLS-1$ + return exportCmdOp; + } + exportCmdOp = status.getMessage(); + Logger.log(exportCmdOp);
349-368: UseFile.pathSeparatorfor PATH to be cross‑platform.Hardcoding “;” breaks on Unix-like systems (uses “:”).
- if (!StringUtil.isEmpty(path1) && !path1.contains(gitDir)) // Git not found on the PATH environment - { - path1 = gitDir.concat(";").concat(path1); //$NON-NLS-1$ + if (!StringUtil.isEmpty(path1) && !path1.contains(gitDir)) // Git not found on the PATH environment + { + path1 = gitDir.concat(File.pathSeparator).concat(path1); environment.put("PATH", path1); //$NON-NLS-1$ } else if (!StringUtil.isEmpty(path2) && !path2.contains(gitDir)) // Git not found on the Path environment { - path2 = gitDir.concat(";").concat(path2); //$NON-NLS-1$ + path2 = gitDir.concat(File.pathSeparator).concat(path2); environment.put("Path", path2); //$NON-NLS-1$ }
290-294: Guard optional EIM logs copy and skip when unavailable.Avoid NPE and accidental massive copies when path is null/nonexistent.
- File eimLogPath = getEimLogPath(); - Logger.log("EIM log path: " + eimLogPath.getAbsolutePath()); //$NON-NLS-1$ - File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ - FileUtil.copyDirectory(eimLogPath, eimLogDir); + File eimLogPath = getEimLogPath(); + if (eimLogPath != null && eimLogPath.exists() && eimLogPath.isDirectory()) { + Logger.log("EIM log path: " + eimLogPath.getAbsolutePath()); //$NON-NLS-1$ + File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ + FileUtil.copyDirectory(eimLogPath, eimLogDir); + } else { + Logger.log("EIM log path not found. Skipping EIM logs copy."); //$NON-NLS-1$ + }
| filesToZip.add(basicSysInfoFile); | ||
|
|
||
| List<File> metadataLogsFile = getIdeMetadataLogsFile(); | ||
| File ideLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "ide_metadata_logs"); //$NON-NLS-1$ ) |
There was a problem hiding this comment.
Fix stray ) causing compilation errors.
There’s an extra closing parenthesis after the $NON-NLS-1$ comment.
- File ideLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "ide_metadata_logs"); //$NON-NLS-1$ )
+ File ideLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "ide_metadata_logs"); //$NON-NLS-1$- File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$ )
+ File eimLogDir = new File(bugReportDirectory.getAbsolutePath() + File.separator + "eim_logs"); //$NON-NLS-1$Also applies to: 292-292
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around lines 279 and 292, there are stray closing parentheses immediately after
the //$NON-NLS-1$ comments causing compilation errors; remove the extra ')' at
both locations so the File constructor calls end with ); (i.e., ensure the
comment is outside the parentheses and the method call is properly closed).
| 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()); |
There was a problem hiding this comment.
Remove logging of full environment map (PII/security risk).
Dumping env vars can leak tokens/credentials.
- Logger.log(environment.toString());📝 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.
| Logger.log(environment.toString()); |
🤖 Prompt for AI Agents
In
bundles/com.espressif.idf.core/src/com/espressif/idf/core/bug/BugReportGenerator.java
around line 323, remove the call that logs the full environment map
(Logger.log(environment.toString())) because it can expose PII/credentials;
either delete the log entirely or replace it with a safe alternative such as
logging only non-sensitive keys or a redacted summary (e.g., count of variables
or a whitelist of allowed keys), ensuring no environment values are printed.
This reverts commit a6383d9.
Description
The idea is to create a bug report accumulating all the things required into a compressed archive that user can then upload to the github. Please see if anyother information you think is required
This should also be workable with v4.00 when that branch is updated with master to also get the EIM logs, you will still get the EIM logs if you are using it in your system
The PR for v4.0 release is here (#1306)
Fixes # (IEP-1618)
Type of change
Please delete options that are not relevant.
How has this been tested?
Try to verify the logs file in the generated bug report by going to espressif menu and clicking generate bug report
Test Configuration:
Checklist
Summary by CodeRabbit
New Features
Documentation