Improve ChobbyLauncher crash reporting

ChobbyLauncher/ChobbylaLocalListener.cs

@@ -656,7 +656,7 @@ private async Task Process(StartNewSpring args)
                             StartScriptContent = args.StartScriptContent
                         });
 
-                        CrashReportHelper.CheckAndReportErrors(logs.ToString(), isOk, "Externally launched spring crashed with code " + process.ExitCode, null, args.Engine);
+                        CrashReportHelper.CheckAndReportErrors(logs.ToString(), isOk, paths, "Externally launched spring crashed with code " + process.ExitCode, null, args.Engine);
                     };
                     process.EnableRaisingEvents = true;
                     process.Start();

ChobbyLauncher/CrashReportHelper.cs

@@ -1,6 +1,8 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.IO;
+using System.IO.Compression;
 using System.Linq;
 using System.Text;
 using System.Text.RegularExpressions;
@@ -26,6 +28,8 @@ public static class CrashReportHelper
         private const string CrashReportsRepoName = "CrashReports";
 
         private const int MaxInfologSize = 62000;
+        private const int IssuesPerRelease = 250;
+
         private const string InfoLogLineStartPattern = @"(^\[t=\d+:\d+:\d+\.\d+\]\[f=-?\d+\] )";
         private const string InfoLogLineEndPattern = @"(\r?\n|\Z)";
         private sealed class GameFromLog
@@ -153,6 +157,81 @@ public void AddGameIDs(IEnumerable<(int, string)> gameIDs)
 
             return result;
         }
+        //See https://github.com/beyond-all-reason/spring/blob/f3ba23635e1462ae2084f10bf9ba777467d16090/rts/System/Sync/DumpState.cpp#L155
+        private static readonly Regex GameStateFileRegex = new Regex(@"\A(Server|Client)GameState--?\d+-\[-?\d+--?\d+\]\.txt\z", RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.ExplicitCapture, TimeSpan.FromSeconds(30));
+
+        private static (Stream, List<string>) CreateZipArchiveFromFiles(string writableDirectory, string[] fileNames, (string, string)[] extraFiles)
+        {
+            var outStream = new MemoryStream();
+            var fullPathWritableDirectory = Path.GetFullPath(writableDirectory + Path.DirectorySeparatorChar.ToString());
+            var archiveManifest = new List<string>(fileNames.Length);
+            try
+            {
+                using (var archive = new ZipArchive(outStream, ZipArchiveMode.Create, leaveOpen: true))
+                {
+                    foreach (
+                        var fileName in
+                            fileNames
+                                .Select(f => Path.GetFullPath(Path.Combine(writableDirectory, f)))
+                                .Distinct(StringComparer.OrdinalIgnoreCase))
+                    {
+                        if (!fileName.StartsWith(fullPathWritableDirectory, StringComparison.Ordinal))
+                        {
+                            //Only upload files that are in WritableDirectory.
+                            //This avoids inadvertent directory traversal, which could
+                            //upload private data from the player's computer.
+                            Trace.TraceWarning("[CrashReportHelper] Tried to upload file that is not in WritableDirectory: {0}", fileName);
+                            continue;
+                        }
+                        var relativePath = fileName.Remove(0, fullPathWritableDirectory.Length);
+                        if (!GameStateFileRegex.IsMatch(relativePath))
+                        {
+                            //Only upload files that we expect to upload. Currently, we only upload GameState files.
+                            Trace.TraceWarning("[CrashReportHelper] Tried to upload unexpected file: {0}", relativePath);
+                            continue;
+                        }
+                        var entryPath = Path.Combine("zk", relativePath);
+                        var entry = archive.CreateEntry(entryPath, CompressionLevel.Optimal);
+                        FileStream fsPre;
+                        try
+                        {
+                            fsPre = new FileStream(fileName, System.IO.FileMode.Open, FileAccess.Read);
+                        }
+                        catch
+                        {
+                            Trace.TraceWarning("[CrashReportHelper] Could not read file to add to archive: {0}", relativePath);
+                            continue;
+                        }
+                        //Errors from here onwards could corrupt the ZipArchive; so do not continue.
+                        using (var fs = fsPre)
+                        using (var entryStream = entry.Open())
+                        {
+                            fs.CopyTo(entryStream);
+                        }
+                        archiveManifest.Add(entryPath);
+                    }
+                    foreach (var extra in extraFiles)
+                    {
+                        var entryPath = Path.Combine("ex", extra.Item1);
+                        var entry = archive.CreateEntry(entryPath, CompressionLevel.Optimal);
+                        using (var ms = new MemoryStream(Encoding.UTF8.GetBytes(extra.Item2)))
+                        using (var entryStream = entry.Open())
+                        {
+                            ms.CopyTo(entryStream);
+                        }
+                        archiveManifest.Add(entryPath);
+                    }
+                }
+                outStream.Position = 0;
+                return (archiveManifest.Count != 0 ? outStream : null, archiveManifest);
+            }
+            catch (Exception ex)
+            {
+                Trace.TraceWarning("[CrashReportHelper] Could not create archive: {0}", ex);
+                outStream.Dispose();
+                return (null, null);
+            }
+        }
 
         private static string EscapeMarkdownTableCell(string str) => str.Replace("\r", "").Replace("\n", " ").Replace("|", @"\|");
         private static string MakeDesyncGameTable(GameFromLogCollection gamesFromLog)
