Introduce url at error message at page.goto (#385)

* Introduce url at error message at page.goto

* Code Review

Author: kblok

lib/PuppeteerSharp.Tests/PageTests/GotoTests.cs

@@ -263,6 +263,15 @@ public async Task ShouldWorkWithSelfRequestingPage()
             Assert.Contains("self-request.html", response.Url);
         }
 
+        [Fact]
+        public async Task ShouldFailWhenNavigatingAndShowTheUrlAtTheErrorMessage()
+        {
+            var url = TestConstants.HttpsPrefix + "/redirect/1.html";
+            var exception = await Assert.ThrowsAnyAsync<NavigationException>(async () => await Page.GoToAsync(url));
+            Assert.Contains(url, exception.Message);
+            Assert.Contains(url, exception.Url);
+        }
+
         [Fact]
         public async Task ResponseOkShouldBeTrueForFile()
         {

lib/PuppeteerSharp/NavigationException.cs

@@ -9,6 +9,12 @@ namespace PuppeteerSharp
     [Serializable]
     public class NavigationException : PuppeteerException
     {
+        /// <summary>
+        /// Url that caused the exception
+        /// </summary>
+        /// <value>The URL.</value>
+        public string Url { get; }
+
         /// <summary>
         /// Initializes a new instance of the <see cref="NavigationException"/> class.
         /// </summary>
@@ -28,11 +34,20 @@ public NavigationException(string message) : base(message)
         /// Initializes a new instance of the <see cref="NavigationException"/> class.
         /// </summary>
         /// <param name="message">Message.</param>
-        /// <param name="innerException">Inner exception.</param>
-        public NavigationException(string message, Exception innerException) : base(message, innerException)
+        /// <param name="url">Url.</param>
+        public NavigationException(string message, string url) : base(message)
         {
+            Url = url;
         }
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="NavigationException"/> class.
+        /// </summary>
+        /// <param name="message">Message.</param>
+        /// <param name="innerException">Inner exception.</param>
+        public NavigationException(string message, Exception innerException) : base(message, innerException)
+            => Url = (innerException as NavigationException)?.Url;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="NavigationException"/> class.
         /// </summary>
@@ -41,5 +56,19 @@ public NavigationException(string message, Exception innerException) : base(mess
         protected NavigationException(SerializationInfo info, StreamingContext context) : base(info, context)
         {
         }
+
+        /// <inheritdoc/>
+        public override string Message
+        {
+            get
+            {
+                if (string.IsNullOrEmpty(Url))
+                {
+                    return base.Message;
+                }
+
+                return $"{base.Message} at {Url}";
+            }
+        }
     }
 }

lib/PuppeteerSharp/Page.cs

@@ -649,13 +649,13 @@ public async Task<Response> GoToAsync(string url, NavigationOptions options)
             var referrer = _networkManager.ExtraHTTPHeaders?.GetValueOrDefault("referer");
             var requests = new Dictionary<string, Request>();
 
-            EventHandler<RequestEventArgs> createRequestEventListener = (object sender, RequestEventArgs e) =>
+            void createRequestEventListener(object sender, RequestEventArgs e)
             {
                 if (!requests.ContainsKey(e.Request.Url))
                 {
                     requests.Add(e.Request.Url, e.Request);
                 }
-            };
+            }
 
             _networkManager.Request += createRequestEventListener;
 
@@ -1283,7 +1283,7 @@ public async Task<Response> WaitForNavigationAsync(NavigationOptions options = n
             var watcher = new NavigatorWatcher(_frameManager, mainFrame, timeout, options);
             var responses = new Dictionary<string, Response>();
 
-            EventHandler<ResponseCreatedEventArgs> createResponseEventListener = (sender, e) => responses[e.Response.Url] = e.Response;
+            void createResponseEventListener(object sender, ResponseCreatedEventArgs e) => responses[e.Response.Url] = e.Response;
 
             _networkManager.Response += createResponseEventListener;
 
@@ -1306,15 +1306,15 @@ public async Task<Response> WaitForNavigationAsync(NavigationOptions options = n
         /// <returns>Task which which resolves to the main resource response. In case of multiple redirects, 
         /// the navigation will resolve with the response of the last redirect. If can not go back, resolves to null.</returns>
         /// <param name="options">Navigation parameters.</param>
-        public Task<Response> GoBackAsync(NavigationOptions options = null) => GoAsync(-1, null);
+        public Task<Response> GoBackAsync(NavigationOptions options = null) => GoAsync(-1, options);
 
         /// <summary>
         /// Navigate to the next page in history.
         /// </summary>
         /// <returns>Task which which resolves to the main resource response. In case of multiple redirects, 
         /// the navigation will resolve with the response of the last redirect. If can not go forward, resolves to null.</returns>
         /// <param name="options">Navigation parameters.</param>
-        public Task<Response> GoForwardAsync(NavigationOptions options = null) => GoAsync(1, null);
+        public Task<Response> GoForwardAsync(NavigationOptions options = null) => GoAsync(1, options);
 
         #endregion
 
@@ -1402,7 +1402,7 @@ private async Task<byte[]> PerformScreenshot(string format, ScreenshotOptions op
                 targetId = Target.TargetId
             });
 
-            var clip = options.Clip != null ? options.Clip.Clone() : null;
+            var clip = options.Clip?.Clone();
             if (clip != null)
             {
                 clip.Scale = 1;
@@ -1740,7 +1740,7 @@ private async Task Navigate(CDPSession client, string url, string referrer)
 
             if (response.errorText != null)
             {
-                throw new NavigationException(response.errorText.ToString());
+                throw new NavigationException(response.errorText.ToString(), url);
             }
         }