Initialize logging earlier (#2218)

When an event handler is attached as part of the constructor and invoked before `_logger` has been populated we might throw an NRE.
hardkoded · Jun 2, 2023 · 791f324 · 791f324
1 parent 5774379
commit 791f324
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 12 deletions.
diff --git a/lib/PuppeteerSharp/Browser.cs b/lib/PuppeteerSharp/Browser.cs
@@ -46,6 +46,7 @@ public class Browser : IBrowser
             Connection = connection;
             _closeCallback = closeCallback;
             _targetFilterCallback = targetFilter ?? ((TargetInfo _) => true);
+            _logger = Connection.LoggerFactory.CreateLogger<Browser>();
             _isPageTargetFunc =
                 isPageTargetFunc ??
                 new Func<TargetInfo, bool>((TargetInfo target) =>

diff --git a/lib/PuppeteerSharp/ChromeTargetManager.cs b/lib/PuppeteerSharp/ChromeTargetManager.cs
@@ -36,9 +36,9 @@ internal class ChromeTargetManager : ITargetManager
             _connection = connection;
             _targetFilterFunc = targetFilterFunc;
             _targetFactoryFunc = targetFactoryFunc;
-            _connection.MessageReceived += OnMessageReceived;
-            _connection.SessionDetached += Connection_SessionDetached;
             _logger = _connection.LoggerFactory.CreateLogger<ChromeTargetManager>();
+            _connection.MessageReceived += OnMessageReceived;
+            _connection.SessionDetached += Connection_SessionDetached;
         }
 
         public event EventHandler<TargetChangedArgs> TargetAvailable;

diff --git a/lib/PuppeteerSharp/FirefoxTargetManager.cs b/lib/PuppeteerSharp/FirefoxTargetManager.cs
@@ -1,4 +1,4 @@
-#pragma warning disable CS0067 // Temporal, do not merge with this
+#pragma warning disable CS0067 // Temporal, do not merge with this
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
@@ -31,9 +31,9 @@ internal class FirefoxTargetManager : ITargetManager
             _connection = connection;
             _targetFilterFunc = targetFilterFunc;
             _targetFactoryFunc = targetFactoryFunc;
+            _logger = _connection.LoggerFactory.CreateLogger<FirefoxTargetManager>();
             _connection.MessageReceived += OnMessageReceived;
             _connection.SessionDetached += Connection_SessionDetached;
-            _logger = _connection.LoggerFactory.CreateLogger<FirefoxTargetManager>();
         }
 
         public event EventHandler<TargetChangedArgs> TargetAvailable;
@@ -191,4 +191,4 @@ private void FinishInitializationIfReady(string targetId = null)
         }
     }
 }
-#pragma warning restore CS0067
+#pragma warning restore CS0067
diff --git a/lib/PuppeteerSharp/IsolatedWorld.cs b/lib/PuppeteerSharp/IsolatedWorld.cs
@@ -39,7 +39,7 @@ internal class IsolatedWorld
             Frame frame,
             TimeoutSettings timeoutSettings)
         {
-            Logger = client.Connection.LoggerFactory.CreateLogger<IsolatedWorld>();
+            _logger = client.Connection.LoggerFactory.CreateLogger<IsolatedWorld>();
             _client = client;
             _frameManager = frameManager;
             _customQueriesManager = ((Browser)frameManager.Page.Browser).CustomQueriesManager;
@@ -48,7 +48,6 @@ internal class IsolatedWorld
 
             _detached = false;
             _client.MessageReceived += Client_MessageReceived;
-            _logger = _client.Connection.LoggerFactory.CreateLogger<IsolatedWorld>();
         }
 
         internal TaskManager TaskManager { get; set; } = new();
@@ -57,8 +56,6 @@ internal class IsolatedWorld
 
         internal bool HasContext => _contextResolveTaskWrapper?.Task.IsCompleted == true;
 
-        internal ILogger Logger { get; }
-
         internal ConcurrentDictionary<string, Delegate> BoundFunctions { get; } = new();
 
         internal TaskCompletionSource<IJSHandle> PuppeteerUtilTaskCompletionSource { get; private set; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -500,7 +497,7 @@ private async Task InjectPuppeteerUtil(ExecutionContext context)
             }
             catch (Exception ex)
             {
-                Logger.LogError(ex.ToString());
+                _logger.LogError(ex.ToString());
             }
         }
 
@@ -518,7 +515,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e)
             catch (Exception ex)
             {
                 var message = $"IsolatedWorld failed to process {e.MessageID}. {ex.Message}. {ex.StackTrace}";
-                Logger.LogError(ex, message);
+                _logger.LogError(ex, message);
                 _client.Close(message);
             }
         }

diff --git a/lib/PuppeteerSharp/NetworkManager.cs b/lib/PuppeteerSharp/NetworkManager.cs
@@ -36,8 +36,8 @@ internal NetworkManager(CDPSession client, bool ignoreHTTPSErrors, FrameManager
             FrameManager = frameManager;
             _client = client;
             _ignoreHTTPSErrors = ignoreHTTPSErrors;
-            _client.MessageReceived += Client_MessageReceived;
             _logger = _client.Connection.LoggerFactory.CreateLogger<NetworkManager>();
+            _client.MessageReceived += Client_MessageReceived;
         }
 
         internal event EventHandler<ResponseCreatedEventArgs> Response;