@@ -173,8 +252,51 @@ private static string MakeDesyncGameTable(GameFromLogCollection gamesFromLog)
             }
             return tableEmpty ? string.Empty : sb.ToString();
         }
+        private static string MakeGameStateLabel(string gameID) => $"GameStateFor-{gameID}";
+
+        private static string MakeDesyncIssueLinksTable(IEnumerable<string> gamesWithGameStateFiles, IEnumerable<IEnumerable<int>> existingIssues)
+        {
+            var hasLinkedIssue = false;
+            var sb = new StringBuilder();
+            sb.AppendLine("\n\nIssues with GameStates for same Game(s):\n\n|GameID|Issue|");
+            sb.AppendLine("|-|-|");
+            foreach (var game in gamesWithGameStateFiles.Zip(existingIssues, (gameID, issues) => new { gameID, issues }))
+            {
+                var gameIDString = EscapeMarkdownTableCell(game.gameID);
+                foreach (var issueNumber in game.issues)
+                {
+                    hasLinkedIssue = true;
+                    sb.AppendLine($"|{gameIDString}|#{issueNumber}|");
+                }
+            }
+            return hasLinkedIssue ? sb.ToString() : string.Empty;
+        }
+        private static void FillIssueLabels(System.Collections.ObjectModel.Collection<string> labels, string[] gamesWithGameStateFiles, bool hasLinkedIssue)
+        {
+            labels.Add("HasDesyncGameState");
+            if (hasLinkedIssue)
+            {
+                labels.Add("HasLinkedDesyncIssue");
+            }
+            foreach (var gameID in gamesWithGameStateFiles)
+            {
+                labels.Add(MakeGameStateLabel(gameID));
+            }
+        }
 
