From d568c4afa231a642bbaa56aa3c42fffd310b8228 Mon Sep 17 00:00:00 2001 From: Lucio Franco Date: Mon, 2 Dec 2024 16:38:25 -0500 Subject: [PATCH 1/2] libsql: improve hrana api error message --- libsql/src/hrana/hyper.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libsql/src/hrana/hyper.rs b/libsql/src/hrana/hyper.rs index 7ebab81a6d..2f40586eeb 100644 --- a/libsql/src/hrana/hyper.rs +++ b/libsql/src/hrana/hyper.rs @@ -52,12 +52,13 @@ impl HttpSender { let resp = self.inner.request(req).await.map_err(HranaError::from)?; - if resp.status() != StatusCode::OK { + let status = resp.status(); + if status != StatusCode::OK { let body = hyper::body::to_bytes(resp.into_body()) .await .map_err(HranaError::from)?; let body = String::from_utf8(body.into()).unwrap(); - return Err(HranaError::Api(body)); + return Err(HranaError::Api(format!("status={}, body={}", status, body))); } let body: super::HttpBody = if resp.is_end_stream() { From 43776d20f56d24ba48e756c43fe716eeaec9e798 Mon Sep 17 00:00:00 2001 From: Lucio Franco Date: Mon, 2 Dec 2024 17:49:04 -0500 Subject: [PATCH 2/2] tests: add hrana baton tests --- .../tests__cluster__create_namespace.snap | 5 +- ...r__replication__replica_lazy_creation.snap | 3 +- libsql-server/tests/hrana/batch.rs | 183 +++++++++++++++++- ...r_restart_query_execute_invalid_baton.snap | 6 + ..._hrana__batch__server_restart_timeout.snap | 6 + ...art_txn_execute_execute_invalid_baton.snap | 6 + ...start_txn_execute_query_invalid_baton.snap | 6 + ...start_txn_query_execute_invalid_baton.snap | 6 + .../tests__hrana__batch__server_timeout.snap | 6 + ..._shared_schema__check_migration_perms.snap | 3 +- ...amespaces__shared_schema__disable_ddl.snap | 3 +- ...ts__standalone__attach__attach_auth-2.snap | 3 +- ...ts__standalone__attach__attach_auth-3.snap | 3 +- ...ests__standalone__attach__attach_auth.snap | 3 +- ...s__standalone__attach__attach_no_auth.snap | 3 +- libsql/src/rows.rs | 6 + 16 files changed, 240 insertions(+), 11 deletions(-) create mode 100644 libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_query_execute_invalid_baton.snap create mode 100644 libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_timeout.snap create mode 100644 libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_execute_invalid_baton.snap create mode 100644 libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_query_invalid_baton.snap create mode 100644 libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_query_execute_invalid_baton.snap create mode 100644 libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_timeout.snap diff --git a/libsql-server/tests/cluster/snapshots/tests__cluster__create_namespace.snap b/libsql-server/tests/cluster/snapshots/tests__cluster__create_namespace.snap index b6f4c09749..0b893a1fe0 100644 --- a/libsql-server/tests/cluster/snapshots/tests__cluster__create_namespace.snap +++ b/libsql-server/tests/cluster/snapshots/tests__cluster__create_namespace.snap @@ -1,5 +1,6 @@ --- -source: sqld/tests/cluster/mod.rs +source: libsql-server/tests/cluster/mod.rs expression: e.to_string() +snapshot_kind: text --- -Hrana: `api error: `{"error":"Namespace `foo` doesn't exist"}`` +Hrana: `api error: `status=404 Not Found, body={"error":"Namespace `foo` doesn't exist"}`` diff --git a/libsql-server/tests/cluster/snapshots/tests__cluster__replication__replica_lazy_creation.snap b/libsql-server/tests/cluster/snapshots/tests__cluster__replication__replica_lazy_creation.snap index 28177b1465..2e1240cb77 100644 --- a/libsql-server/tests/cluster/snapshots/tests__cluster__replication__replica_lazy_creation.snap +++ b/libsql-server/tests/cluster/snapshots/tests__cluster__replication__replica_lazy_creation.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/cluster/replication.rs expression: "conn.execute(\"create table test (x)\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Replicator error: Requested namespace doesn't exist\"}", + "status=404 Not Found, body={\"error\":\"Replicator error: Requested namespace doesn't exist\"}", ), ) diff --git a/libsql-server/tests/hrana/batch.rs b/libsql-server/tests/hrana/batch.rs index f521d6295f..5e5aad5482 100644 --- a/libsql-server/tests/hrana/batch.rs +++ b/libsql-server/tests/hrana/batch.rs @@ -1,12 +1,15 @@ +use std::sync::Arc; use std::time::Duration; use futures::StreamExt; -use insta::assert_json_snapshot; +use insta::{assert_json_snapshot, assert_snapshot}; use libsql::{params, Database}; +use libsql_server::config::UserApiConfig; use libsql_server::hrana_proto::{Batch, BatchStep, Stmt}; +use tokio::sync::Notify; use crate::common::http::Client; -use crate::common::net::TurmoilConnector; +use crate::common::net::{init_tracing, SimServer as _, TestServer, TurmoilConnector}; #[test] fn sample_request() { @@ -402,3 +405,179 @@ fn test_simulate_vector_index_load_from_dump() { sim.run().unwrap(); } + +#[test] +fn server_restart_query_execute_invalid_baton() { + server_restart(|notify, db| async move { + let conn = db.connect().unwrap(); + + conn.query("select 1;", ()).await.unwrap(); + + notify.notify_waiters(); + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + + let err = conn.execute("select 1;", ()).await.unwrap_err(); + assert_snapshot!(err); + + Ok(()) + }); +} + +#[test] +fn server_restart_txn_execute_execute_invalid_baton() { + server_restart(|notify, db| async move { + let conn = db.connect().unwrap(); + + let txn = conn.transaction().await.unwrap(); + + txn.execute("select 1;", ()).await.unwrap(); + + notify.notify_waiters(); + + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + + let err = txn.execute("select 1;", ()).await.unwrap_err(); + + assert_snapshot!(err); + + Ok(()) + }); +} + +#[test] +fn server_restart_txn_query_execute_invalid_baton() { + server_restart(|notify, db| async move { + let conn = db.connect().unwrap(); + + let txn = conn.transaction().await.unwrap(); + + txn.query("select 1;", ()).await.unwrap(); + + notify.notify_waiters(); + + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + + let err = txn.execute("select 1;", ()).await.unwrap_err(); + + assert_snapshot!(err); + + Ok(()) + }); +} + +#[test] +fn server_restart_txn_execute_query_invalid_baton() { + server_restart(|notify, db| async move { + let conn = db.connect().unwrap(); + + let txn = conn.transaction().await.unwrap(); + + txn.execute("select 1;", ()).await.unwrap(); + + notify.notify_waiters(); + + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + + let err = txn.query("select 1;", ()).await.unwrap_err(); + + assert_snapshot!(err); + + Ok(()) + }); +} + +#[test] +fn server_restart_execute_query() { + server_restart(|notify, db| async move { + let conn = db.connect().unwrap(); + conn.execute("select 1;", ()).await.unwrap(); + + notify.notify_waiters(); + + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + + conn.query("select 1;", ()).await.unwrap(); + + Ok(()) + }); +} + +#[test] +fn server_timeout() { + server_restart(|_notify, db| async move { + let conn = db.connect().unwrap(); + conn.query("select 1;", ()).await.unwrap(); + + tokio::time::sleep(std::time::Duration::from_secs(20)).await; + + let err = conn.execute("select 1;", ()).await.unwrap_err(); + + assert_snapshot!(err); + + let conn = db.connect().unwrap(); + conn.execute("select 1;", ()).await.unwrap(); + + tokio::time::sleep(std::time::Duration::from_secs(20)).await; + + conn.execute("select 1;", ()).await.unwrap(); + + Ok(()) + }); +} + +#[track_caller] +fn server_restart(f: F) +where + F: Fn(Arc, Database) -> Fut + 'static, + Fut: std::future::Future> + 'static, +{ + let mut sim = turmoil::Builder::new() + .simulation_duration(Duration::from_secs(1000)) + .build(); + + init_tracing(); + + let notify = Arc::new(Notify::new()); + + let notify_clone = notify.clone(); + + sim.host("primary", move || { + let notify = notify.clone(); + async move { + let tmp = tempfile::tempdir()?; + + let make_server = || TestServer { + path: tmp.path().to_owned().into(), + user_api_config: UserApiConfig { + hrana_ws_acceptor: None, + ..Default::default() + }, + ..Default::default() + }; + + let server = make_server(); + + tokio::select! { + res = server.start_sim(8080) => { + res.unwrap() + } + _ = notify.notified() => (), + } + + let server = make_server(); + server.start_sim(8080).await.unwrap(); + + Ok(()) + } + }); + + sim.client("client", async move { + let db = Database::open_remote_with_connector("http://primary:8080", "", TurmoilConnector)?; + + f(notify_clone, db).await.unwrap(); + + Ok(()) + }); + + sim.run().unwrap(); +} diff --git a/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_query_execute_invalid_baton.snap b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_query_execute_invalid_baton.snap new file mode 100644 index 0000000000..8e1bc4db9b --- /dev/null +++ b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_query_execute_invalid_baton.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/hrana/batch.rs +expression: err +snapshot_kind: text +--- +Hrana: `api error: `status=400 Bad Request, body=Received an invalid baton`` diff --git a/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_timeout.snap b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_timeout.snap new file mode 100644 index 0000000000..edff7f3dab --- /dev/null +++ b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_timeout.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/hrana/batch.rs +expression: err +snapshot_kind: text +--- +Hrana: `api error: `status=400 Bad Request, body={"message":"The stream has expired due to inactivity","code":"STREAM_EXPIRED"}`` diff --git a/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_execute_invalid_baton.snap b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_execute_invalid_baton.snap new file mode 100644 index 0000000000..8e1bc4db9b --- /dev/null +++ b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_execute_invalid_baton.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/hrana/batch.rs +expression: err +snapshot_kind: text +--- +Hrana: `api error: `status=400 Bad Request, body=Received an invalid baton`` diff --git a/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_query_invalid_baton.snap b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_query_invalid_baton.snap new file mode 100644 index 0000000000..8e1bc4db9b --- /dev/null +++ b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_execute_query_invalid_baton.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/hrana/batch.rs +expression: err +snapshot_kind: text +--- +Hrana: `api error: `status=400 Bad Request, body=Received an invalid baton`` diff --git a/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_query_execute_invalid_baton.snap b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_query_execute_invalid_baton.snap new file mode 100644 index 0000000000..8e1bc4db9b --- /dev/null +++ b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_restart_txn_query_execute_invalid_baton.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/hrana/batch.rs +expression: err +snapshot_kind: text +--- +Hrana: `api error: `status=400 Bad Request, body=Received an invalid baton`` diff --git a/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_timeout.snap b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_timeout.snap new file mode 100644 index 0000000000..edff7f3dab --- /dev/null +++ b/libsql-server/tests/hrana/snapshots/tests__hrana__batch__server_timeout.snap @@ -0,0 +1,6 @@ +--- +source: libsql-server/tests/hrana/batch.rs +expression: err +snapshot_kind: text +--- +Hrana: `api error: `status=400 Bad Request, body={"message":"The stream has expired due to inactivity","code":"STREAM_EXPIRED"}`` diff --git a/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__check_migration_perms.snap b/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__check_migration_perms.snap index 5e0da980ca..b37dfbe157 100644 --- a/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__check_migration_perms.snap +++ b/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__check_migration_perms.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/namespaces/shared_schema.rs expression: "conn.execute(\"create table test (x)\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Authorization forbidden: Current session doesn't not have Write permission to namespace schema\"}", + "status=403 Forbidden, body={\"error\":\"Authorization forbidden: Current session doesn't not have Write permission to namespace schema\"}", ), ) diff --git a/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__disable_ddl.snap b/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__disable_ddl.snap index 2a001980a4..1d67598215 100644 --- a/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__disable_ddl.snap +++ b/libsql-server/tests/namespaces/snapshots/tests__namespaces__shared_schema__disable_ddl.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/namespaces/shared_schema.rs expression: "conn.execute(\"create table test (x)\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Authorization forbidden: DDL statements not permitted on namespace ns1\"}", + "status=403 Forbidden, body={\"error\":\"Authorization forbidden: DDL statements not permitted on namespace ns1\"}", ), ) diff --git a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-2.snap b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-2.snap index 0adc6a4392..4c3adbf63b 100644 --- a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-2.snap +++ b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-2.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/standalone/attach.rs expression: "txn.execute(\"ATTACH DATABASE bar as bar\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Authorization forbidden: Current session doesn't not have AttachRead permission to namespace bar\"}", + "status=403 Forbidden, body={\"error\":\"Authorization forbidden: Current session doesn't not have AttachRead permission to namespace bar\"}", ), ) diff --git a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-3.snap b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-3.snap index 3045a81510..71a6c31db4 100644 --- a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-3.snap +++ b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth-3.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/standalone/attach.rs expression: "bar_conn.execute(\"ATTACH foo as foo\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Authorization forbidden: Namespace `foo` doesn't allow attach\"}", + "status=403 Forbidden, body={\"error\":\"Authorization forbidden: Namespace `foo` doesn't allow attach\"}", ), ) diff --git a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth.snap b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth.snap index 99cacae60e..81fff11b53 100644 --- a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth.snap +++ b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_auth.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/standalone/attach.rs expression: "bar_conn.execute(\"ATTACH foo as foo\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Authorization forbidden: Current session doesn't not have AttachRead permission to namespace foo\"}", + "status=403 Forbidden, body={\"error\":\"Authorization forbidden: Current session doesn't not have AttachRead permission to namespace foo\"}", ), ) diff --git a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_no_auth.snap b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_no_auth.snap index 3045a81510..71a6c31db4 100644 --- a/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_no_auth.snap +++ b/libsql-server/tests/standalone/snapshots/tests__standalone__attach__attach_no_auth.snap @@ -1,9 +1,10 @@ --- source: libsql-server/tests/standalone/attach.rs expression: "bar_conn.execute(\"ATTACH foo as foo\", ()).await.unwrap_err()" +snapshot_kind: text --- Hrana( Api( - "{\"error\":\"Authorization forbidden: Namespace `foo` doesn't allow attach\"}", + "status=403 Forbidden, body={\"error\":\"Authorization forbidden: Namespace `foo` doesn't allow attach\"}", ), ) diff --git a/libsql/src/rows.rs b/libsql/src/rows.rs index a10d82b827..39411b8373 100644 --- a/libsql/src/rows.rs +++ b/libsql/src/rows.rs @@ -89,6 +89,12 @@ impl Rows { } } +impl fmt::Debug for Rows { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("Rows").finish() + } +} + /// A libsql row. pub struct Row { pub(crate) inner: Box,