Skip to content

Commit 4108308

Browse files
GabrielDufGabrielDuf
authored
Keep Scoop package listings working when scoop commands fail (#5321)
1 parent fd81227 commit 4108308

7 files changed

Lines changed: 280 additions & 62 deletions

File tree

‎src/UniGetUI.PackageEngine.Managers.Scoop/Helpers/ScoopPkgDetailsHelper.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,10 @@ protected override void GetDetails_UnSafe(IPackageDetails details)
7272
);
7373

7474
p.Start();
75+
Task<string> stdErr = ScoopProcess.ReadStdErrAsync(p);
7576
string JsonString = p.StandardOutput.ReadToEnd();
7677
logger.AddToStdOut(JsonString);
77-
logger.AddToStdErr(p.StandardError.ReadToEnd());
78+
logger.AddToStdErr(stdErr.GetAwaiter().GetResult());
7879

7980
if (JsonNode.Parse(JsonString) is not JsonObject contents)
8081
{

‎src/UniGetUI.PackageEngine.Managers.Scoop/Helpers/ScoopProcess.cs‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
using System.Diagnostics;
2+
using UniGetUI.Core.Logging;
3+
using UniGetUI.PackageEngine.ManagerClasses.Classes;
4+
5+
namespace UniGetUI.PackageEngine.Managers.ScoopManager
6+
{
7+
internal static class ScoopProcess
8+
{
9+
public static IReadOnlyList<string> ReadLines(Process p, IProcessTaskLogger logger)
10+
{
11+
Task<string> stdErr = ReadStdErrAsync(p);
12+
13+
List<string> lines = [];
14+
string? line;
15+
while ((line = p.StandardOutput.ReadLine()) is not null)
16+
{
17+
logger.AddToStdOut(line);
18+
lines.Add(line);
19+
}
20+
21+
Finish(p, logger, stdErr);
22+
return lines;
23+
}
24+
25+
public static string ReadToEnd(Process p, IProcessTaskLogger logger)
26+
{
27+
Task<string> stdErr = ReadStdErrAsync(p);
28+
29+
string stdOut = p.StandardOutput.ReadToEnd();
30+
logger.AddToStdOut(stdOut);
31+
32+
Finish(p, logger, stdErr);
33+
return stdOut;
34+
}
35+
36+
public static Task<string> ReadStdErrAsync(Process p) =>
37+
Task.Run(p.StandardError.ReadToEnd);
38+
39+
public static Task<string> ReadStdOutAsync(Process p) =>
40+
Task.Run(p.StandardOutput.ReadToEnd);
41+
42+
public static string ReadWithTimeout(Task<string> read, int millisecondsTimeout)
43+
{
44+
try
45+
{
46+
return read.Wait(millisecondsTimeout) ? read.Result.Trim() : "";
47+
}
48+
catch (Exception ex)
49+
{
50+
Logger.Warn($"Could not read the output of a Scoop process: {ex.Message}");
51+
return "";
52+
}
53+
}
54+
55+
private static void Finish(Process p, IProcessTaskLogger logger, Task<string> stdErr)
56+
{
57+
logger.AddToStdErr(stdErr.GetAwaiter().GetResult());
58+
p.WaitForExit();
59+
logger.Close(p.ExitCode);
60+
}
61+
}
62+
}

‎src/UniGetUI.PackageEngine.Managers.Scoop/Helpers/ScoopSourceHelper.cs‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,9 @@ protected override IReadOnlyList<IManagerSource> GetSources_UnSafe()
6666
);
6767

6868
p.Start();
69+
p.StandardInput.Close();
6970

70-
List<string> lines = [];
71-
string? line;
72-
while ((line = p.StandardOutput.ReadLine()) is not null)
73-
{
74-
logger.AddToStdOut(line);
75-
lines.Add(line);
76-
}
77-
logger.AddToStdErr(p.StandardError.ReadToEnd());
78-
p.WaitForExit();
79-
logger.Close(p.ExitCode);
80-
81-
return ParseSources(lines);
71+
return ParseSources(ScoopProcess.ReadLines(p, logger));
8272
}
8373

8474
internal IReadOnlyList<IManagerSource> ParseSources(IEnumerable<string> lines)

‎src/UniGetUI.PackageEngine.Managers.Scoop/Scoop.cs‎

Lines changed: 44 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ public class Scoop : PackageManager
3333
"removed,",
3434
];
3535

36+
private const int VersionProbeTimeout = 20_000;
37+
private const int StreamDrainTimeout = 5_000;
38+
3639
private long LastScoopSourceUpdateTime;
3740

3841
public Scoop()
@@ -391,10 +394,7 @@ protected override IReadOnlyList<Package> FindPackages_UnSafe(string query)
391394
proc
392395
);
393396
proc.Start();
394-
aux_logger.AddToStdOut(proc.StandardOutput.ReadToEnd());
395-
aux_logger.AddToStdErr(proc.StandardError.ReadToEnd());
396-
proc.WaitForExit();
397-
aux_logger.Close(proc.ExitCode);
397+
ScoopProcess.ReadToEnd(proc, aux_logger);
398398
path = "scoop-search.exe";
399399
}
400400

