Add error cases for reaching a server that is not Solr

Sh1nku · Oct 25, 2024 · aa5df6f · aa5df6f
1 parent 0291d43
commit aa5df6f
Show file tree

Hide file tree

Showing 11 changed files with 230 additions and 36 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,6 @@
+# v0.6.0
+* Breaking changes to error handling. Clearer error messages.
+
 # v0.5.0
 
 * Add logging of solr requests

diff --git a/Cargo.toml b/Cargo.toml
@@ -7,7 +7,7 @@ members = [
 
 [workspace.package]
 edition = "2021"
-version = "0.5.0"
+version = "0.6.0"
 
 [workspace.dependencies]
 solrstice = { path = "framework" }

diff --git a/docker/error-nginx.conf b/docker/error-nginx.conf
@@ -3,12 +3,20 @@ events {}
 http {
     server {
         listen 80;
-	location /solr/notfound_collection/ {
+	    location /solr/notfound_collection/ {
             return 404 "Collection not found";
         }
 
         location /solr/error_collection/ {
             return 500 "Internal Server Error";
         }
+
+        location /solr/always_200/ {
+            return 200 "Always 200";
+        }
+
+        location /status {
+            return 200 "We are up";
+        }
     }
 }
diff --git a/docs/src/index.md b/docs/src/index.md
@@ -8,7 +8,7 @@ The library is written in Rust, and has a wrapper to Python. Both async and bloc
 ### Rust
 You can install the library by putting this in your `Cargo.toml`
 ```toml
-solrstice = { version = "0.5.0", features = ["blocking"] }
+solrstice = { version = "0.6.0", features = ["blocking"] }
 ```
 If the `blocking` feature is not provided, only async will work.
 * [Rust mdBook docs]()

diff --git a/framework/src/queries/request_builder.rs b/framework/src/queries/request_builder.rs
@@ -1,6 +1,7 @@
 use crate::error::{try_solr_error, Error};
 use crate::models::context::SolrServerContext;
 use crate::models::response::SolrResponse;
+use crate::Error::SolrConnectionError;
 use log::debug;
 use reqwest::header::HeaderMap;
 use reqwest::{Body, Request, RequestBuilder, Response, Url};
@@ -73,10 +74,7 @@ impl<'a> SolrRequestBuilder<'a> {
         log_request_info(&request, self.context.logging_policy);
 
         let response = client.execute(request).await?;
-        try_request_auth_error(&response).await?;
-        let solr_response = response.json::<SolrResponse>().await?;
-        try_solr_error(&solr_response)?;
-        Ok(solr_response)
+        handle_solr_response(response).await
     }
 
     pub async fn send_post_with_json<T: Serialize + 'a + ?Sized>(
@@ -98,10 +96,7 @@ impl<'a> SolrRequestBuilder<'a> {
         log_request_info(&request, self.context.logging_policy);
 
         let response = client.execute(request).await?;
-        try_request_auth_error(&response).await?;
-        let solr_response = response.json::<SolrResponse>().await?;
-        try_solr_error(&solr_response)?;
-        Ok(solr_response)
+        handle_solr_response(response).await
     }
 
     pub async fn send_post_with_body<T: Into<Body>>(self, data: T) -> Result<SolrResponse, Error> {
@@ -120,10 +115,7 @@ impl<'a> SolrRequestBuilder<'a> {
         log_request_info(&request, self.context.logging_policy);
 
         let response = client.execute(request).await?;
-        try_request_auth_error(&response).await?;
-        let solr_response = response.json::<SolrResponse>().await?;
-        try_solr_error(&solr_response)?;
-        Ok(solr_response)
+        handle_solr_response(response).await
     }
 }
 
@@ -154,22 +146,24 @@ async fn create_standard_request<'a>(
     Ok(request)
 }
 
-async fn try_request_auth_error(response: &Response) -> Result<(), Error> {
-    match response.error_for_status_ref() {
-        Ok(_) => Ok(()),
-        Err(e) => {
-            if e.status().ok_or(Error::Unknown(
-                "Error while getting response code from request".to_string(),
-            ))? == 401
-            {
-                Err(Error::SolrAuthError(
-                    "Authentication failed with 401. Check credentials.".to_string(),
-                ))
-            } else {
-                Ok(())
-            }
-        }
+async fn handle_solr_response(response: Response) -> Result<SolrResponse, Error> {
+    let url = response.url().clone();
+    let status_code = response.status();
+    let body = response.text().await.unwrap_or_default();
+    let solr_response = serde_json::from_str::<SolrResponse>(&body);
+    if let Ok(r) = solr_response {
+        try_solr_error(&r)?;
+        return Ok(r);
+    }
+    if status_code == 401 {
+        return Err(Error::SolrAuthError(
+            "Authentication failed with 401. Check credentials.".to_string(),
+        ));
     }
+    Err(SolrConnectionError(format!(
+        "Error while sending request to {}: {}\n{}",
+        url, status_code, body
+    )))
 }
 
 static NO_BODY: &[u8] = "No body".as_bytes();

diff --git a/framework/tests/functionality/error_test.rs b/framework/tests/functionality/error_test.rs
@@ -0,0 +1,44 @@
+use crate::structures::ErrrorTestsSetup;
+use serial_test::serial;
+use solrstice::{AsyncSolrCloudClient, Error, SelectQuery};
+
+#[tokio::test]
+#[serial]
+async fn sensible_error_message_if_not_solr_server() -> Result<(), Error> {
+    let config = ErrrorTestsSetup::new().await;
+    let client = AsyncSolrCloudClient::new(config.context);
+
+    let result = client.select(SelectQuery::new(), "error_collection").await;
+    assert!(
+        result.is_err()
+            && result
+                .unwrap_err()
+                .to_string()
+                .contains("500 Internal Server Error")
+    );
+    Ok(())
+}
+
+#[tokio::test]
+#[serial]
+async fn sensible_error_message_if_non_existent_collection() -> Result<(), Error> {
+    let config = ErrrorTestsSetup::new().await;
+    let client = AsyncSolrCloudClient::new(config.context);
+
+    let result = client
+        .select(SelectQuery::new(), "notfound_collection")
+        .await;
+    assert!(result.is_err() && result.unwrap_err().to_string().contains("404 Not Found"));
+    Ok(())
+}
+
+#[tokio::test]
+#[serial]
+async fn sensible_error_message_if_200_but_not_solr() -> Result<(), Error> {
+    let config = ErrrorTestsSetup::new().await;
+    let client = AsyncSolrCloudClient::new(config.context);
+
+    let result = client.select(SelectQuery::new(), "always_200").await;
+    assert!(result.is_err() && result.unwrap_err().to_string().contains("200 OK"));
+    Ok(())
+}
diff --git a/framework/tests/functionality/logging_test.rs b/framework/tests/functionality/logging_test.rs
@@ -1,10 +1,10 @@
-use crate::structures::{BaseTestsBuildup, FunctionalityTestsBuildup};
+use crate::structures::{BaseTestsBuildup, ErrrorTestsSetup, FunctionalityTestsBuildup};
 use log::{Metadata, Record};
 use serial_test::serial;
-use solrstice::{AsyncSolrCloudClient, SelectQuery};
 use solrstice::Error;
 use solrstice::LoggingPolicy;
 use solrstice::SolrServerContextBuilder;
+use solrstice::{AsyncSolrCloudClient, SelectQuery};
 use std::sync::{Arc, Mutex, OnceLock};
 
 struct TestLogger {
@@ -120,4 +120,26 @@ async fn logging_works_if_request_fails() -> Result<(), Error> {
         }
     }
     Err(Error::Unknown("No log message found".to_string()))
-}
+}
+
+#[tokio::test]
+#[serial]
+async fn logging_works_if_request_never_reaches_solr() -> Result<(), Error> {
+    let config = ErrrorTestsSetup::new().await;
+    let client = AsyncSolrCloudClient::new(config.context);
+
+    LOGGER_MESSAGES
+        .get_or_init(init_logger)
+        .lock()
+        .unwrap()
+        .clear();
+
+    let _ = client.select(SelectQuery::new(), "error_collection").await;
+    let messages = LOGGER_MESSAGES.get().unwrap().lock().unwrap();
+    for message in messages.iter() {
+        if message.contains("Sending Solr request to") {
+            return Ok(());
+        }
+    }
+    Err(Error::Unknown("No log message found".to_string()))
+}
diff --git a/framework/tests/functionality/mod.rs b/framework/tests/functionality/mod.rs
@@ -13,6 +13,8 @@ pub mod zk_test;
 
 pub mod auth_test;
 
+pub mod error_test;
+pub mod logging_test;
+
 #[cfg(feature = "blocking")]
 pub mod blocking_tests;
-mod logging_test;
diff --git a/framework/tests/structures.rs b/framework/tests/structures.rs
@@ -47,6 +47,46 @@ impl BaseTestsBuildup {
     }
 }
 
