Skip to content

Commit 65c4185

Browse files
authored
[ENH]: (Rust client) parse and return API errors (#5656)
## Description of changes Prior to this change, responses containing error messages would never be visible. ## Test plan _How are these changes tested?_ - [X] Tests pass locally with `pytest` for python, `yarn test` for js, `cargo test` for rust ## Migration plan _Are there any migrations, or any forwards/backwards compatibility changes needed in order to make sure this change deploys reliably?_ ## Observability plan _What is the plan to instrument and monitor this change?_ ## Documentation Changes _Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the [docs section](https://github.com/chroma-core/chroma/tree/main/docs/docs.trychroma.com)?_
1 parent bf8e462 commit 65c4185

File tree

3 files changed

+70
-1
lines changed

3 files changed

+70
-1
lines changed

rust/api-types/src/error.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
use serde::{Deserialize, Serialize};
2+
3+
#[derive(Serialize, Deserialize, Debug, Clone)]
4+
pub struct ErrorResponse {
5+
pub error: String,
6+
pub message: String,
7+
}

rust/api-types/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1+
mod error;
12
mod heartbeat;
23
mod user_identity;
34

5+
pub use error::*;
46
pub use heartbeat::*;
57
pub use user_identity::*;

rust/chroma/src/client/chroma_client.rs

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use backon::ExponentialBuilder;
22
use backon::Retryable;
3+
use chroma_api_types::ErrorResponse;
34
use chroma_error::ChromaValidationError;
45
use chroma_types::Collection;
56
use parking_lot::Mutex;
@@ -25,6 +26,8 @@ const USER_AGENT: &str = concat!(
2526
pub enum ChromaClientError {
2627
#[error("Request error: {0:?}")]
2728
RequestError(#[from] reqwest::Error),
29+
#[error("API error: {0:?} ({1})")]
30+
ApiError(String, reqwest::StatusCode),
2831
#[error("Could not resolve database ID: {0}")]
2932
CouldNotResolveDatabaseId(String),
3033
#[error("Serialization/Deserialization error: {0}")]
@@ -397,7 +400,24 @@ impl ChromaClient {
397400
Ok(response) => response,
398401
Err((err, maybe_response)) => {
399402
if let Some(response) = maybe_response {
400-
let json = response.json::<serde_json::Value>().await?;
403+
let status = response.status();
404+
let text = response.text().await.unwrap_or_default();
405+
let json = match serde_json::from_str::<serde_json::Value>(&text) {
406+
Ok(json) => json,
407+
Err(_) => {
408+
tracing::trace!(
409+
url = %url,
410+
method =? method,
411+
"Received non-JSON error response: {}",
412+
text
413+
);
414+
415+
return Err(ChromaClientError::ApiError(
416+
format!("Non-JSON error response: {}", text),
417+
status,
418+
));
419+
}
420+
};
401421

402422
if tracing::enabled!(tracing::Level::TRACE) {
403423
tracing::trace!(
@@ -407,6 +427,13 @@ impl ChromaClient {
407427
serde_json::to_string_pretty(&json).unwrap_or_else(|_| "<failed to serialize>".to_string())
408428
);
409429
}
430+
431+
if let Ok(api_error) = serde_json::from_value::<ErrorResponse>(json) {
432+
return Err(ChromaClientError::ApiError(
433+
format!("{}: {}", api_error.error, api_error.message),
434+
status,
435+
));
436+
}
410437
}
411438

412439
return Err(ChromaClientError::RequestError(err));
@@ -617,6 +644,39 @@ mod tests {
617644
assert_eq!(mock.calls(), 2);
618645
}
619646

647+
#[tokio::test]
648+
#[test_log::test]
649+
async fn test_live_cloud_parses_error() {
650+
with_client(|client| async move {
651+
client
652+
.create_collection(
653+
CreateCollectionRequest::builder()
654+
.name("foo".to_string())
655+
.build(),
656+
)
657+
.await
658+
.unwrap();
659+
660+
let err = client
661+
.create_collection(
662+
CreateCollectionRequest::builder()
663+
.name("foo".to_string())
664+
.build(),
665+
)
666+
.await
667+
.unwrap_err();
668+
669+
match err {
670+
ChromaClientError::ApiError(msg, status) => {
671+
assert_eq!(status, StatusCode::CONFLICT);
672+
assert!(msg.contains("already exists"));
673+
}
674+
_ => panic!("Expected ApiError"),
675+
};
676+
})
677+
.await;
678+
}
679+
620680
#[tokio::test]
621681
#[test_log::test]
622682
async fn test_live_cloud_list_collections() {

0 commit comments

Comments
 (0)