Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return the formatted error from log_and_500, so the CLI can report it #338

Merged
merged 1 commit into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading