From 65f19e7045d3d8b4381ce8898e34ccac89ecab6e Mon Sep 17 00:00:00 2001
From: axunonb <axunonb@users.noreply.github.com>
Date: Wed, 25 Sep 2024 15:58:12 +0200
Subject: [PATCH] Refactor PDF generation with ReportSheetCache (#190)

* Refactor PDF generation with ReportSheetCache
* Pass a `CancellationToken` to `CreateReportSheetPdfChromium`.
* Enhanced `CreateReportSheetPdfChromium` to handle process timeouts using `Task.WhenAny` and `TimeSpan`, and to throw `OperationCanceledException` if the process exceeds the timeout.
* Minor comment adjustment and clarification of `ProtocolTimeout` in `GetPathToCacheFile`.
* ReportSheetCache.UsePuppeteer default to false
---
 League/Caching/ReportSheetCache.cs | 35 ++++++++++++++----------------
 League/Controllers/Match.cs        |  2 +-
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/League/Caching/ReportSheetCache.cs b/League/Caching/ReportSheetCache.cs
index 0e8eaf7..9d19236 100644
--- a/League/Caching/ReportSheetCache.cs
+++ b/League/Caching/ReportSheetCache.cs
@@ -47,13 +47,14 @@ public ReportSheetCache(ITenantContext tenantContext, IConfiguration configurati
         _pathToChromium = Path.Combine(webHostEnvironment.ContentRootPath, configuration["Chromium:ExecutablePath"] ?? string.Empty);
         _loggerFactory = loggerFactory;
         _logger = loggerFactory.CreateLogger<ReportSheetCache>();
+        UsePuppeteer = false;
     }
 
     /// <summary>
     /// Gets or sets a value indicating whether to use Puppeteer for generating the report sheet,
     /// instead of Chromium command line.
     /// </summary>
-    public bool UsePuppeteer { get; set; } = false;
+    public bool UsePuppeteer { get; set; }
 
     private void EnsureCacheFolder()
     {
@@ -108,7 +109,7 @@ private static bool IsOutdated(string cacheFile, DateTime dataModifiedOn)
         // Temporary file with HTML content - extension must be ".html"!
         var htmlUri = await CreateHtmlFile(html, tempFolder, cancellationToken);
 
-        var pdfFile = await CreateReportSheetPdfChromium(tempFolder, htmlUri);
+        var pdfFile = await CreateReportSheetPdfChromium(tempFolder, htmlUri, cancellationToken);
 
         var cacheFile = MovePdfToCache(pdfFile, matchId);
 
@@ -154,7 +155,7 @@ private string GetPathToCacheFile(long matchId)
                 { "--no-sandbox", "--disable-gpu", "--disable-extensions", "--use-cmd-decoder=passthrough" },
             ExecutablePath = _pathToChromium,
             Timeout = 5000,
-            ProtocolTimeout = 10000 // default is 180,000
+            ProtocolTimeout = 10000 // default is 180,000 - used for page.PdfDataAsync
         };
         // Use Puppeteer as a wrapper for the browser, which can generate PDF from HTML
         // Start command line arguments set by Puppeteer v20:
@@ -168,7 +169,6 @@ private string GetPathToCacheFile(long matchId)
         var fullPath = GetPathToCacheFile(matchId);
         try
         {
-            // page.PdfDataAsync times out after 180,000ms (3 minutes)
             var bytes = await page.PdfDataAsync(new PuppeteerSharp.PdfOptions
                 { Scale = 1.0M, Format = PuppeteerSharp.Media.PaperFormat.A4 }).ConfigureAwait(false);
 
@@ -183,9 +183,10 @@ private string GetPathToCacheFile(long matchId)
         return fullPath;
     }
 
-    private async Task<string> CreateReportSheetPdfChromium(string tempFolder, string htmlUri)
+    private async Task<string> CreateReportSheetPdfChromium(string tempFolder, string htmlUri, CancellationToken cancellationToken)
     {
-        // Temporary file for the PDF stream form Chromium
+        // Temporary file for the PDF stream from Chromium
+        // Note: non-existing file is handled in MovePdfToCache
         var pdfFile = Path.Combine(tempFolder, Path.GetRandomFileName() + ".pdf");
 
         // Run Chromium
@@ -196,25 +197,21 @@ private async Task<string> CreateReportSheetPdfChromium(string tempFolder, strin
             { CreateNoWindow = true, UseShellExecute = false };
         var proc = System.Diagnostics.Process.Start(startInfo);
 
-        if (proc is null)
+        if (proc == null)
         {
             _logger.LogError("Process '{PathToChromium}' could not be started.", _pathToChromium);
+            return pdfFile;
         }
 
-        const int timeout = 8000;
-        var timePassed = 0;
-        while (proc is { HasExited: false })
-        {
-            timePassed += 100;
-            await Task.Delay(100, default);
-            if (timePassed < timeout) continue;
+        var timeout = TimeSpan.FromMilliseconds(5000);
+        var processTask = proc.WaitForExitAsync(cancellationToken);
 
-            proc.Kill(true);
-            throw new OperationCanceledException($"Chromium timed out after {timeout}ms.");
-        }
+        await Task.WhenAny(processTask, Task.Delay(timeout, cancellationToken));
+
+        if (processTask.IsCompleted) return pdfFile;
 
-        // non-existing file is handled in MovePdfToCache
-        return pdfFile;
+        proc.Kill(true);
+        throw new OperationCanceledException($"Chromium timed out after {timeout.TotalMilliseconds}ms.");
     }
 
     private static async Task<string> CreateHtmlFile(string html, string tempFolder, CancellationToken cancellationToken)
diff --git a/League/Controllers/Match.cs b/League/Controllers/Match.cs
index b4c0b35..5f1f2cc 100644
--- a/League/Controllers/Match.cs
+++ b/League/Controllers/Match.cs
@@ -714,7 +714,7 @@ private async Task<EnterResultViewModel> GetEnterResultViewModel(MatchEntity mat
     public async Task<IActionResult> ReportSheet(long id, [FromServices] ReportSheetCache cache, CancellationToken cancellationToken)
     {
         MatchReportSheetRow? model = null;
-        cache.UsePuppeteer = true;
+        cache.UsePuppeteer = false;
             
         try
         {