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

libsql: improve hrana api error message #1863

Merged
merged 2 commits into from
Dec 4, 2024
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
Original file line number Diff line number Diff line change
@@ -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"}``
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
183 changes: 181 additions & 2 deletions libsql-server/tests/hrana/batch.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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, Fut>(f: F)
where
F: Fn(Arc<Notify>, Database) -> Fut + 'static,
Fut: std::future::Future<Output = Result<(), libsql::Error>> + '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();
}
Original file line number Diff line number Diff line change
@@ -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``
Original file line number Diff line number Diff line change
@@ -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"}``
Original file line number Diff line number Diff line change
@@ -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``
Original file line number Diff line number Diff line change
@@ -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``
Original file line number Diff line number Diff line change
@@ -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``
Original file line number Diff line number Diff line change
@@ -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"}``
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
Original file line number Diff line number Diff line change
@@ -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\"}",
),
)
5 changes: 3 additions & 2 deletions libsql/src/hrana/hyper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ByteStream> = if resp.is_end_stream() {
Expand Down
6 changes: 6 additions & 0 deletions libsql/src/rows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn RowInner + Send + Sync>,
Expand Down
Loading