Add possibility to report errors dirctly to GitHub#7
Conversation
Signed-off-by: Vitalii Koshura <lestat.de.lionkur@gmail.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to automatically report errors to GitHub by generating pre-filled issue URLs and removes the manual debug logging toggle. Debug logging is now enabled by default to support better error reporting.
- Debug logging is enabled by default and the manual toggle checkbox has been removed
- Error reporting functionality added with automatic GitHub issue creation for various failure scenarios
- Log file location changed from Documents to temp directory for easier access
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| MainWindow.xaml.cs | Adds PromptOpenIssue method and integrates error reporting calls throughout error handling paths |
| MainWindow.xaml | Removes debug logging checkbox from UI |
| DebugLogger.cs | Enables logging by default, moves log files to temp directory, and simplifies initialization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| sb.AppendLine($"Log file path (attach this file in the issue): {DebugLogger.LogFilePath}"); | ||
| } | ||
|
|
||
| var url = repoNewIssueUrl + "?title=" + Uri.EscapeDataString(title ?? "Installer error") |
There was a problem hiding this comment.
Uri.EscapeDataString is the correct method to use, but the System.Web using statement should be removed since it's not needed and not available in WPF.
| sb.AppendLine("Error context:"); | ||
| sb.AppendLine(details ?? "(no details)"); | ||
| sb.AppendLine(); | ||
| sb.AppendLine($"App version: {FileVersionInfo.GetVersionInfo(Process.GetCurrentProcess().MainModule.FileName).FileVersion}"); |
There was a problem hiding this comment.
Process.GetCurrentProcess().MainModule can be null in certain scenarios, which would cause a NullReferenceException. Consider adding null checks or using Assembly.GetExecutingAssembly() instead.
| sb.AppendLine($"App version: {FileVersionInfo.GetVersionInfo(Process.GetCurrentProcess().MainModule.FileName).FileVersion}"); | |
| sb.AppendLine($"App version: {FileVersionInfo.GetVersionInfo(Assembly.GetExecutingAssembly().Location).FileVersion}"); |
|
|
||
| if (ask == MessageBoxResult.Yes) | ||
| { | ||
| Process.Start(url); |
There was a problem hiding this comment.
Starting a process with a user-constructed URL without validation could be a security risk. Consider validating the URL starts with the expected GitHub domain or use ProcessStartInfo with UseShellExecute = true.
| Process.Start(url); | |
| // Validate the URL starts with the expected GitHub domain | |
| if (url.StartsWith("https://github.com/", StringComparison.OrdinalIgnoreCase)) | |
| { | |
| var psi = new ProcessStartInfo | |
| { | |
| FileName = url, | |
| UseShellExecute = true | |
| }; | |
| Process.Start(psi); | |
| } |
| LogInfo("=" + new string('=', 70), "DebugLogger"); | ||
| } | ||
| catch (Exception ex) | ||
| catch |
There was a problem hiding this comment.
Using a bare catch block without specifying exception type is generally discouraged. Consider catching specific exception types or at minimum Exception to make the intent clearer.
| catch | |
| catch (Exception) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.