+pub struct ErrrorTestsSetup {
+    pub context: SolrServerContext,
+    pub config_path: String,
+    pub host: SolrSingleServerHost,
+    pub auth: Option<SolrBasicAuth>,
+}
+
+impl ErrrorTestsSetup {
+    pub async fn new() -> Self {
+        dotenv::from_filename("../test_setup/.env").ok();
+        let username = std::env::var("SOLR_USERNAME").unwrap();
+        let password = std::env::var("SOLR_PASSWORD").unwrap();
+        let auth = match username.is_empty() {
+            true => None,
+            false => Some(SolrBasicAuth::new(
+                username.as_str(),
+                Some(password.as_str()),
+            )),
+        };
+        let error_nginx_hostname = std::env::var("ERROR_NGINX_HOST")
+            .unwrap()
+            .as_str()
+            .to_string();
+        let host = SolrSingleServerHost::new(&error_nginx_hostname);
+        let builder = SolrServerContextBuilder::new(host.clone());
+        let context = if let Some(auth) = auth.clone() {
+            builder.with_auth(auth).build()
+        } else {
+            builder.build()
+        };
+        wait_for_error_nginx(&error_nginx_hostname, Duration::from_secs(30)).await;
+        ErrrorTestsSetup {
+            context,
+            config_path: "../test_setup/test_collection".to_string(),
+            host,
+            auth,
+        }
+    }
+}
+
 pub struct FunctionalityTestsBuildup {
     pub context: SolrServerContext,
     pub async_client: AsyncSolrCloudClient,
@@ -147,3 +187,14 @@ pub async fn wait_for_solr(context: &SolrServerContext, max_time: Duration) {
     }
     panic!("Solr did not respond within {:?} seconds", max_time);
 }
