Classify SQL execution errors for logging

crates/client-api/src/lib.rs

@@ -15,6 +15,7 @@ use spacetimedb::host::{HostController, MigratePlanResult, ModuleHost, NoSuchMod
 use spacetimedb::identity::{AuthCtx, Identity};
 use spacetimedb::messages::control_db::{Database, HostType, Node, Replica};
 use spacetimedb::sql;
+use spacetimedb::error::DBError;
 use spacetimedb_client_api_messages::http::{SqlStmtResult, SqlStmtStats};
 use spacetimedb_client_api_messages::name::{DomainName, InsertDomainResult, RegisterTldResult, SetDomainsResult, Tld};
 use spacetimedb_lib::{ProductTypeElement, ProductValue};
@@ -164,9 +165,39 @@ impl Host {
         )
         .await
         .map_err(|e| {
-            // TODO: Review log level after user SQL errors can be distinguished from internal database failures.
-            log::warn!("{e}");
-            (StatusCode::BAD_REQUEST, e.to_string())
+            // Classify errors coming from SQL execution. Client errors (syntax,
+            // planning, typing, authorization, etc.) are logged at `warn!`
+            // and surface as `400 Bad Request`. Internal failures (datastore,
+            // I/O, durability, snapshots, view internal errors, etc.) are
+            // logged at `error!` and return `500 Internal Server Error` with a
+            // generic message to avoid leaking internal details.
+            match &e {
+                DBError::SqlParser { .. }
+                | DBError::Plan { .. }
+                | DBError::TypeError(_)
+                | DBError::WithSql { .. }
+                | DBError::Subscription(_)
+                | DBError::Sequence2(_)
+                | DBError::Schema(_)
+                | DBError::ParseInt(_)
+                | DBError::DecodeHex(_)
+                | DBError::DecodeHexHash(_)
+                | DBError::ReadViaBsatnError(_)
+                | DBError::ModuleValidationErrors(_)
+                => {
+                    log::warn!("{e}");
+                    (StatusCode::BAD_REQUEST, e.to_string())
+                }
+
+                // View errors that are clearly internal should be treated as
+                // internal failures. Simpler view errors (missing view,
+                // args) are treated as client errors above via `Subscription`/
+                // other mappings; fall back to internal for safety.
+                _ => {
+                    log::error!("internal sql execution error: {e}");
+                    (StatusCode::INTERNAL_SERVER_ERROR, "internal server error".to_string())
+                }
+            }
         })?;
 
         let total_duration = sql_start.elapsed();

crates/client-api/src/routes/database.rs

@@ -139,9 +139,12 @@ fn map_procedure_error(e: ProcedureCallError, procedure: &str) -> (StatusCode, S
             log::info!("Procedure {procedure} could not run because the module is out of energy");
             StatusCode::PAYMENT_REQUIRED
         }
+        ProcedureCallError::GuestPanic(_) => {
+            log::debug!("Guest error while invoking procedure {procedure}: {e:#}");
+            StatusCode::INTERNAL_SERVER_ERROR
+        }
         ProcedureCallError::InternalError(_) => {
-            // TODO: May need to split this from module errors vs host errors
-            log::info!("Internal error while invoking procedure {procedure}: {e:#}");
+            log::error!("Internal error while invoking procedure {procedure}: {e:#}");
             StatusCode::INTERNAL_SERVER_ERROR
         }
     };
@@ -2454,6 +2457,21 @@ mod tests {
         remove_http_response_size_metric(database_identity);
     }
 
+    #[test]
+    fn test_map_procedure_error_distinguishes_guest_panic_and_internal_error() {
+        let (status, msg) = map_procedure_error(
+            ProcedureCallError::GuestPanic("panic in guest".to_string()),
+            "my_proc",
+        );
+        assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR);
+        assert!(msg.contains("panic in guest"));
+
+        let (status, msg) = map_procedure_error(
+            ProcedureCallError::InternalError("host failure".to_string()),
+            "my_proc",
+        );
+        assert_eq!(status, StatusCode::INTERNAL_SERVER_ERROR);
+        assert!(msg.contains("host failure"));
     fn root_router(root_routes: RootRoutes<DummyState>) -> axum::Router {
         let state = DummyState::new();
         router_with_root_routes(

crates/core/src/host/module_host.rs

@@ -1662,6 +1662,8 @@ pub enum ProcedureCallError {
     #[error("Procedure terminated due to insufficient budget")]
     OutOfEnergy,
     #[error("The module instance encountered a fatal error: {0}")]
+    GuestPanic(String),
+    #[error("The procedure call encountered an internal error: {0}")]
     InternalError(String),
 }
 

crates/core/src/host/wasm_common/module_host_actor.rs

@@ -841,7 +841,7 @@ impl InstanceCommon {
                 //     return Err(ProcedureCallError::OutOfEnergy);
                 // } else
                 {
-                    Err(ProcedureCallError::InternalError(format!("{err}")))
+                    Err(ProcedureCallError::GuestPanic(format!("{err}")))
                 }
             }
             Ok(return_val) => {