[rust] Throw a descriptive message when error parsing JSON from respo…

…nse (#13291)
SeleniumHQ · Dec 12, 2023 · dee5bc5 · dee5bc5
1 parent 82f7cf7
commit dee5bc5
Show file tree

Hide file tree

Showing 6 changed files with 49 additions and 48 deletions.
diff --git a/rust/src/chrome.rs b/rust/src/chrome.rs
@@ -96,23 +96,27 @@ impl ChromeManager {
         )
     }
 
-    fn create_cft_url(&self, endpoint: &str) -> String {
-        format!(
-            "{}{}",
-            self.get_browser_mirror_url_or_default(CFT_URL),
-            endpoint
-        )
+    fn create_cft_url(&self, base_url: &str, endpoint: &str) -> String {
+        format!("{}{}", base_url, endpoint)
+    }
+
+    fn create_cft_url_for_browsers(&self, endpoint: &str) -> String {
+        self.create_cft_url(&self.get_browser_mirror_url_or_default(CFT_URL), endpoint)
+    }
+
+    fn create_cft_url_for_drivers(&self, endpoint: &str) -> String {
+        self.create_cft_url(&self.get_driver_mirror_url_or_default(CFT_URL), endpoint)
     }
 
-    fn request_driver_version_from_latest(&self, driver_url: String) -> Result<String, Error> {
+    fn request_driver_version_from_latest(&self, driver_url: &str) -> Result<String, Error> {
         self.log.debug(format!(
             "Reading {} version from {}",
             &self.driver_name, driver_url
         ));
         read_version_from_link(self.get_http_client(), driver_url, self.get_logger())
     }
 
-    fn request_versions_from_online<T>(&self, driver_url: String) -> Result<T, Error>
+    fn request_versions_from_online<T>(&self, driver_url: &str) -> Result<T, Error>
     where
         T: Serialize + for<'a> Deserialize<'a>,
     {
@@ -128,17 +132,17 @@ impl ChromeManager {
             driver_name
         ));
 
-        let latest_versions_url = self.create_cft_url(LATEST_VERSIONS_ENDPOINT);
+        let latest_versions_url = self.create_cft_url_for_drivers(LATEST_VERSIONS_ENDPOINT);
         let versions_with_downloads =
-            self.request_versions_from_online::<LatestVersionsWithDownloads>(latest_versions_url)?;
+            self.request_versions_from_online::<LatestVersionsWithDownloads>(&latest_versions_url)?;
         let stable_channel = versions_with_downloads.channels.stable;
         let chromedriver = stable_channel.downloads.chromedriver;
         if chromedriver.is_none() {
             self.log.warn(format!(
                 "Latest stable version of {} not found using CfT endpoints. Trying with {}",
                 &self.driver_name, LATEST_RELEASE
             ));
-            return self.request_driver_version_from_latest(self.create_latest_release_url());
+            return self.request_driver_version_from_latest(&self.create_latest_release_url());
         }
 
         let platform_url: Vec<&PlatformUrl> = chromedriver
@@ -168,9 +172,9 @@ impl ChromeManager {
             "Driver version used to request CfT: {version_for_filtering}"
         ));
 
-        let good_versions_url = self.create_cft_url(GOOD_VERSIONS_ENDPOINT);
+        let good_versions_url = self.create_cft_url_for_drivers(GOOD_VERSIONS_ENDPOINT);
         let all_versions =
-            self.request_versions_from_online::<VersionsWithDownloads>(good_versions_url)?;
+            self.request_versions_from_online::<VersionsWithDownloads>(&good_versions_url)?;
         let filtered_versions: Vec<Version> = all_versions
             .versions
             .into_iter()
@@ -308,7 +312,7 @@ impl SeleniumManager for ChromeManager {
                     // For old versions (chromedriver 114-), the traditional method should work:
                     // https://chromedriver.chromium.org/downloads
                     self.request_driver_version_from_latest(
-                        self.create_latest_release_with_version_url(),
+                        &self.create_latest_release_with_version_url(),
                     )?
                 } else {
                     // As of chromedriver 115+, the metadata for version discovery are published
@@ -442,9 +446,9 @@ impl SeleniumManager for ChromeManager {
             browser_name
         ));
 
-        let latest_versions_url = self.create_cft_url(LATEST_VERSIONS_ENDPOINT);
+        let latest_versions_url = self.create_cft_url_for_browsers(LATEST_VERSIONS_ENDPOINT);
         let versions_with_downloads =
-            self.request_versions_from_online::<LatestVersionsWithDownloads>(latest_versions_url)?;
+            self.request_versions_from_online::<LatestVersionsWithDownloads>(&latest_versions_url)?;
         let stable_channel = versions_with_downloads.channels.stable;
         let chrome = stable_channel.downloads.chrome;
 
@@ -476,9 +480,11 @@ impl SeleniumManager for ChromeManager {
         ));
 
         if self.is_browser_version_unstable() {
-            let latest_versions_url = self.create_cft_url(LATEST_VERSIONS_ENDPOINT);
+            let latest_versions_url = self.create_cft_url_for_browsers(LATEST_VERSIONS_ENDPOINT);
             let versions_with_downloads = self
-                .request_versions_from_online::<LatestVersionsWithDownloads>(latest_versions_url)?;
+                .request_versions_from_online::<LatestVersionsWithDownloads>(
+                    &latest_versions_url,
+                )?;
             let channel = if browser_version.eq_ignore_ascii_case(BETA) {
                 versions_with_downloads.channels.beta
             } else if browser_version.eq_ignore_ascii_case(DEV) {
@@ -497,9 +503,9 @@ impl SeleniumManager for ChromeManager {
 
             Ok(browser_version)
         } else {
-            let good_versions_url = self.create_cft_url(GOOD_VERSIONS_ENDPOINT);
+            let good_versions_url = self.create_cft_url_for_browsers(GOOD_VERSIONS_ENDPOINT);
             let all_versions =
-                self.request_versions_from_online::<VersionsWithDownloads>(good_versions_url)?;
+                self.request_versions_from_online::<VersionsWithDownloads>(&good_versions_url)?;
             let filtered_versions: Vec<Version> = all_versions
                 .versions
                 .into_iter()

diff --git a/rust/src/downloads.rs b/rust/src/downloads.rs
@@ -21,7 +21,6 @@ use anyhow::anyhow;
 use anyhow::Error;
 use reqwest::{Client, StatusCode};
 use serde::{Deserialize, Serialize};
-use serde_json::Value;
 use std::fs::File;
 use std::io::copy;
 use std::io::Cursor;
@@ -76,14 +75,14 @@ pub async fn download_to_tmp_folder(
 
 pub fn read_version_from_link(
     http_client: &Client,
-    url: String,
+    url: &str,
     log: &Logger,
 ) -> Result<String, Error> {
     parse_version(read_content_from_link(http_client, url)?, log)
 }
 
 #[tokio::main]
-pub async fn read_content_from_link(http_client: &Client, url: String) -> Result<String, Error> {
+pub async fn read_content_from_link(http_client: &Client, url: &str) -> Result<String, Error> {
     Ok(http_client.get(url).send().await?.text().await?)
 }
 
@@ -99,17 +98,16 @@ pub async fn read_redirect_from_link(
     )
 }
 
-pub fn parse_json_from_url<T>(http_client: &Client, url: String) -> Result<T, Error>
+pub fn parse_json_from_url<T>(http_client: &Client, url: &str) -> Result<T, Error>
 where
     T: Serialize + for<'a> Deserialize<'a>,
 {
     let content = read_content_from_link(http_client, url)?;
-    let response: T = serde_json::from_str(&content)?;
-    Ok(response)
-}
-
-pub fn parse_generic_json_from_url(http_client: &Client, url: String) -> Result<Value, Error> {
-    let content = read_content_from_link(http_client, url)?;
-    let response: Value = serde_json::from_str(&content)?;
-    Ok(response)
+    match serde_json::from_str(&content) {
+        Ok(json) => Ok(json),
+        Err(err) => Err(anyhow!(format!(
+            "Error parsing JSON from URL {} {}",
+            url, err
+        ))),
+    }
 }
diff --git a/rust/src/edge.rs b/rust/src/edge.rs
@@ -220,11 +220,10 @@ impl SeleniumManager for EdgeManager {
                     ));
                     let latest_driver_version = read_version_from_link(
                         self.get_http_client(),
-                        latest_stable_url,
+                        &latest_stable_url,
                         self.get_logger(),
                     )?;
-                    major_browser_version =
-                        self.get_major_version(latest_driver_version.as_str())?;
+                    major_browser_version = self.get_major_version(&latest_driver_version)?;
                     self.log.debug(format!(
                         "Latest {} major version is {}",
                         &self.driver_name, major_browser_version
@@ -242,7 +241,7 @@ impl SeleniumManager for EdgeManager {
                     &self.driver_name, driver_url
                 ));
                 let driver_version =
-                    read_version_from_link(self.get_http_client(), driver_url, self.get_logger())?;
+                    read_version_from_link(self.get_http_client(), &driver_url, self.get_logger())?;
 
                 let driver_ttl = self.get_ttl();
                 if driver_ttl > 0 && !major_browser_version.is_empty() {
@@ -365,7 +364,7 @@ impl SeleniumManager for EdgeManager {
         ));
 
         let edge_products =
-            parse_json_from_url::<Vec<EdgeProduct>>(self.get_http_client(), edge_updates_url)?;
+            parse_json_from_url::<Vec<EdgeProduct>>(self.get_http_client(), &edge_updates_url)?;
 
         let edge_channel = if self.is_beta(browser_version) {
             "Beta"

diff --git a/rust/src/firefox.rs b/rust/src/firefox.rs
@@ -19,10 +19,7 @@
 use crate::config::ManagerConfig;
 use crate::config::ARCH::{ARM64, X32};
 use crate::config::OS::{LINUX, MACOS, WINDOWS};
-use crate::downloads::{
-    parse_generic_json_from_url, parse_json_from_url, read_content_from_link,
-    read_redirect_from_link,
-};
+use crate::downloads::{parse_json_from_url, read_content_from_link, read_redirect_from_link};
 use crate::files::{compose_driver_path_in_cache, BrowserPath};
 use crate::metadata::{
     create_driver_metadata, get_driver_version_from_metadata, get_metadata, write_metadata,
@@ -36,6 +33,7 @@ use anyhow::Error;
 use reqwest::Client;
 use serde::Deserialize;
 use serde::Serialize;
+use serde_json::Value;
 use std::collections::HashMap;
 use std::path::PathBuf;
 
@@ -101,7 +99,7 @@ impl FirefoxManager {
         let browser_version = self.get_browser_version().to_string();
         let firefox_versions_url = self.create_firefox_details_url(endpoint);
         let firefox_versions =
-            parse_generic_json_from_url(self.get_http_client(), firefox_versions_url)?;
+            parse_json_from_url::<Value>(self.get_http_client(), &firefox_versions_url)?;
 
         let versions_map = firefox_versions.as_object().unwrap();
         let filter_key = if browser_version.contains('.') {
@@ -220,7 +218,7 @@ impl SeleniumManager for FirefoxManager {
 
                 let driver_version = match parse_json_from_url::<GeckodriverReleases>(
                     self.get_http_client(),
-                    DRIVER_VERSIONS_URL.to_string(),
+                    DRIVER_VERSIONS_URL,
                 ) {
                     Ok(driver_releases) => {
                         let major_browser_version_int =
@@ -407,7 +405,7 @@ impl SeleniumManager for FirefoxManager {
 
         let firefox_versions_url = self.create_firefox_details_url(FIREFOX_VERSIONS_ENDPOINT);
         let firefox_versions =
-            parse_generic_json_from_url(self.get_http_client(), firefox_versions_url)?;
+            parse_json_from_url::<Value>(self.get_http_client(), &firefox_versions_url)?;
         let browser_version = firefox_versions
             .get(FIREFOX_STABLE_LABEL)
             .unwrap()
@@ -430,7 +428,7 @@ impl SeleniumManager for FirefoxManager {
         if self.is_unstable(browser_version) {
             let firefox_versions_url = self.create_firefox_details_url(FIREFOX_VERSIONS_ENDPOINT);
             let firefox_versions =
-                parse_generic_json_from_url(self.get_http_client(), firefox_versions_url)?;
+                parse_json_from_url::<Value>(self.get_http_client(), &firefox_versions_url)?;
             let version_label = if self.is_beta(browser_version) {
                 FIREFOX_BETA_LABEL
             } else if self.is_dev(browser_version) {
@@ -480,7 +478,7 @@ impl SeleniumManager for FirefoxManager {
                 );
                 self.get_logger()
                     .trace(format!("Checking release URL: {}", release_url));
-                let content = read_content_from_link(self.get_http_client(), release_url)?;
+                let content = read_content_from_link(self.get_http_client(), &release_url)?;
                 if !content.contains("Not Found") {
                     return Ok(version.to_string());
                 }

diff --git a/rust/src/grid.rs b/rust/src/grid.rs
@@ -121,7 +121,7 @@ impl SeleniumManager for GridManager {
 
                 let selenium_releases = parse_json_from_url::<Vec<SeleniumRelease>>(
                     self.get_http_client(),
-                    MIRROR_URL.to_string(),
+                    MIRROR_URL,
                 )?;
 
                 let filtered_releases: Vec<SeleniumRelease> = selenium_releases

diff --git a/rust/src/iexplorer.rs b/rust/src/iexplorer.rs
@@ -134,7 +134,7 @@ impl SeleniumManager for IExplorerManager {
 
                 let selenium_releases = parse_json_from_url::<Vec<SeleniumRelease>>(
                     self.get_http_client(),
-                    MIRROR_URL.to_string(),
+                    MIRROR_URL,
                 )?;
 
                 let filtered_releases: Vec<SeleniumRelease> = selenium_releases