Fix browser check in Page.ScreenshotAsync (#2373)

Author: kblok

lib/PuppeteerSharp/Browser.cs

@@ -3,13 +3,10 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Linq;
-using System.Runtime.InteropServices;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using PuppeteerSharp.Helpers;
-using PuppeteerSharp.Helpers.Json;
 using PuppeteerSharp.Messaging;
-using PuppeteerSharp.QueryHandlers;
 
 namespace PuppeteerSharp
 {
@@ -37,6 +34,7 @@ internal Browser(
             Func<Target, bool> targetFilter = null,
             Func<Target, bool> isPageTargetFunc = null)
         {
+            BrowserType = browser;
             IgnoreHTTPSErrors = ignoreHTTPSErrors;
             DefaultViewport = defaultViewport;
             Launcher = launcher;
@@ -96,6 +94,9 @@ internal Browser(
         /// <inheritdoc/>
         public string WebSocketEndpoint => Connection.Url;
 
+        /// <inheritdoc/>
+        public SupportedBrowser BrowserType { get; }
+
         /// <inheritdoc/>
         public Process Process => Launcher?.Process;
 

lib/PuppeteerSharp/IBrowser.cs

@@ -2,7 +2,6 @@
 using System.Diagnostics;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
-using PuppeteerSharp.QueryHandlers;
 
 namespace PuppeteerSharp
 {
@@ -70,6 +69,11 @@ public interface IBrowser : IDisposable, IAsyncDisposable
         /// <value>The default context.</value>
         IBrowserContext DefaultContext { get; }
 
+        /// <summary>
+        /// Returns the browser type. Chrome, Chromium or Firefox.
+        /// </summary>
+        SupportedBrowser BrowserType { get; }
+
         /// <summary>
         /// Default wait time in milliseconds. Defaults to 30 seconds.
         /// </summary>

lib/PuppeteerSharp/Page.cs

@@ -239,15 +239,15 @@ public int DefaultTimeout
 
         internal bool IsDragging { get; set; }
 
-        internal Browser Browser => Target.Browser;
+        internal bool HasPopupEventListeners => Popup?.GetInvocationList().Any() == true;
 
-        internal Target Target { get; }
+        private Browser Browser => Target.Browser;
 
-        internal bool JavascriptEnabled { get; set; } = true;
+        private Target Target { get; }
 
-        internal bool HasPopupEventListeners => Popup?.GetInvocationList().Any() == true;
+        private bool JavascriptEnabled { get; set; } = true;
 
-        internal FrameManager FrameManager { get; private set; }
+        private FrameManager FrameManager { get; set; }
 
         private Task SessionClosedTask
         {
@@ -405,7 +405,7 @@ public Task SetRequestInterceptionAsync(bool value)
         public async Task<CookieParam[]> GetCookiesAsync(params string[] urls)
             => (await Client.SendAsync<NetworkGetCookiesResponse>("Network.getCookies", new NetworkGetCookiesRequest
             {
-                Urls = urls.Length > 0 ? urls : new string[] { Url },
+                Urls = urls.Length > 0 ? urls : new[] { Url },
             }).ConfigureAwait(false)).Cookies;
 
         /// <inheritdoc/>
@@ -620,10 +620,8 @@ public async Task ScreenshotAsync(string file, ScreenshotOptions options)
 
             var data = await ScreenshotDataAsync(options).ConfigureAwait(false);
 
-            using (var fs = AsyncFileHelper.CreateStream(file, FileMode.Create))
-            {
-                await fs.WriteAsync(data, 0, data.Length).ConfigureAwait(false);
-            }
+            using var fs = AsyncFileHelper.CreateStream(file, FileMode.Create);
+            await fs.WriteAsync(data, 0, data.Length).ConfigureAwait(false);
         }
 
         /// <inheritdoc/>
@@ -659,12 +657,12 @@ public Task<string> ScreenshotBase64Async(ScreenshotOptions options)
                 }
             }
 
-            if (options?.Clip?.Width == 0)
+            if (options.Clip?.Width == 0)
             {
                 throw new PuppeteerException("Expected options.Clip.Width not to be 0.");
             }
 
-            if (options?.Clip?.Height == 0)
+            if (options.Clip?.Height == 0)
             {
                 throw new PuppeteerException("Expected options.Clip.Height not to be 0.");
             }
@@ -826,7 +824,7 @@ public async Task WaitForNetworkIdleAsync(WaitForNetworkIdleOptions options = nu
                 Interval = idleTime,
             };
 
-            idleTimer.Elapsed += (sender, args) =>
+            idleTimer.Elapsed += (_, _) =>
             {
                 networkIdleTcs.TrySetResult(true);
             };
@@ -1098,7 +1096,7 @@ await Client.SendAsync(
         /// <inheritdoc/>
         public Task EmulateCPUThrottlingAsync(decimal? factor = null)
         {
-            if (factor != null && factor < 1)
+            if (factor is < 1)
             {
                 throw new ArgumentException("Throttling rate should be greater or equal to 1", nameof(factor));
             }
@@ -1150,7 +1148,19 @@ internal static async Task<Page> CreateAsync(
             }
         }
 
-        internal async Task<byte[]> PdfInternalAsync(string file, PdfOptions options)
+        internal void OnPopup(IPage popupPage) => Popup?.Invoke(this, new PopupEventArgs { PopupPage = popupPage });
+
+        /// <summary>
+        /// Dispose resources.
+        /// </summary>
+        /// <param name="disposing">Indicates whether disposal was initiated by <see cref="Dispose()"/> operation.</param>
+        protected virtual void Dispose(bool disposing)
+        {
+            Mouse.Dispose();
+            _ = DisposeAsync();
+        }
+
+        private async Task<byte[]> PdfInternalAsync(string file, PdfOptions options)
         {
             var paperWidth = PaperFormat.Letter.Width;
             var paperHeight = PaperFormat.Letter.Height;
@@ -1211,18 +1221,6 @@ internal async Task<byte[]> PdfInternalAsync(string file, PdfOptions options)
             return await ProtocolStreamReader.ReadProtocolStreamByteAsync(Client, result.Stream, file).ConfigureAwait(false);
         }
 
-        internal void OnPopup(IPage popupPage) => Popup?.Invoke(this, new PopupEventArgs { PopupPage = popupPage });
-
-        /// <summary>
-        /// Dispose resources.
-        /// </summary>
-        /// <param name="disposing">Indicates whether disposal was initiated by <see cref="Dispose()"/> operation.</param>
-        protected virtual void Dispose(bool disposing)
-        {
-            Mouse.Dispose();
-            _ = DisposeAsync();
-        }
-
         private async Task InitializeAsync()
         {
             await FrameManager.InitializeAsync().ConfigureAwait(false);
@@ -1240,8 +1238,8 @@ private async Task InitializeAsync()
             networkManager.RequestServedFromCache += (_, e) => RequestServedFromCache?.Invoke(this, e);
 
             await Task.WhenAll(
-               Client.SendAsync("Performance.enable", null),
-               Client.SendAsync("Log.enable", null)).ConfigureAwait(false);
+               Client.SendAsync("Performance.enable"),
+               Client.SendAsync("Log.enable")).ConfigureAwait(false);
         }
 
         private async Task<IResponse> GoAsync(int delta, NavigationOptions options)
@@ -1293,7 +1291,7 @@ private async Task<string> PerformScreenshot(ScreenshotType type, ScreenshotOpti
 
             // FromSurface is not supported on Firefox.
             // It seems that Puppeteer solved this just by ignoring screenshot tests in firefox.
-            if (Browser.Launcher.Options.Browser == SupportedBrowser.Firefox)
+            if (Browser.BrowserType == SupportedBrowser.Firefox)
             {
                 if (options.FromSurface != null)
                 {
@@ -1302,15 +1300,15 @@ private async Task<string> PerformScreenshot(ScreenshotType type, ScreenshotOpti
             }
             else
             {
-                options.FromSurface = options.FromSurface.HasValue ? options.FromSurface : true;
+                options.FromSurface ??= true;
             }
 
             var clip = options.Clip != null ? ProcessClip(options.Clip) : null;
             var captureBeyondViewport = options.CaptureBeyondViewport;
 
             if (!_screenshotBurstModeOn)
             {
-                if (options?.FullPage == true)
+                if (options.FullPage)
                 {
                     // Overwrite clip for full page at all times.
                     clip = null;
@@ -1356,13 +1354,13 @@ private async Task<string> PerformScreenshot(ScreenshotType type, ScreenshotOpti
                     }
                 }
 
-                if (options?.OmitBackground == true && type == ScreenshotType.Png)
+                if (options.OmitBackground && type == ScreenshotType.Png)
                 {
                     await SetTransparentBackgroundColorAsync().ConfigureAwait(false);
                 }
             }
 
-            if (options?.FullPage == false && clip == null)
+            if (options.FullPage == false && clip == null)
             {
                 captureBeyondViewport = false;
             }
@@ -1417,7 +1415,7 @@ private Clip ProcessClip(Clip clip)
 
         private Task ResetBackgroundColorAndViewportAsync(ScreenshotOptions options)
         {
-            var omitBackgroundTask = options?.OmitBackground == true && options.Type == ScreenshotType.Png ?
+            var omitBackgroundTask = options is { OmitBackground: true, Type: ScreenshotType.Png } ?
                 ResetDefaultBackgroundColorAsync() : Task.CompletedTask;
             var setViewPortTask = (options?.FullPage == true && Viewport != null) ?
                 SetViewportAsync(Viewport) : Task.CompletedTask;
@@ -1604,7 +1602,7 @@ private async Task OnLogEntryAddedAsync(LogEntryAddedResponse e)
         {
             if (e.Entry.Args != null)
             {
-                foreach (var arg in e.Entry?.Args)
+                foreach (var arg in e.Entry.Args)
                 {
                     await RemoteObjectHelper.ReleaseObjectAsync(Client, arg, _logger).ConfigureAwait(false);
                 }
@@ -1648,14 +1646,16 @@ private string GetExceptionMessage(EvaluateExceptionResponseDetails exceptionDet
             }
 
             var message = exceptionDetails.Text;
-            if (exceptionDetails.StackTrace != null)
+            if (exceptionDetails.StackTrace == null)
             {
-                foreach (var callframe in exceptionDetails.StackTrace.CallFrames)
-                {
-                    var location = $"{callframe.Url}:{callframe.LineNumber}:{callframe.ColumnNumber}";
-                    var functionName = callframe.FunctionName ?? "<anonymous>";
-                    message += $"\n at {functionName} ({location})";
-                }
+                return message;
+            }
+
+            foreach (var callFrame in exceptionDetails.StackTrace.CallFrames)
+            {
+                var location = $"{callFrame.Url}:{callFrame.LineNumber}:{callFrame.ColumnNumber}";
+                var functionName = callFrame.FunctionName ?? "<anonymous>";
+                message += $"\n at {functionName} ({location})";
             }
 
             return message;
@@ -1732,7 +1732,7 @@ await Task.WhenAll(Frames.Select(
                     .ContinueWith(
                         task =>
                         {
-                            if (task.IsFaulted)
+                            if (task.IsFaulted && task.Exception != null)
                             {
                                 _logger.LogError(task.Exception.ToString());
                             }

lib/PuppeteerSharp/Puppeteer.cs

@@ -1,6 +1,4 @@
-using System;
 using System.Collections.Generic;
-using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using PuppeteerSharp.Mobile;
@@ -41,7 +39,7 @@ public static class Puppeteer
         public static IReadOnlyDictionary<DeviceDescriptorName, DeviceDescriptor> Devices => DeviceDescriptors.ToReadOnly();
 
         /// <summary>
-        /// Returns a list of network conditions to be used with <seealso cref="IPage.EmulateNetworkConditionsAsync(NetworkConditions)"/>.
+        /// Returns a list of network conditions to be used with <seealso cref="IPage.EmulateNetworkConditionsAsync(PuppeteerSharp.NetworkConditions)"/>.
         /// Actual list of conditions can be found in <seealso cref="PredefinedNetworkConditions.Conditions"/>.
         /// </summary>
         /// <example>

lib/PuppeteerSharp/PuppeteerSharp.csproj

@@ -12,10 +12,10 @@
     <Description>Headless Browser .NET API</Description>
     <PackageId>PuppeteerSharp</PackageId>
     <PackageReleaseNotes></PackageReleaseNotes>
-    <PackageVersion>13.0.1</PackageVersion>
-    <ReleaseVersion>13.0.1</ReleaseVersion>
-    <AssemblyVersion>13.0.1</AssemblyVersion>
-    <FileVersion>13.0.1</FileVersion>
+    <PackageVersion>13.0.2</PackageVersion>
+    <ReleaseVersion>13.0.2</ReleaseVersion>
+    <AssemblyVersion>13.0.2</AssemblyVersion>
+    <FileVersion>13.0.2</FileVersion>
     <SynchReleaseVersion>false</SynchReleaseVersion>
     <StyleCopTreatErrorsAsWarnings>false</StyleCopTreatErrorsAsWarnings>
     <DebugType>embedded</DebugType>