From 50cb56dd5233503c90a713cfc0a58b8d5a476c2b Mon Sep 17 00:00:00 2001
From: kangalioo <jannik.a.schaper@web.de>
Date: Thu, 10 Nov 2022 23:04:11 +0100
Subject: [PATCH] Make HTTP error more useful (#2277)

While developing the automod regex PR, I noticed that HTTP errors include lots
of useless info about the URL, but don't even include the HTTP method
(POST/GET/PATCH/...). This commit changes that.

Old: `Http(UnsuccessfulRequest(ErrorResponse { status_code: 403, url: Url { scheme: "https", cannot_be_a_base: false, username: "", password: None, host: Some(Domain("discord.com")), port: None, path: "/api/v10/guilds/703332075914264606/auto-moderation/rules", query: None, fragment: None }, error: DiscordJsonError { code: 50001, message: "Missing Access", errors: [] } }))`

New: `Http(UnsuccessfulRequest(ErrorResponse { status_code: 403, url: "https://discord.com/api/v10/guilds/703332075914264606/auto-moderation/rules", method: POST, error: DiscordJsonError { code: 50001, message: "Missing Access", errors: [] } }))`
---
 src/http/client.rs | 12 +++++++++---
 src/http/error.rs  | 22 ++++++++++------------
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/src/http/client.rs b/src/http/client.rs
index 81fa7d67c0f..8a47cf58596 100644
--- a/src/http/client.rs
+++ b/src/http/client.rs
@@ -19,7 +19,7 @@ use super::ratelimiting::{RatelimitedRequest, Ratelimiter};
 use super::request::Request;
 use super::routing::RouteInfo;
 use super::typing::Typing;
-use super::{GuildPagination, HttpError, UserPagination};
+use super::{ErrorResponse, GuildPagination, HttpError, UserPagination};
 use crate::builder::CreateAttachment;
 use crate::constants;
 use crate::internal::prelude::*;
@@ -4046,6 +4046,7 @@ impl Http {
     /// ```
     #[instrument]
     pub async fn request(&self, req: Request<'_>) -> Result<ReqwestResponse> {
+        let method = req.route.deconstruct().0;
         let response = if self.ratelimiter_disabled {
             let request = req.build(&self.client, &self.token, self.proxy.as_ref())?.build()?;
             self.client.execute(request).await?
@@ -4057,7 +4058,9 @@ impl Http {
         if response.status().is_success() {
             Ok(response)
         } else {
-            Err(Error::Http(HttpError::from_response(response).await))
+            Err(Error::Http(HttpError::UnsuccessfulRequest(
+                ErrorResponse::from_response(response, method.reqwest_method()).await,
+            )))
         }
     }
 
@@ -4067,6 +4070,7 @@ impl Http {
     /// This is a function that performs a light amount of work and returns an
     /// empty tuple, so it's called "self.wind" to denote that it's lightweight.
     pub(super) async fn wind(&self, expected: u16, req: Request<'_>) -> Result<()> {
+        let method = req.route.deconstruct().0;
         let response = self.request(req).await?;
 
         if response.status().as_u16() == expected {
@@ -4076,7 +4080,9 @@ impl Http {
         debug!("Expected {}, got {}", expected, response.status());
         trace!("Unsuccessful response: {:?}", response);
 
-        Err(Error::Http(HttpError::from_response(response).await))
+        Err(Error::Http(HttpError::UnsuccessfulRequest(
+            ErrorResponse::from_response(response, method.reqwest_method()).await,
+        )))
     }
 }
 
diff --git a/src/http/error.rs b/src/http/error.rs
index f80388ecc86..e67a7d7e156 100644
--- a/src/http/error.rs
+++ b/src/http/error.rs
@@ -2,7 +2,7 @@ use std::error::Error as StdError;
 use std::fmt;
 
 use reqwest::header::InvalidHeaderValue;
-use reqwest::{Error as ReqwestError, Response, StatusCode, Url};
+use reqwest::{Error as ReqwestError, Method, Response, StatusCode};
 use serde::de::{Deserialize, Deserializer, Error as _};
 use url::ParseError as UrlError;
 
@@ -33,19 +33,22 @@ pub struct DiscordJsonSingleError {
 }
 
 #[derive(Clone, Debug, Eq, PartialEq)]
+#[non_exhaustive]
 pub struct ErrorResponse {
     pub status_code: StatusCode,
-    pub url: Url,
+    pub url: String,
+    pub method: Method,
     pub error: DiscordJsonError,
 }
 
 impl ErrorResponse {
     // We need a freestanding from-function since we cannot implement an async
     // From-trait.
-    pub async fn from_response(r: Response) -> Self {
+    pub async fn from_response(r: Response, method: Method) -> Self {
         ErrorResponse {
             status_code: r.status(),
-            url: r.url().clone(),
+            url: r.url().to_string(),
+            method,
             error: decode_resp(r).await.unwrap_or_else(|e| DiscordJsonError {
                 code: -1,
                 message: format!("[Serenity] Could not decode json when receiving error response from discord:, {e}"),
@@ -83,12 +86,6 @@ pub enum HttpError {
 }
 
 impl HttpError {
-    // We need a freestanding from-function since we cannot implement an async
-    // From-trait.
-    pub async fn from_response(r: Response) -> Self {
-        ErrorResponse::from_response(r).await.into()
-    }
-
     /// Returns true when the error is caused by an unsuccessful request
     #[must_use]
     pub fn is_unsuccessful_request(&self) -> bool {
@@ -274,11 +271,12 @@ mod test {
         let response = builder.body(body_string.into_bytes()).unwrap();
 
         let reqwest_response: reqwest::Response = response.into();
-        let error_response = ErrorResponse::from_response(reqwest_response).await;
+        let error_response = ErrorResponse::from_response(reqwest_response, Method::POST).await;
 
         let known = ErrorResponse {
             status_code: reqwest::StatusCode::from_u16(403).unwrap(),
-            url: String::from("https://ferris.crab").parse().unwrap(),
+            url: String::from("https://ferris.crab/"),
+            method: Method::POST,
             error,
         };