-        private static async Task<Issue> ReportCrash(string infolog, CrashType type, string engine, string bugReportTitle, string bugReportDescription, GameFromLogCollection gamesFromLog)
+        private static string MakeArchiveManifestTable(IEnumerable<string> archiveManifest)
+        {
+            var sb = new StringBuilder();
+            sb.AppendLine("\n\n|Contents|");
+            sb.AppendLine("|-|");
+            foreach (var f in archiveManifest)
+            {
+                sb.AppendLine($"|{EscapeMarkdownTableCell(f)}|");
+            }
+            return sb.ToString();
+        }
+
+        private static async Task<Issue> ReportCrash(string infolog, CrashType type, string engine, SpringPaths paths, string bugReportTitle, string bugReportDescription, GameFromLogCollection gamesFromLog)
         {
             try
             {
@@ -185,16 +307,71 @@ private static async Task<Issue> ReportCrash(string infolog, CrashType type, str
 
                 var infologTruncated = TextTruncator.Truncate(infolog, MaxInfologSize, MakeRegionsOfInterest(infolog.Length, gamesFromLog.Games.Where(g => g.HasDesync).Select(g => g.FirstDesyncIdxInLog.Value), gamesFromLog.AsGameStartReadOnlyList()));
 
+                var gamesWithGameStateFiles = gamesFromLog.Games.Where(g => g.HasDesync && g.GameStateFileNames != null && g.GameStateFileNames.Count > 0 && g.GameID != null).Select(g => g.GameID).Distinct().ToArray();
+
+
+                var existingIssues = new List<int[]>(gamesWithGameStateFiles.Length);
+                foreach (var gameID in gamesWithGameStateFiles)
+                {
+                    var rir = new RepositoryIssueRequest
+                    {
+                        Filter = IssueFilter.All,
+                        State = ItemStateFilter.All
+                    };
+                    rir.Labels.Add(MakeGameStateLabel(gameID));
+
+                    var issues = await client.Issue.GetAllForRepository(CrashReportsRepoOwner, CrashReportsRepoName, rir);
+
+                    existingIssues.Add(issues.Select(i => i.Number).ToArray());
+                }
+
+                var desyncIssueLinks = MakeDesyncIssueLinksTable(gamesWithGameStateFiles, existingIssues);
                 var desyncDebugInfo = MakeDesyncGameTable(gamesFromLog);
 
                 var newIssueRequest = new NewIssue($"Spring {type} [{engine}] {bugReportTitle}")
                 {
-                    Body = $"{bugReportDescription}{desyncDebugInfo}"
+                    Body = $"{bugReportDescription}{desyncDebugInfo}{desyncIssueLinks}"
                 };
+                FillIssueLabels(newIssueRequest.Labels, gamesWithGameStateFiles, hasLinkedIssue: desyncIssueLinks.Length != 0);
+
                 var createdIssue = await client.Issue.Create(CrashReportsRepoOwner, CrashReportsRepoName, newIssueRequest);
 
                 await client.Issue.Comment.Create(CrashReportsRepoOwner, CrashReportsRepoName, createdIssue.Number, $"infolog_full.txt (truncated):\n\n```{infologTruncated}```");
 
+
+                var releaseNumber = (createdIssue.Number - 1) / IssuesPerRelease;
+                var issueRangeString = $"{releaseNumber * IssuesPerRelease + 1}-{(releaseNumber + 1) * IssuesPerRelease}";
+
+                var releaseName = $"FilesForIssues-{issueRangeString}";
+                Release releaseForUpload;
+                try
+                {
+                    releaseForUpload = await client.Repository.Release.Create(CrashReportsRepoOwner, CrashReportsRepoName, new NewRelease(releaseName) { TargetCommitish = "main", Prerelease = true, Name = $"Files for Issues {issueRangeString}", Body = $"Files for Issues {issueRangeString}" });
+                }
+                catch (ApiValidationException ex)
+                {
+                    if (!(ex.ApiError.Errors.Count == 1 && ex.ApiError.Errors[0].Code == "already_exists")) throw;
+                    //Release already exists
+                    releaseForUpload = await client.Repository.Release.Get(CrashReportsRepoOwner, CrashReportsRepoName, releaseName);
+                }
+
+                var zar =
+                    CreateZipArchiveFromFiles(
+                        paths.WritableDirectory,
+                        gamesFromLog.Games.SelectMany(g => g.GameStateFileNames ?? Enumerable.Empty<string>()).ToArray(),
+                        new[] { ("infolog_full.txt", infolog) });
+
+                if (zar.Item1 != null)
+                {
+                    using (var zipArchive = zar.Item1)
+                    {
+                        var upload = await client.Repository.Release.UploadAsset(releaseForUpload, new ReleaseAssetUpload($"FilesForIssue-{createdIssue.Number}.zip", "application/zip", zipArchive, timeout: null));
+
+                        var archiveManifestTable = MakeArchiveManifestTable(zar.Item2);
+                        var comment = await client.Issue.Comment.Create(CrashReportsRepoOwner, CrashReportsRepoName, createdIssue.Number, $"See {upload.BrowserDownloadUrl}{archiveManifestTable}");
+                    }
+                }
+
                 return createdIssue;
             }
             catch (Exception ex)
@@ -241,7 +418,7 @@ private static (int, string)[] ReadGameStateFileNames(string logStr)
                     Regex
                         .Matches(
                             logStr,
-                            $@"(?<={InfoLogLineStartPattern})\[DumpState\] using dump-file ""(?<d>[^{Regex.Escape(System.IO.Path.DirectorySeparatorChar.ToString())}{Regex.Escape(System.IO.Path.AltDirectorySeparatorChar.ToString())}""]+)""{InfoLogLineEndPattern}",
+                            $@"(?<={InfoLogLineStartPattern})\[DumpState\] using dump-file ""(?<d>[^{Regex.Escape(Path.DirectorySeparatorChar.ToString())}{Regex.Escape(Path.AltDirectorySeparatorChar.ToString())}""]+)""{InfoLogLineEndPattern}",
                             RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.ExplicitCapture | RegexOptions.Multiline,
                             TimeSpan.FromSeconds(30))
                         .Cast<Match>().Select(m => (m.Index, m.Groups["d"].Value)).Distinct()
@@ -301,7 +478,7 @@ private static (int, string)[] ReadGameIDs(string logStr)
             }
         }
 
-        public static void CheckAndReportErrors(string logStr, bool springRunOk, string bugReportTitle, string bugReportDescription, string engineVersion)
+        public static void CheckAndReportErrors(string logStr, bool springRunOk, SpringPaths paths, string bugReportTitle, string bugReportDescription, string engineVersion)
         {
             var gamesFromLog = new GameFromLogCollection(ReadGameReloads(logStr));
 
@@ -364,6 +541,7 @@ public static void CheckAndReportErrors(string logStr, bool springRunOk, string
                             logStr,
                             crashType,
                             engineVersion,
+                            paths,
                             bugReportTitle,
                             bugReportDescription,
                             gamesFromLog)

ChobbyLauncher/Program.cs

@@ -168,7 +168,7 @@ private static void RunWrapper(Chobbyla chobbyla, ulong connectLobbyID, TextWrit
             logWriter.Flush();
             var logStr = logSb.ToString();
 
-            CrashReportHelper.CheckAndReportErrors(logStr, springRunOk, chobbyla.BugReportTitle, chobbyla.BugReportDescription, chobbyla.engine);
+            CrashReportHelper.CheckAndReportErrors(logStr, springRunOk, chobbyla.paths, chobbyla.BugReportTitle, chobbyla.BugReportDescription, chobbyla.engine);
         }
 
         static async Task<bool> PrepareWithoutGui(Chobbyla chobbyla)