Add IsBadResponseCode, check request object before accessing it for e…

…rror message
Blake-Madden · Feb 5, 2024 · 40ff4c7 · 40ff4c7
1 parent 3016242
commit 40ff4c7
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 18 deletions.
diff --git a/src/util/downloadfile.cpp b/src/util/downloadfile.cpp
@@ -163,7 +163,6 @@ bool FileDownload::Download(const wxString& url, const wxString& localDownloadPa
     m_lastState = wxWebRequest::State_Failed;
     m_stillActive = true;
     m_downloadSuccessful = false;
-    wxWebRequest tempRequest{ request };
     request.Start();
 
     wxProgressDialog* progressDlg = m_showProgress ?
@@ -215,7 +214,9 @@ void FileDownload::RequestResponse(const wxString& url)
     request.Start();
 
     while (m_stillActive)
-        { wxYield(); }
+        {
+        wxYield();
+        }
     }
 
 //--------------------------------------------------
@@ -243,7 +244,9 @@ bool FileDownload::Read(const wxString& url)
     request.Start();
 
     while (m_stillActive)
-        { wxYield(); }
+        {
+        wxYield();
+        }
 
     return (m_lastState == wxWebRequest::State_Completed);
     }
@@ -321,8 +324,17 @@ void FileDownload::ProcessRequest(wxWebRequestEvent& evt)
             break;
             }
         case wxWebRequest::State_Failed:
-            wxLogError(L"Web Request failed: %s (%s)",
-                       evt.GetErrorDescription(), QueueDownload::GetResponseMessage(evt.GetRequest().GetResponse().GetStatus()));
+            if (evt.GetRequest().IsOk() && evt.GetRequest().GetResponse().IsOk())
+                {
+                wxLogError(L"Web Request failed: %s (%s)",
+                    evt.GetErrorDescription(),
+                    QueueDownload::GetResponseMessage(evt.GetRequest().GetResponse().GetStatus()));
+                }
+            else
+                {
+                wxLogError(L"Web Request failed: %s",
+                    evt.GetErrorDescription());
+                }
             m_stillActive = false;
             fillResponseInfo();
             break;
@@ -332,7 +344,8 @@ void FileDownload::ProcessRequest(wxWebRequestEvent& evt)
             break;
         case wxWebRequest::State_Unauthorized:
             {
-            if (!evt.GetRequest().GetAuthChallenge().IsOk())
+            if (evt.GetRequest().IsOk() &&
+                !evt.GetRequest().GetAuthChallenge().IsOk())
                 {
                 wxLogStatus(L"Unexpectedly missing authentication challenge");
                 m_stillActive = false;

diff --git a/src/util/downloadfile.h b/src/util/downloadfile.h
@@ -140,7 +140,7 @@ class QueueDownload
     [[nodiscard]]
     static wxString GetResponseMessage(const int responseCode)
         {
-        if (responseCode < 300)
+        if (responseCode > 0 && responseCode < 300)
             {
             return _(L"Connection successful.");
             }
@@ -174,6 +174,17 @@ class QueueDownload
             return _(L"Unknown connection error.");
             };
         }
+    /// @brief Determines if a response code indicates a connection failure.
+    /// @param responseCode The web request response code.
+    /// @returns @c true if the response code indicates a connection failure.
+    [[nodiscard]]
+    inline constexpr static bool IsBadResponseCode(const int responseCode) noexcept
+        {
+        return (responseCode == 204 || responseCode == 400 || responseCode == 401 ||
+                responseCode == 402 || responseCode == 403 || responseCode == 404 ||
+                responseCode == 500 || responseCode == 501 || responseCode == 502 ||
+                responseCode == 503 || responseCode == 0);
+        }
 private:
     wxString GetLocalPath(const int ID) const;
     void Remove(const int ID);
@@ -194,21 +205,14 @@ class QueueDownload
         // You can also call SetEventHandler() and bind wxEVT_WEBREQUEST_STATE and
         // wxEVT_WEBREQUEST_DATA yourself if you prefer; this is a shortcut for that.
         m_downloadFile.SetAndBindEventHandler(this);
-
-        // Either bind this, or call m_downloadFile.CancelPending() in the
-        // wxEvtHandler's already-existing close event.
-        Bind(wxEVT_CLOSE_WINDOW, [this](wxCloseEvent& event)
-            {
-            m_downloadFile.CancelPending();
-            event.Skip();
-            });
     @endcode
 
     Later, the `wxEvtHandler`-derived class can call GetResponse(), Read(), or Download() as such:
     @code
         // get the content type of a page (without reading its full content)
         const wxString contentType = m_downloadFile.GetResponse(
-            "https://github.com/wxWidgets/wxWidgets/blob/master/README-GIT.md").GetHeader("Content-Type");
+            "https://github.com/wxWidgets/wxWidgets/blob/master/README-GIT.md")
+            .GetHeader("Content-Type");
 
         // download a file locally
         m_downloadFile.Download("https://github.com/wxWidgets/wxWidgets/blob/master/README-GIT.md",
@@ -219,8 +223,9 @@ class QueueDownload
                                 &m_downloadFile.GetLastRead()[0] : wxString{});
     @endcode
     @warning An `wxEvtHandler`-derived class can either be connected to a single QueueDownload
-        or a single FileDownload object. This is because the class must bind its @c wxEVT_WEBREQUEST_STATE
-        event to the `QueueDownload`'s or `FileDownload`'s @c ProcessRequest() method.
+        or a single FileDownload object. This is because the class must bind its
+        @c wxEVT_WEBREQUEST_STATE event to the `QueueDownload`'s or
+        `FileDownload`'s @c ProcessRequest() method.
 */
 class FileDownload
     {