fix: Bubble homeserver error back to user cleanly. Remove middleware

Author: 86667

src/shared/homeserver_admin_api.rs

@@ -53,13 +53,4 @@ impl HomeserverAdminAPI {
         response.error_for_status()?;
         Ok(())
     }
-
-    /// Checks if the homeserver is responsive by making a GET request to the root URL
-    /// This is NOT password protected so it won't validate the admin password.
-    pub async fn health_check(&self) -> Result<(), reqwest::Error> {
-        let url = self.base_url.join("/").expect("Failed to join URL path");
-        let response = self.http_client.get(url).send().await?;
-        response.error_for_status()?;
-        Ok(())
-    }
 }

src/shared/homeserver_health_middleware.rs

@@ -1,5 +1,3 @@
 mod homeserver_admin_api;
-mod homeserver_health_middleware;
 
 pub use homeserver_admin_api::HomeserverAdminAPI;
-pub use homeserver_health_middleware::check_homeserver_health;

src/shared/mod.rs

@@ -8,7 +8,6 @@ use crate::{
 pub struct AppState {
     pub db: SqlDb,
     pub sms_verification: SmsVerificationService,
-    pub homeserver_admin_api: HomeserverAdminAPI,
 }
 
 impl AppState {
@@ -28,7 +27,6 @@ impl AppState {
         Self {
             db,
             sms_verification,
-            homeserver_admin_api,
         }
     }
 }

src/sms_verification/app_state.rs

@@ -24,6 +24,9 @@ pub enum SmsVerificationError {
     #[error("External service rate limit exceeded")]
     RateLimited { retry_after: Option<u64> },
 
+    #[error("Homeserver temporarily unavailable, please retry")]
+    HomeserverUnavailable,
+
     #[error("HTTP request failed: {0}")]
     RequestFailed(#[from] reqwest::Error),
 

src/sms_verification/error.rs

@@ -2,7 +2,6 @@ use axum::{
     Json, Router,
     extract::State,
     http::StatusCode,
-    middleware,
     response::{IntoResponse, Response},
     routing::post,
 };
@@ -22,22 +21,9 @@ pub async fn router(
     db: &crate::infrastructure::sql::SqlDb,
 ) -> Result<Router, HttpServerError> {
     let state = AppState::new(config, db.clone());
-    let homeserver_api = state.homeserver_admin_api.clone();
-
-    // NOTE: SMS verification routes REQUIRE homeserver health check middleware.
-    // Unlike LN verification, SMS cannot use database transactions for atomicity because:
-    // 1. Prelude API destructively marks verification as complete when check_code() succeeds
-    // 2. If generate_signup_token() fails after that, we can't rollback Prelude's state
-    // 3. This would create an inconsistent state: Prelude says "verified", our DB says "failed"
-    //
-    // The middleware mitigates this by failing fast (503) if homeserver is unreachable.
     Ok(Router::new()
         .route("/send_code", post(send_code_handler))
         .route("/validate_code", post(validate_code_handler))
-        .route_layer(middleware::from_fn(move |req, next| {
-            let api = homeserver_api.clone();
-            crate::shared::check_homeserver_health(api, req, next)
-        }))
         .with_state(state))
 }
 
@@ -95,8 +81,12 @@ impl IntoResponse for SmsVerificationError {
                 }
                 return response;
             }
+            SmsVerificationError::HomeserverUnavailable => {
+                tracing::error!("Homeserver unavailable during SMS verification");
+                StatusCode::INTERNAL_SERVER_ERROR
+            }
             SmsVerificationError::RequestFailed(ref err) => {
-                tracing::error!(error = %err, "Failed to communicate with SMS provider");
+                tracing::error!(error = %err, "Failed to communicate with external service");
                 StatusCode::INTERNAL_SERVER_ERROR
             }
             SmsVerificationError::Database(ref err) => {

src/sms_verification/http.rs

@@ -159,7 +159,7 @@ impl SmsVerificationService {
             PreludeCheckCodeResponse::Success { id, .. } => {
                 let code = match self.homeserver_admin_api.generate_signup_token().await {
                     Ok(code) => code,
-                    Err(e) => {
+                    Err(_e) => {
                         if let Err(e) = SmsVerificationRepository::mark_failed(
                             &mut executor,
                             &id,
@@ -169,7 +169,7 @@ impl SmsVerificationService {
                         {
                             tracing::error!("{}", e);
                         }
-                        return Err(e.into());
+                        return Err(SmsVerificationError::HomeserverUnavailable);
                     }
                 };
 

src/sms_verification/service.rs

@@ -794,6 +794,20 @@ async fn test_service_success_but_homeserver_fails_marks_failed(pool: PgPool) {
         record.failure_reason,
         Some("homeserver_signup_token_generation_failed".to_string())
     );
+
+    // Verify that failed verification does NOT count towards quota
+    let phone_hash = test_phone_hasher().hash_phone_number(phone.as_str());
+    let failed_count = SmsVerificationRepository::count_verified_sessions_in_last_days(
+        &mut executor,
+        &phone_hash,
+        7,
+    )
+    .await
+    .unwrap();
+    assert_eq!(
+        failed_count, 0,
+        "Failed verification should NOT count towards weekly quota"
+    );
 }
 
 // This circumstance happens if validate_code() is called before send_code() - Prelude has no prelude_id for the verification session yet so it returns a dummy value which doesnt match to anything in our db.