@@ -415,23 +415,14 @@ protected override IReadOnlyList<Package> FindPackages_UnSafe(string query)
415415
IProcessTaskLogger logger = TaskLogger.CreateNew(LoggableTaskType.FindPackages, p);
416416

417417
p.Start();
418+
RegisterListingProcess(p);
418419

419-
List<string> lines = [];
420-
string? line;
421-
while ((line = p.StandardOutput.ReadLine()) is not null)
422-
{
423-
logger.AddToStdOut(line);
424-
lines.Add(line);
425-
}
426-
logger.AddToStdErr(p.StandardError.ReadToEnd());
427-
p.WaitForExit();
428-
logger.Close(p.ExitCode);
429-
return ParseSearchOutput(lines);
420+
return ParseSearchOutput(ScoopProcess.ReadLines(p, logger));
430421
}
431422

432423
protected override IReadOnlyList<Package> GetAvailableUpdates_UnSafe()
433424
{
434-
IReadOnlyList<IPackage> installedPackages = GetInstalledPackages();
425+
IReadOnlyList<IPackage> installedPackages = GetInstalledPackages_UnSafe();
435426

436427
using Process p = new()
437428
{
@@ -449,18 +440,9 @@ protected override IReadOnlyList<Package> GetAvailableUpdates_UnSafe()
449440
IProcessTaskLogger logger = TaskLogger.CreateNew(LoggableTaskType.ListUpdates, p);
450441

451442
p.Start();
443+
RegisterListingProcess(p);
452444

453-
List<string> lines = [];
454-
string? line;
455-
while ((line = p.StandardOutput.ReadLine()) is not null)
456-
{
457-
logger.AddToStdOut(line);
458-
lines.Add(line);
459-
}
460-
logger.AddToStdErr(p.StandardError.ReadToEnd());
461-
p.WaitForExit();
462-
logger.Close(p.ExitCode);
463-
return ParseAvailableUpdates(lines, installedPackages);
445+
return ParseAvailableUpdates(ScoopProcess.ReadLines(p, logger), installedPackages);
464446
}
465447

466448
protected override IReadOnlyList<Package> GetInstalledPackages_UnSafe() =>
@@ -487,17 +469,7 @@ private IReadOnlyList<Package> _getInstalledPackages_UnSafe()
487469
);
488470
p.Start();
489471

490-
List<string> lines = [];
491-
string? line;
492-
while ((line = p.StandardOutput.ReadLine()) is not null)
493-
{
494-
logger.AddToStdOut(line);
495-
lines.Add(line);
496-
}
497-
logger.AddToStdErr(p.StandardError.ReadToEnd());
498-
p.WaitForExit();
499-
logger.Close(p.ExitCode);
500-
return ParseInstalledPackages(lines);
472+
return ParseInstalledPackages(ScoopProcess.ReadLines(p, logger));
501473
}
502474

503475
public override void RefreshPackageIndexes()
@@ -525,10 +497,10 @@ public override void RefreshPackageIndexes()
525497
p.StartInfo = StartInfo;
526498
IProcessTaskLogger logger = TaskLogger.CreateNew(LoggableTaskType.RefreshIndexes, p);
527499
p.Start();
528-
logger.AddToStdOut(p.StandardOutput.ReadToEnd());
529-
logger.AddToStdErr(p.StandardError.ReadToEnd());
530-
p.WaitForExit();
531-
logger.Close(p.ExitCode);
500+
RegisterListingProcess(p);
501+
p.StandardInput.Close();
502+
503+
ScoopProcess.ReadToEnd(p, logger);
532504
}
533505

534506
public override IReadOnlyList<string> FindCandidateExecutableFiles() =>
@@ -567,7 +539,36 @@ protected override void _loadManagerVersion(out string version)
567539
},
568540
};
569541
process.Start();
570-
version = process.StandardOutput.ReadToEnd().Trim();
542+
Task<string> stdOut = ScoopProcess.ReadStdOutAsync(process);
543+
Task<string> stdErr = ScoopProcess.ReadStdErrAsync(process);
544+
545+
if (!process.WaitForExit(VersionProbeTimeout))
546+
{
547+
Logger.Warn(
548+
$"\"scoop --version\" did not finish after {VersionProbeTimeout / 1000} seconds, "
549+
+ "it will be killed and Scoop will be loaded with an unknown version"
550+
);
551+
try
552+
{
553+
process.Kill(entireProcessTree: true);
554+
}
555+
catch (Exception ex)
556+
{
557+
Logger.Warn($"Could not kill the timed-out scoop version process: {ex.Message}");
558+
}
559+
}
560+
561+
string errors = ScoopProcess.ReadWithTimeout(stdErr, StreamDrainTimeout);
562+
if (errors.Length > 0)
563+
{
564+
Logger.Warn($"\"scoop --version\" reported errors: {errors}");
565+
}
566+
567+
version = ScoopProcess.ReadWithTimeout(stdOut, StreamDrainTimeout);
568+
if (version.Length == 0)
569+
{
570+
version = CoreTools.Translate("Unknown");
571+
}
571572
}
572573

