refactor: improve gp response parsing

crates/gpapi/src/gateway/login.rs

@@ -8,7 +8,7 @@ use crate::{
   credential::Credential,
   error::PortalError,
   gp_params::GpParams,
-  utils::{normalize_server, parse_gp_error, remove_url_scheme},
+  utils::{normalize_server, parse_gp_response, remove_url_scheme},
 };
 
 pub enum GatewayLogin {
@@ -41,20 +41,10 @@ pub async fn gateway_login(gateway: &str, cred: &Credential, gp_params: &GpParam
     .await
     .map_err(|e| anyhow::anyhow!(PortalError::NetworkError(e.to_string())))?;
 
-  let status = res.status();
+  let res = parse_gp_response(res).await.map_err(|err| {
-
+    warn!("{err}");
-  if status.is_client_error() || status.is_server_error() {
+    anyhow::anyhow!("Gateway login error: {}", err.reason)
-    let (reason, res) = parse_gp_error(res).await;
+  })?;
-
-    warn!(
-      "Gateway login error: reason={}, status={}, response={}",
-      reason, status, res
-    );
-
-    bail!("Gateway login error, reason: {}", reason);
-  }
-
-  let res = res.text().await?;
 
   // MFA detected
   if res.contains("Challenge") {

crates/gpapi/src/portal/config.rs

@@ -10,7 +10,7 @@ use crate::{
   error::PortalError,
   gateway::{parse_gateways, Gateway},
   gp_params::GpParams,
-  utils::{normalize_server, parse_gp_error, remove_url_scheme, xml},
+  utils::{normalize_server, parse_gp_response, remove_url_scheme, xml},
 };
 
 #[derive(Debug, Serialize, Type)]
@@ -108,24 +108,19 @@ pub async fn retrieve_config(portal: &str, cred: &Credential, gp_params: &GpPara
     .send()
     .await
     .map_err(|e| anyhow::anyhow!(PortalError::NetworkError(e.to_string())))?;
-  let status = res.status();
 
-  if status == StatusCode::NOT_FOUND {
+  let res_xml = parse_gp_response(res).await.or_else(|err| {
-    bail!(PortalError::ConfigError("Config endpoint not found".to_string()))
+    if err.status == StatusCode::NOT_FOUND {
+      bail!(PortalError::ConfigError("Config endpoint not found".to_string()));
     }
 
-  if status.is_client_error() || status.is_server_error() {
+    if err.is_status_error() {
-    let (reason, res) = parse_gp_error(res).await;
+      warn!("{err}");
-
+      bail!("Portal config error: {}", err.reason);
-    warn!(
-      "Portal config error: reason={}, status={}, response={}",
-      reason, status, res
-    );
-
-    bail!("Portal config error, reason: {}", reason);
     }
 
-  let res_xml = res.text().await.map_err(|e| PortalError::ConfigError(e.to_string()))?;
+    Err(anyhow::anyhow!(PortalError::ConfigError(err.reason)))
+  })?;
 
   if res_xml.is_empty() {
     bail!(PortalError::ConfigError("Empty portal config response".to_string()))

crates/gpapi/src/portal/prelogin.rs

@@ -8,7 +8,7 @@ use specta::Type;
 use crate::{
   error::PortalError,
   gp_params::GpParams,
-  utils::{base64, normalize_server, parse_gp_error, xml},
+  utils::{base64, normalize_server, parse_gp_response, xml},
 };
 
 const REQUIRED_PARAMS: [&str; 8] = [
@@ -126,23 +126,18 @@ pub async fn prelogin(portal: &str, gp_params: &GpParams) -> anyhow::Result<Prel
     .await
     .map_err(|e| anyhow::anyhow!(PortalError::NetworkError(e.to_string())))?;
 
-  let status = res.status();
+  let res_xml = parse_gp_response(res).await.or_else(|err| {
-  if status == StatusCode::NOT_FOUND {
+    if err.status == StatusCode::NOT_FOUND {
       bail!(PortalError::PreloginError("Prelogin endpoint not found".to_string()))
     }
 
-  if status.is_client_error() || status.is_server_error() {
+    if err.is_status_error() {
-    let (reason, res) = parse_gp_error(res).await;
+      warn!("{err}");
-
+      bail!("Prelogin error: {}", err.reason)
-    warn!("Prelogin error: reason={}, status={}, response={}", reason, status, res);
-
-    bail!("Prelogin error: {}", status)
     }
 
-  let res_xml = res
+    Err(anyhow!(PortalError::PreloginError(err.reason)))
-    .text()
+  })?;
-    .await
-    .map_err(|e| PortalError::PreloginError(e.to_string()))?;
 
   let prelogin = parse_res_xml(res_xml, is_gateway).map_err(|e| PortalError::PreloginError(e.to_string()))?;
 

crates/gpapi/src/utils/mod.rs

@@ -1,5 +1,3 @@
-use reqwest::{Response, Url};
-
 pub(crate) mod xml;
 
 pub mod base64;
@@ -15,8 +13,12 @@ pub mod window;
 
 mod shutdown_signal;
 
+use log::warn;
 pub use shutdown_signal::shutdown_signal;
 
+use reqwest::{Response, StatusCode, Url};
+use thiserror::Error;
+
 /// Normalize the server URL to the format `https://<host>:<port>`
 pub fn normalize_server(server: &str) -> anyhow::Result<String> {
   let server = if server.starts_with("https://") || server.starts_with("http://") {
@@ -42,7 +44,41 @@ pub fn remove_url_scheme(s: &str) -> String {
   s.replace("http://", "").replace("https://", "")
 }
 
-pub(crate) async fn parse_gp_error(res: Response) -> (String, String) {
+#[derive(Error, Debug)]
+#[error("GP response error: reason={reason}, status={status}, body={body}")]
+pub(crate) struct GpError {
+  pub status: StatusCode,
+  pub reason: String,
+  body: String,
+}
+
+impl GpError {
+  pub fn is_status_error(&self) -> bool {
+    self.status.is_client_error() || self.status.is_server_error()
+  }
+}
+
+pub(crate) async fn parse_gp_response(res: Response) -> anyhow::Result<String, GpError> {
+  let status = res.status();
+
+  if status.is_client_error() || status.is_server_error() {
+    let (reason, body) = parse_gp_error(res).await;
+
+    return Err(GpError { status, reason, body });
+  }
+
+  res.text().await.map_err(|err| {
+    warn!("Failed to read response: {}", err);
+
+    GpError {
+      status,
+      reason: "failed to read response".to_string(),
+      body: "<failed to read response>".to_string(),
+    }
+  })
+}
+
+async fn parse_gp_error(res: Response) -> (String, String) {
   let reason = res
     .headers()
     .get("x-private-pan-globalprotect")