fix: prefer reporting error over status for 200 OK

Chatterino · May 5, 2024 · be88d66 · be88d66
1 parent 401feac
commit be88d66
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,7 @@
 
 ## Unversioned
 
+- Bugfix: If a network request errors with 200 OK, Qt's error code is now reported instead of the HTTP status. (#5378)
 - Dev: Add doxygen build target. (#5377)
 - Dev: Make printing of strings in tests easier. (#5379)
 

diff --git a/src/common/network/NetworkResult.cpp b/src/common/network/NetworkResult.cpp
@@ -67,7 +67,9 @@ const QByteArray &NetworkResult::getData() const
 
 QString NetworkResult::formatError() const
 {
-    if (this->status_)
+    // Print the status for errors that mirror HTTP status codes (=0 || >99)
+    if (this->status_ && (this->error_ == QNetworkReply::NoError ||
+                          this->error_ > QNetworkReply::UnknownNetworkError))
     {
         return QString::number(*this->status_);
     }
@@ -77,6 +79,13 @@ QString NetworkResult::formatError() const
             this->error_);
     if (name == nullptr)
     {
+        if (this->status_)
+        {
+            return QStringLiteral("unknown error (status: %1, error: %2)")
+                .arg(QString::number(*this->status_),
+                     QString::number(this->error_));
+        }
+
         return QStringLiteral("unknown error (%1)").arg(this->error_);
     }
     return name;

diff --git a/tests/src/NetworkResult.cpp b/tests/src/NetworkResult.cpp
@@ -37,12 +37,21 @@ TEST(NetworkResult, Errors)
                 "RemoteHostClosedError");
 
     // status code takes precedence
-    checkResult({Error::TimeoutError, 400, {}}, Error::TimeoutError, 400,
-                "400");
+    checkResult({Error::InternalServerError, 400, {}},
+                Error::InternalServerError, 400, "400");
+
+    // error takes precedence (1..=99)
+    checkResult({Error::BackgroundRequestNotAllowedError, 400, {}},
+                Error::BackgroundRequestNotAllowedError, 400,
+                "BackgroundRequestNotAllowedError");
+    checkResult({Error::UnknownNetworkError, 400, {}},
+                Error::UnknownNetworkError, 400, "UnknownNetworkError");
 }
 
 TEST(NetworkResult, InvalidError)
 {
     checkResult({static_cast<Error>(-1), {}, {}}, static_cast<Error>(-1),
                 std::nullopt, "unknown error (-1)");
+    checkResult({static_cast<Error>(-1), 42, {}}, static_cast<Error>(-1), 42,
+                "unknown error (status: 42, error: -1)");
 }