573574
protected override void _performExtraLoadingSteps()

‎src/UniGetUI.PackageEngine.PackageManagerClasses/Manager/PackageManager.cs‎

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,38 @@ private static void KillListingProcesses(List<Process> processes)
301301
}
302302
}
303303

304-
private T RunListingTaskWithTimeout<T>(Func<T> method, string taskName)
304+
private void RefreshPackageIndexesSafely()
305+
{
306+
try
307+
{
308+
RunListingTaskWithTimeout<object?>(
309+
() =>
310+
{
311+
RefreshPackageIndexes();
312+
return null;
313+
},
314+
"RefreshPackageIndexes",
315+
allowDisablingTimeout: false
316+
);
317+
}
318+
catch (Exception e)
319+
{
320+
while (e is AggregateException)
321+
e = e.InnerException ?? new InvalidOperationException("How did we get here?");
322+
323+
Logger.Warn(
324+
$"Manager {DisplayName} could not refresh its package indexes "
325+
+ $"({e.GetType().Name}: {e.Message}). The available updates will be listed "
326+
+ "with the indexes as they are, which may result in an incomplete list."
327+
);
328+
}
329+
}
330+
331+
private T RunListingTaskWithTimeout<T>(
332+
Func<T> method,
333+
string taskName,
334+
bool allowDisablingTimeout = true
335+
)
305336
{
306337
List<Process> processes = [];
307338
var task = Task.Run(() =>
@@ -312,14 +343,21 @@ private T RunListingTaskWithTimeout<T>(Func<T> method, string taskName)
312343

313344
if (!task.Wait(TimeSpan.FromSeconds(PackageListingTaskTimeout)))
314345
{
315-
if (!Settings.Get(Settings.K.DisableTimeoutOnPackageListingTasks))
346+
if (
347+
!allowDisablingTimeout
348+
|| !Settings.Get(Settings.K.DisableTimeoutOnPackageListingTasks)
349+
)
316350
{
317351
KillListingProcesses(processes);
318352
CoreTools.FinalizeDangerousTask(task);
319353
throw new TimeoutException(
320354
$"Task {taskName} for manager {Name} did not finish after "
321-
+ $"{PackageListingTaskTimeout} seconds, aborting. You may disable "
322-
+ $"timeouts from UniGetUI Advanced Settings"
355+
+ $"{PackageListingTaskTimeout} seconds, aborting."
356+
+ (
357+
allowDisablingTimeout
358+
? " You may disable timeouts from UniGetUI Advanced Settings"
359+
: ""
360+
)
323361
);
324362
}
325363

@@ -397,8 +435,7 @@ private IReadOnlyList<IPackage> _getAvailableUpdates(bool SecondAttempt)
397435
}
398436
try
399437
{
400-
Task.Run(RefreshPackageIndexes)
401-
.Wait(TimeSpan.FromSeconds(PackageListingTaskTimeout));
438+
RefreshPackageIndexesSafely();
402439

403440
var packages = RunListingTaskWithTimeout(
404441
GetAvailableUpdates_UnSafe,

‎src/UniGetUI.PackageEngine.Tests/Infrastructure/Fakes/TestProcessTaskLogger.cs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using UniGetUI.PackageEngine.ManagerClasses.Classes;
2+
3+
namespace UniGetUI.PackageEngine.Tests.Infrastructure.Fakes;
4+
5+
public sealed class TestProcessTaskLogger : IProcessTaskLogger
6+
{
7+
public List<string> StdOut { get; } = [];
8+
public List<string> StdErr { get; } = [];
9+
public List<string> StdIn { get; } = [];
10+
public int? ReturnCode { get; private set; }
11+
12+
public void AddToStdOut(IReadOnlyList<string> lines) => StdOut.AddRange(lines);
13+
14+
public void AddToStdOut(string? line)
15+
{
16+
if (line is not null)
17+
StdOut.Add(line);
18+
}
19+
20+
public void AddToStdErr(IReadOnlyList<string> lines) => StdErr.AddRange(lines);
21+
22+
public void AddToStdErr(string? line)
23+
{
24+
if (line is not null)
25+
StdErr.Add(line);
26+
}
27+
28+
public void AddToStdIn(IReadOnlyList<string> lines) => StdIn.AddRange(lines);
29+
30+
public void AddToStdIn(string? line)
31+
{
32+
if (line is not null)
33+
StdIn.Add(line);
34+
}
35+
36+
public IReadOnlyList<string> AsColoredString(bool verbose = false) => [];
37+
38+
public void Close(int returnCode) => ReturnCode = returnCode;
39+
}

0 commit comments

Comments
 (0)