diff --git a/crates/client-api/src/lib.rs b/crates/client-api/src/lib.rs index e8999e1e617..44777e94424 100644 --- a/crates/client-api/src/lib.rs +++ b/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(); diff --git a/crates/client-api/src/routes/database.rs b/crates/client-api/src/routes/database.rs index f170f9b290e..979d2ea5971 100644 --- a/crates/client-api/src/routes/database.rs +++ b/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) -> axum::Router { let state = DummyState::new(); router_with_root_routes( diff --git a/crates/core/src/host/module_host.rs b/crates/core/src/host/module_host.rs index 25eb09e6382..17155d22c1f 100644 --- a/crates/core/src/host/module_host.rs +++ b/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), } diff --git a/crates/core/src/host/wasm_common/module_host_actor.rs b/crates/core/src/host/wasm_common/module_host_actor.rs index aee984fe3a5..5098d5acf06 100644 --- a/crates/core/src/host/wasm_common/module_host_actor.rs +++ b/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) => {