Cleaning up stale puppeteer pages

Author: JudahGabriel-MS

apps/pwabuilder/Services/PuppeteerService.cs

@@ -1,14 +1,14 @@
-using Azure;
+using System.Collections.Concurrent;
 using PuppeteerSharp;
 using PWABuilder.Common;
-using PWABuilder.Models;
 
 namespace PWABuilder.Services
 {
     public class PuppeteerService : IPuppeteerService
     {
         private readonly Task<IBrowser> reusableBrowser;
         private readonly ILogger<PuppeteerService> logger;
+        private readonly ConcurrentBag<PageReference> openPages = [];
 
         public PuppeteerService(Task<IBrowser> reusableBrowser, ILogger<PuppeteerService> logger)
         {
@@ -101,24 +101,12 @@ public async Task<IPage> NavigateAsync(Uri site)
             {
                 logger.LogWarning("There are {pageCount} pages open in the Puppeteer browser. Possible page leak detected.", pages.Length);
             }
-            if (pages.Length > 100)
-            {
-                // Extreme case: close all pages to free up resources.
-                logger.LogError("Too many pages ({pageCount}) open in the Puppeteer browser. Closing all pages to free up resources.", pages.Length);
-                foreach (var openPage in pages)
-                {
-                    try
-                    {
-                        await openPage.CloseAsync();
-                    }
-                    catch (Exception ex)
-                    {
-                        logger.LogError(ex, "Error closing page during cleanup.");
-                    }
-                }
-            }
+
+            // Close any pages that have been opened for more than an hour.
+            await TryCleanupOldPages();
 
             var page = await browser.NewPageAsync();
+            this.openPages.Add(new PageReference(page, site));
 
             // Disable performance monitoring to avoid Protocol error (Performance.enable) issues
             await page.SetCacheEnabledAsync(false);
@@ -154,5 +142,47 @@ public async ValueTask DisposeAsync()
             var browser = await this.reusableBrowser;
             await browser.DisposeAsync();
         }
+
+        private async Task TryCleanupOldPages()
+        {
+            var hourAgo = DateTimeOffset.UtcNow.AddHours(-1);
+            foreach (var openPage in this.openPages)
+            {
+                if (openPage.CreatedAt < hourAgo)
+                {
+                    logger.LogInformation("Closing stale Puppeteer page for {url} opened {time} ago.", openPage.Url, (DateTimeOffset.UtcNow - openPage.CreatedAt).ToString(@"hh\:mm"));
+                    openPage.Page.TryGetTarget(out var page);
+                    if (page != null)
+                    {
+                        openPages.TryTake(out var _);
+                        try
+                        {
+                            await page.CloseAsync();
+                        }
+                        catch (Exception closeError)
+                        {
+                            logger.LogWarning(closeError, "Error closing stale Puppeteer page for {url}.", openPage.Url);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    /// <summary>
+    /// An open page in Puppeteer. Uses a weak reference to allow for garbage collection. Used for cleaning up old pages that weren't properly disposed.
+    /// </summary>
+    public class PageReference
+    {
+        public WeakReference<IPage> Page { get; }
+        public DateTimeOffset CreatedAt { get; }
+        public Uri Url { get; }
+
+        public PageReference(IPage page, Uri url)
+        {
+            Page = new WeakReference<IPage>(page);
+            CreatedAt = DateTimeOffset.UtcNow;
+            Url = url;
+        }
     }
 }