+
+pub async fn wait_for_error_nginx(host: &str, max_time: Duration) {
+    let end = std::time::Instant::now() + max_time;
+    while std::time::Instant::now() < end {
+        let response = reqwest::get(format!("{}/status", host)).await.unwrap();
+        if response.status().is_success() {
+            return;
+        }
+        tokio::time::sleep(Duration::from_secs(1)).await;
+    }
+}
diff --git a/wrappers/python/tests/helpers.py b/wrappers/python/tests/helpers.py
@@ -24,6 +24,7 @@
 class Config:
     solr_host: str
     speedbump_host: Optional[str]
+    error_nginx_host: Optional[str]
     solr_username: Optional[str]
     solr_password: Optional[str]
     solr_auth: Optional[SolrBasicAuth]
@@ -57,6 +58,7 @@ def create_config(logging: bool = False) -> Config:
     host = os.getenv("SOLR_HOST")
     assert host is not None
     speedbump_host = os.getenv("SPEEDBUMP_HOST")
+    error_nginx_host = os.getenv("ERROR_NGINX_HOST")
     solr_host = SolrSingleServerHost(host)
     context = SolrServerContext(
         solr_host, solr_auth, OffLoggingPolicy() if not logging else None
@@ -65,6 +67,7 @@ def create_config(logging: bool = False) -> Config:
     return Config(
         host,
         speedbump_host,
+        error_nginx_host,
         solr_username,
         solr_password,
         solr_auth,
@@ -79,7 +82,7 @@ def wait_for_solr(host: str, max_time: int) -> None:
     while time.time() < end:
         try:
             with urlopen(
-                f'{host}{"/solr/admin/collections"}?action=CLUSTERSTATUS'
+                    f'{host}{"/solr/admin/collections"}?action=CLUSTERSTATUS'
             ) as response:
                 if response.status == 200:
                     return
@@ -92,6 +95,38 @@ def wait_for_solr(host: str, max_time: int) -> None:
     raise RuntimeError(f"Solr did not respond within {max_time} seconds")
 
 
+@dataclass
+class ErrorTestsSetup:
+    error_nginx_host: str
+    context: SolrServerContext
+    async_client: AsyncSolrCloudClient
+
+
+def create_nginx_error_config() -> ErrorTestsSetup:
+    path = os.path.join(get_path_prefix(), "test_setup/.env")
+    load_dotenv(path)
+    error_nginx_host = os.getenv("ERROR_NGINX_HOST")
+    assert error_nginx_host is not None
+    context = SolrServerContext(SolrSingleServerHost(error_nginx_host))
+    wait_for_error_nginx(error_nginx_host, 30)
+    return ErrorTestsSetup(error_nginx_host, context, AsyncSolrCloudClient(context))
+
+
+def wait_for_error_nginx(host: str, max_time: int) -> None:
+    end = time.time() + max_time
+    while time.time() < end:
+        try:
+            with urlopen(
+                    f'{host}{"/status"}'
+            ) as response:
+                if response.status == 200:
+                    return
+        except Exception:
+            pass
+        time.sleep(1)
+    raise RuntimeError(f"Error nginx did not respond within {max_time} seconds")
+
+
 @dataclass_json
 @dataclass
 class Population(DataClassJsonMixin):
@@ -121,7 +156,7 @@ async def index_test_data(context: SolrServerContext, name: str) -> None:
 
 
 async def setup_collection(
-    context: SolrServerContext, name: str, config_path: str
+        context: SolrServerContext, name: str, config_path: str
 ) -> None:
     try:
         await delete_collection(context, name)