Skip to content

Commit

Permalink
Return the formatted error from log_and_500, so the CLI can report …
Browse files Browse the repository at this point in the history
…it (#338)

In the client API's log_and_500,
return an ErrorResponse with a body derived from the error,
rather than a bare 500 code.

Note that this exposes previously-internal error messages to users.
  • Loading branch information
gefjon authored Sep 28, 2023
1 parent 205ad6b commit 0a3a9d6
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
5 changes: 3 additions & 2 deletions crates/client-api/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::sync::Arc;

use async_trait::async_trait;
use axum::response::ErrorResponse;
use http::StatusCode;

use spacetimedb::address::Address;
Expand Down Expand Up @@ -477,7 +478,7 @@ impl<T: NodeDelegate + ?Sized> NodeDelegate for Arc<T> {
}
}

pub fn log_and_500(e: impl std::fmt::Display) -> StatusCode {
pub fn log_and_500(e: impl std::fmt::Display) -> ErrorResponse {
log::error!("internal error: {e:#}");
StatusCode::INTERNAL_SERVER_ERROR
(StatusCode::INTERNAL_SERVER_ERROR, format!("{e:#}")).into()
}
17 changes: 6 additions & 11 deletions crates/client-api/src/routes/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ fn mime_ndjson() -> mime::Mime {
async fn worker_ctx_find_database(
worker_ctx: &(impl ControlStateDelegate + ?Sized),
address: &Address,
) -> Result<Option<Database>, StatusCode> {
) -> axum::response::Result<Option<Database>> {
worker_ctx.get_database_by_address(address).map_err(log_and_500)
}

Expand Down Expand Up @@ -603,19 +603,14 @@ pub struct RegisterTldParams {
tld: String,
}

fn auth_or_bad_request(auth: SpacetimeAuthHeader) -> axum::response::Result<SpacetimeAuth> {
auth.get()
.ok_or((StatusCode::BAD_REQUEST, "Invalid credentials.").into())
}

pub async fn register_tld<S: ControlStateDelegate>(
State(ctx): State<S>,
Query(RegisterTldParams { tld }): Query<RegisterTldParams>,
auth: SpacetimeAuthHeader,
) -> axum::response::Result<impl IntoResponse> {
// You should not be able to publish to a database that you do not own
// so, unless you are the owner, this will fail, hence not using get_or_create
let auth = auth_or_bad_request(auth)?;
let auth = auth_or_unauth(auth)?;

let tld = tld.parse::<DomainName>().map_err(DomainParsingRejection)?.into();
let result = ctx.register_tld(&auth.identity, tld).await.map_err(log_and_500)?;
Expand Down Expand Up @@ -750,7 +745,7 @@ pub async fn publish<S: NodeDelegate + ControlStateDelegate>(

// You should not be able to publish to a database that you do not own
// so, unless you are the owner, this will fail.
let auth = auth_or_bad_request(auth)?;
let auth = auth_or_unauth(auth)?;

let (db_addr, db_name) = match name_or_address {
Some(noa) => match noa.try_resolve(&ctx).await? {
Expand Down Expand Up @@ -838,7 +833,7 @@ pub async fn delete_database<S: ControlStateDelegate>(
Path(DeleteDatabaseParams { address }): Path<DeleteDatabaseParams>,
auth: SpacetimeAuthHeader,
) -> axum::response::Result<impl IntoResponse> {
let auth = auth_or_bad_request(auth)?;
let auth = auth_or_unauth(auth)?;

ctx.delete_database(&auth.identity, &address)
.await
Expand All @@ -858,7 +853,7 @@ pub async fn set_name<S: ControlStateDelegate>(
Query(SetNameQueryParams { domain, address }): Query<SetNameQueryParams>,
auth: SpacetimeAuthHeader,
) -> axum::response::Result<impl IntoResponse> {
let auth = auth_or_bad_request(auth)?;
let auth = auth_or_unauth(auth)?;

let database = ctx
.get_database_by_address(&address)
Expand All @@ -874,7 +869,7 @@ pub async fn set_name<S: ControlStateDelegate>(
.create_dns_record(&auth.identity, &domain, &address)
.await
.map_err(|err| match err {
spacetimedb::control_db::Error::RecordAlreadyExists(_) => StatusCode::CONFLICT,
spacetimedb::control_db::Error::RecordAlreadyExists(_) => StatusCode::CONFLICT.into(),
_ => log_and_500(err),
})?;

Expand Down
3 changes: 3 additions & 0 deletions crates/standalone/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ impl spacetimedb_client_api::ControlStateWriteAccess for StandaloneEnv {
return Ok(());
};
if &database.identity != identity {
// TODO: `PermissionDenied` should be a variant of `Error`,
// so we can match on it and return better error responses
// from HTTP endpoints.
return Err(anyhow!(
"Permission denied: `{}` does not own database `{}`",
identity.to_hex(),
Expand Down

0 comments on commit 0a3a9d6

Please sign in to comment.