Skip to content

Commit

Permalink
NBug: Include GE specific exception info (#9564)
Browse files Browse the repository at this point in the history
* Open bug report in GitHub could fail
  • Loading branch information
gerhardol committed Oct 5, 2021
1 parent 56ffa8a commit 60dfae4
Show file tree
Hide file tree
Showing 9 changed files with 61 additions and 26 deletions.
10 changes: 6 additions & 4 deletions BugReporter/BugReportForm.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ public partial class BugReportForm : Form, ITranslate
private static readonly GitHubUrlBuilder UrlBuilder;
private SerializableException? _lastException;
private Report? _lastReport;
private string _exceptionInfo;
private string? _environmentInfo;

static BugReportForm()
Expand Down Expand Up @@ -78,10 +79,11 @@ public BugReportForm()
mainTabs.TabPages.Remove(mainTabs.TabPages["reportContentsTabPage"]);
}

public DialogResult ShowDialog(IWin32Window? owner, SerializableException exception, string environmentInfo, bool canIgnore, bool showIgnore, bool focusDetails)
public DialogResult ShowDialog(IWin32Window? owner, SerializableException exception, string exceptionInfo, string environmentInfo, bool canIgnore, bool showIgnore, bool focusDetails)
{
_lastException = exception;
_lastReport = new Report(_lastException);
_exceptionInfo = exceptionInfo;
_environmentInfo = environmentInfo;

Validates.NotNull(_lastReport.GeneralInfo);
Expand Down Expand Up @@ -158,8 +160,8 @@ private void SendAndQuitButton_Click(object sender, EventArgs e)

Validates.NotNull(_lastException);

string? url = UrlBuilder.Build("https://github.com/gitextensions/gitextensions/issues/new", _lastException, _environmentInfo, descriptionTextBox.Text);
new Executable(url!).Start(useShellExecute: true);
string? url = UrlBuilder.Build("https://github.com/gitextensions/gitextensions/issues/new", _lastException, _exceptionInfo, _environmentInfo, descriptionTextBox.Text);
new Executable(url!).Start(useShellExecute: true, throwOnErrorExit: false);

DialogResult = DialogResult.Abort;
Close();
Expand All @@ -169,7 +171,7 @@ private void btnCopy_Click(object sender, EventArgs e)
{
Validates.NotNull(_lastException);

var report = ErrorReportBodyBuilder.Build(_lastException, _environmentInfo, descriptionTextBox.Text);
var report = ErrorReportBodyBuilder.Build(_lastException, _exceptionInfo, _environmentInfo, descriptionTextBox.Text);
if (string.IsNullOrWhiteSpace(report))
{
return;
Expand Down
10 changes: 8 additions & 2 deletions BugReporter/ErrorReportMarkDownBodyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ namespace BugReporter
{
public interface IErrorReportMarkDownBodyBuilder
{
string Build(SerializableException exception, string? environmentInfo, string? additionalInfo);
string Build(SerializableException exception, string exceptionInfo, string? environmentInfo, string? additionalInfo);
}

public sealed class ErrorReportMarkDownBodyBuilder : IErrorReportMarkDownBodyBuilder
{
public string Build(SerializableException exception, string? environmentInfo, string? additionalInfo)
public string Build(SerializableException exception, string exceptionInfo, string? environmentInfo, string? additionalInfo)
{
if (exception is null)
{
Expand Down Expand Up @@ -40,6 +40,12 @@ public string Build(SerializableException exception, string? environmentInfo, st
## Error Details");
if (!string.IsNullOrEmpty(exceptionInfo))
{
sb.AppendLine();
sb.AppendLine(exceptionInfo);
}

sb.AppendLine("```");
sb.AppendLine(exception.ToString());
sb.AppendLine("```");
Expand Down
12 changes: 9 additions & 3 deletions BugReporter/GitHubUrlBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,15 @@ public GitHubUrlBuilder(IErrorReportMarkDownBodyBuilder errorReportMarkDownBodyB

/// <summary>
/// Generates a URL to create a new issue on GitHub.
/// </summary>
/// <see href="https://help.github.com/en/articles/about-automation-for-issues-and-pull-requests-with-query-parameters"/>
public string? Build(string url, SerializableException exception, string? environmentInfo, string? additionalInfo)
/// </summary>
/// <param name="url">url to create a new issue for the project.</param>
/// <param name="exception">The exception to report.</param>
/// <param name="exceptionInfo">Info string for GE specific exceptions (like GitExtUtils.ExternalOperationException) unknown in BugReporter.</param>
/// <param name="environmentInfo">Git version, directory etc.</param>
/// <param name="additionalInfo">Information from the popup textbox.</param>
/// <returns>The URL as a string</returns>
public string? Build(string url, SerializableException exception, string exceptionInfo, string? environmentInfo, string? additionalInfo)
{
if (string.IsNullOrWhiteSpace(url) || exception is null)
{
Expand All @@ -44,7 +50,7 @@ public GitHubUrlBuilder(IErrorReportMarkDownBodyBuilder errorReportMarkDownBodyB
subject = subject.Substring(0, 66) + "...";
}

string body = Uri.EscapeDataString(_errorReportMarkDownBodyBuilder.Build(exception, environmentInfo, additionalInfo));
string body = Uri.EscapeDataString(_errorReportMarkDownBodyBuilder.Build(exception, exceptionInfo, environmentInfo, additionalInfo));

return $"{validatedUri}{separator}title={Uri.EscapeDataString(subject)}&body={body}";
}
Expand Down
4 changes: 3 additions & 1 deletion BugReporter/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ private static void Main()
}
}

// No specific info extracted from these exceptions
string exceptionInfo = "";
exception ??= new(new Exception("Missing error payload"));

new BugReportForm().ShowDialog(owner: null, exception, UserEnvironmentInformation.GetInformation(), canIgnore: true, showIgnore: false, focusDetails: false);
new BugReportForm().ShowDialog(owner: null, exception, exceptionInfo, UserEnvironmentInformation.GetInformation(), canIgnore: true, showIgnore: false, focusDetails: false);
}

private static string Base64Decode(string base64EncodedData)
Expand Down
1 change: 1 addition & 0 deletions GitCommands/Settings/AppSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1945,6 +1945,7 @@ public static bool LogCaptureCallStacks
// Return false whilst we're in the designer.
public static bool IsPortable() => !IsDesignMode && Properties.Settings.Default.IsPortable;

// Set manually in settings file
public static bool WriteErrorLog
{
get => GetBool("WriteErrorLog", false);
Expand Down
4 changes: 4 additions & 0 deletions GitUI/BranchTreePanel/RepoObjectsTree.Nodes.Submodules.cs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,10 @@ private void LoadNodeToolTips(Nodes loadedNodes, CancellationToken token)
catch (OperationCanceledException)
{
}
//// Comment out to debug BugReporter
////catch (GitExtUtils.ExternalOperationException)
////{
////}
});
#pragma warning restore VSTHRD101 // Avoid unsupported async delegates
}
Expand Down
29 changes: 21 additions & 8 deletions GitUI/NBugReports/BugReportInvoker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,11 @@ private static IntPtr OwnerFormHandle
=> OwnerForm?.Handle ?? IntPtr.Zero;

/// <summary>
/// Appends the exception data and gets the root error.
/// Gets the root error.
/// </summary>
/// <param name="text">A StringBuilder to which the exception data is appended.</param>
/// <param name="exception">An Exception to describe.</param>
/// <returns>The inner-most exception message.</returns>
internal static string Append(StringBuilder text, Exception exception)
internal static string GetRootError(Exception exception)
{
string rootError = exception.Message;
for (Exception innerException = exception.InnerException; innerException is not null; innerException = innerException.InnerException)
Expand All @@ -37,6 +36,17 @@ internal static string Append(StringBuilder text, Exception exception)
}
}

return rootError;
}

/// <summary>
/// Get the exception data.
/// </summary>
/// <param name="exception">An Exception to describe.</param>
internal static StringBuilder GetExceptionInfo(Exception exception)
{
StringBuilder text = new();

if (exception is UserExternalOperationException userExternalOperationException && !string.IsNullOrWhiteSpace(userExternalOperationException.Context))
{
// Context contains an error message as UserExternalOperationException is currently used. So append just "<context>"
Expand All @@ -46,7 +56,7 @@ internal static string Append(StringBuilder text, Exception exception)
if (exception is ExternalOperationException externalOperationException)
{
// Exit code: <n>
AppendIfNotEmpty($"{externalOperationException.ExitCode}{Environment.NewLine}", TranslatedStrings.ExitCode);
AppendIfNotEmpty(externalOperationException.ExitCode.ToString(), TranslatedStrings.ExitCode);

// Command: <command>
AppendIfNotEmpty(externalOperationException.Command, TranslatedStrings.Command);
Expand All @@ -58,7 +68,7 @@ internal static string Append(StringBuilder text, Exception exception)
AppendIfNotEmpty(externalOperationException.WorkingDirectory, TranslatedStrings.WorkingDirectory);
}

return rootError;
return text;

void AppendIfNotEmpty(string? value, string designation)
{
Expand Down Expand Up @@ -96,6 +106,7 @@ public static void Report(Exception exception, bool isTerminating)
if (isTerminating)
{
// TODO: this is not very efficient
// GetExceptionInfo() info is lost
SerializableException serializableException = new(exception);
string xml = serializableException.ToXmlString();
string encoded = Base64Encode(xml);
Expand All @@ -113,8 +124,8 @@ or DirectoryNotFoundException
or PathTooLongException
or Win32Exception;

StringBuilder text = new();
string rootError = Append(text, exception);
StringBuilder text = GetExceptionInfo(exception);
string rootError = GetRootError(exception);

TaskDialogPage page = new()
{
Expand Down Expand Up @@ -167,7 +178,9 @@ void AddIgnoreOrCloseButton()
private static void ShowNBug(IWin32Window? owner, Exception exception, bool isExternalOperation, bool isTerminating)
{
using BugReportForm form = new();
DialogResult result = form.ShowDialog(owner, new SerializableException(exception),
DialogResult result = form.ShowDialog(owner,
new SerializableException(exception),
GetExceptionInfo(exception).ToString(),
UserEnvironmentInformation.GetInformation(),
canIgnore: !isTerminating,
showIgnore: isExternalOperation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,27 +75,28 @@ public void Should_show_load_exception_info_correctly()
Assert.AreEqual(listView.Items.Count - 1, index);
},
exception,
null);
exceptionInfo: "",
environmentInfo: "");
}

private void RunFormTest(Action<BugReportForm> testDriver, SerializableException exception, string environmentInfo)
private void RunFormTest(Action<BugReportForm> testDriver, SerializableException exception, string exceptionInfo, string environmentInfo)
{
RunFormTest(
form =>
{
testDriver(form);
return Task.CompletedTask;
},
exception, environmentInfo,
exception, exceptionInfo, environmentInfo,
expected: DialogResult.Cancel);
}

private void RunFormTest(Func<BugReportForm, Task> testDriverAsync, SerializableException exception, string environmentInfo, DialogResult expected)
private void RunFormTest(Func<BugReportForm, Task> testDriverAsync, SerializableException exception, string exceptionInfo, string environmentInfo, DialogResult expected)
{
UITest.RunForm(
() =>
{
Assert.AreEqual(expected, _form.ShowDialog(owner: null, exception, environmentInfo, canIgnore: true, showIgnore: false, focusDetails: false));
Assert.AreEqual(expected, _form.ShowDialog(owner: null, exception, exceptionInfo, environmentInfo, canIgnore: true, showIgnore: false, focusDetails: false));
},
testDriverAsync);
}
Expand Down
6 changes: 3 additions & 3 deletions UnitTests/GitUI.Tests/NBugReports/BugReportInvokerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ public sealed class BugReportInvokerTests
[Test, TestCaseSource(typeof(TestExceptions), "TestCases")]
public void Append(Exception exception, string expectedRootError, string expectedText)
{
StringBuilder text = new();
string rootError = BugReportInvoker.Append(text, exception);
StringBuilder text = BugReportInvoker.GetExceptionInfo(exception);
string rootError = BugReportInvoker.GetRootError(exception);
rootError.Should().Be(expectedRootError);
text.ToString().Should().Be(expectedText);
}
Expand Down Expand Up @@ -49,7 +49,7 @@ public static IEnumerable TestCases
new ExternalOperationException(_command, _arguments, _directory, _exitCode, new Exception(_messageOuter, new Exception(_messageInner)))),
_messageInner,
$"{_context}{Environment.NewLine}"
+ $"Exit code: {_exitCode}{Environment.NewLine}{Environment.NewLine}"
+ $"Exit code: {_exitCode}{Environment.NewLine}"
+ $"Command: {_command}{Environment.NewLine}"
+ $"Arguments: {_arguments}{Environment.NewLine}"
+ $"Working directory: {_directory}{Environment.NewLine}");
Expand Down

0 comments on commit 60dfae4

Please sign in to comment.