diff --git a/AGENTS.md b/AGENTS.md index f919a01..4c262f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -27,7 +27,7 @@ cargo test # runs all non-ignored tests cargo test # substring match on test name cargo test -- --ignored # run tests marked #[ignore] (e.g. long-running datapoint tests) cargo test ::tests:: # e.g. `events::tests::test_events_full` -cargo test -- --nocapture # show println! from tests (the SDK prints response bodies) +cargo test -- --nocapture # show println! from tests cargo test --release # ALWAYS --release for anything timed (see below) ./run_python_tests.sh # Python-bindings suite (rebuilds the PyO3 module first — see below) ``` @@ -206,7 +206,7 @@ Every subservice implements `ApiServiceProvider`, which owns the HTTP plumbing: ### Response shape: `DataWrapper` -The API wraps collections in `{ "items": [...] }`. `DataWrapper` mirrors that and carries the HTTP status code + raw error body alongside items. Deserialization goes through the `DataWrapperDeserialization` trait, which tolerates 204/empty bodies and stores non-2xx bodies in `error_body` instead of failing. When adding new endpoint methods, return `Result, ResponseError>`. +The API wraps collections in `{ "items": [...] }`. `DataWrapper` mirrors that and carries the HTTP status code alongside items. Deserialization goes through the `DataWrapperDeserialization` trait, which tolerates 204/empty bodies; it only ever sees 2xx responses, since `process_response` turns everything else into a `ResponseError`. When adding new endpoint methods, return `Result, ResponseError>`. ### Entity → request-body conversion @@ -567,5 +567,5 @@ under another test, and use a fixed *pair* when a test has to tell two labels ap - `#[serde(rename = "camelCase")]` or explicit `#[serde(rename = "...")]` on fields — the backend is camelCase, Rust is snake_case. - **A request body naming a field the api does not have is a 400.** Jackson used to drop unknown properties, so a stale or misspelled key was answered with 200 and no effect; a strict converter now rejects the body and names every offender alongside the fields the endpoint accepts. Two consequences for this SDK: a struct that doubles as request *and* response must `#[serde(skip_serializing)]` its response-only fields — `GraphDataWrapper`'s `errorBody`/`httpStatusCode` reached `/resources/create` and made every resource and function create and update a 400 — and one Rust type may not stand in for two endpoints that disagree on their fields (see the search forms above). Reading is unaffected: responses stay lenient in both directions. - `externalId` (string, user-supplied) and numeric `id` are both valid identifiers across the API. `IdAndExtId` / `IdAndExtIdCollection` model this choice. -- `process_response` (`src/http.rs`) prints response bodies to stdout (truncated to 2000 chars). This is deliberate for debugging — don't silently remove it. +- The SDK does not print. Everything a caller could want to see is in the returned value or the `ResponseError`, and a library writing to stdout or stderr cannot be silenced by the application embedding it. - Tests that depend on backend state being empty are brittle; recent fixes moved away from exact-count assertions (see commit `7f0a059`). Don't add new ones. diff --git a/src/files/mod.rs b/src/files/mod.rs index 810d0fa..69659e7 100644 --- a/src/files/mod.rs +++ b/src/files/mod.rs @@ -157,13 +157,10 @@ impl FileService { let file_name = filename_from_content_disposition(&response); let mime_type = header_value(&response, reqwest::header::CONTENT_TYPE); let status = response.status(); - let bytes = response.bytes().await.map_err(|err| { - eprintln!("Failed to read download body: {}", err); - ResponseError { - status, - message: err.to_string(), - content_type: None, - } + let bytes = response.bytes().await.map_err(|err| ResponseError { + status, + message: err.to_string(), + content_type: None, })?; Ok(FileDownload { @@ -194,13 +191,10 @@ impl FileService { let mut file = File::create(destination.as_ref()).await.map_err(io_error)?; let mut written: u64 = 0; - while let Some(chunk) = response.chunk().await.map_err(|err| { - eprintln!("Download stream failed: {}", err); - ResponseError { - status, - message: err.to_string(), - content_type: None, - } + while let Some(chunk) = response.chunk().await.map_err(|err| ResponseError { + status, + message: err.to_string(), + content_type: None, })? { file.write_all(&chunk).await.map_err(io_error)?; written += chunk.len() as u64; @@ -424,14 +418,8 @@ impl FileUpload { let kind: Option = match infer::get_from_path(file_path) { Ok(Some(file_type)) => Some(file_type.mime_type().to_string()), - Ok(None) => { - println!("Could not determine file type for: {}", file_path); - Some("application/octet-stream".to_string()) - } - Err(e) => { - eprintln!("Error detecting file type for {}: {}", file_path, e); - None - } + Ok(None) => Some("application/octet-stream".to_string()), + Err(_) => None, }; Ok(Self { diff --git a/src/generic.rs b/src/generic.rs index 88c7354..ecbb05d 100644 --- a/src/generic.rs +++ b/src/generic.rs @@ -743,10 +743,7 @@ pub trait ApiServiceProvider { .query(param) .send() .await - .map_err(|err| { - eprintln!("HTTP request failed: {}", err); - ResponseError::from_err(err) - })? + .map_err(ResponseError::from_err)? } else { self.get_api_service() .http_client @@ -754,12 +751,9 @@ pub trait ApiServiceProvider { .bearer_auth(token.clone()) .send() .await - .map_err(|err| { - eprintln!("HTTP request failed: {}", err); - ResponseError::from_err(err) - })? + .map_err(ResponseError::from_err)? }; - match process_response::(response, path).await { + match process_response::(response).await { Ok(value) => Ok(value), Err(e) => Err(self.on_request_error(e, &token).await), } @@ -782,14 +776,10 @@ pub trait ApiServiceProvider { .bearer_auth(token.clone()) .send() .await - .map_err(|err| { - eprintln!("HTTP request failed: {}", err); - ResponseError::from_err(err) - })?; + .map_err(ResponseError::from_err)?; if response.status() == 204 { // Return deserialized `T` with an empty body and the HTTP status code T::deserialize_and_set_status("", response.status().as_u16()).map_err(|err| { - eprintln!("Failed to create object from empty response: {}", err); ResponseError { status: response.status(), message: err.to_string(), @@ -797,7 +787,7 @@ pub trait ApiServiceProvider { } }) } else { - match process_response::(response, path).await { + match process_response::(response).await { Ok(value) => Ok(value), Err(e) => Err(self.on_request_error(e, &token).await), } @@ -820,11 +810,8 @@ pub trait ApiServiceProvider { .bearer_auth(token.clone()) .send() .await - .map_err(|err| { - eprintln!("HTTP request failed: {}", err); - ResponseError::from_err(err) - })?; - match process_response::(response, path).await { + .map_err(ResponseError::from_err)?; + match process_response::(response).await { Ok(value) => Ok(value), Err(e) => Err(self.on_request_error(e, &token).await), } @@ -852,11 +839,8 @@ pub trait ApiServiceProvider { request = request.header(name, value); } - let response = request.send().await.map_err(|err| { - eprintln!("HTTP file upload request failed: {}", err); - ResponseError::from_err(err) - })?; - match process_response::(response, path).await { + let response = request.send().await.map_err(ResponseError::from_err)?; + match process_response::(response).await { Ok(value) => Ok(value), Err(e) => Err(self.on_request_error(e, &token).await), } @@ -882,10 +866,7 @@ pub trait ApiServiceProvider { .bearer_auth(token.clone()) .send() .await - .map_err(|err| { - eprintln!("HTTP request failed: {}", err); - ResponseError::from_err(err) - })?; + .map_err(ResponseError::from_err)?; if response.status() == 204 { return T::deserialize_and_set_status("", response.status().as_u16()).map_err(|err| { ResponseError { @@ -895,7 +876,7 @@ pub trait ApiServiceProvider { } }); } - match process_response::(response, path).await { + match process_response::(response).await { Ok(value) => Ok(value), Err(e) => Err(self.on_request_error(e, &token).await), } @@ -904,10 +885,9 @@ pub trait ApiServiceProvider { /// `GET` an endpoint that answers with bytes rather than JSON (currently only /// `/files/download/{id}`). /// - /// The body is *not* passed through [`process_response`] — there is no `DataWrapper` to - /// deserialize and no reason to print a binary body to stdout. Non-2xx responses still surface - /// as a [`ResponseError`] carrying the server's text explanation, so error handling matches the - /// JSON helpers. + /// The body is *not* passed through `process_response` — there is no `DataWrapper` to + /// deserialize. Non-2xx responses still surface as a [`ResponseError`] carrying the server's + /// text explanation, so error handling matches the JSON helpers. async fn execute_get_stream_request( &self, path: &str, @@ -921,10 +901,7 @@ pub trait ApiServiceProvider { .header(http::header::ACCEPT, "*/*") .send() .await - .map_err(|err| { - eprintln!("HTTP request failed: {}", err); - ResponseError::from_err(err) - })?; + .map_err(ResponseError::from_err)?; let status = response.status(); if status.is_success() { @@ -937,7 +914,6 @@ pub trait ApiServiceProvider { if status == http::StatusCode::UNAUTHORIZED { self.get_api_service().config.invalidate_token().await; } - eprintln!("Request failed with status: {status}"); // Read the header before the body: `text()` consumes the response. let content_type = response .headers() @@ -1045,48 +1021,15 @@ where DataWrapper: Sized, { fn deserialize_and_set_status(body: &str, status_code: u16) -> Result { - if status_code >= 200 && status_code < 300 { - if status_code == 204 || body.is_empty() { - // HTTP No content doesnt return anything - let mut wrapper: DataWrapper = DataWrapper::new(); - wrapper.set_http_status_code(status_code); - return Ok(wrapper); - } - // For 2xx responses, we expect the body to be a valid DataWrapper - // If body is empty, it's fine for `from_str` to fail and return an error - // Or, if you specifically want an empty wrapper for 2xx with empty body: - // let mut wrapper = DataWrapper::new(); - // wrapper.set_http_status_code(status_code); - // return Ok(wrapper); - // However, typically a successful response with a body should be parsed. - serde_json::from_str(body).map(|mut wrapper: DataWrapper| { - wrapper.set_http_status_code(status_code); - wrapper - }) - } else { - // For non-2xx responses (errors) - eprintln!( - "HTTP request failed with status code {}: {}", - status_code, body - ); - - // Attempt to deserialize the body into DataWrapper - // This is useful if the error response *itself* is a structured JSON, - // for example, containing an error object. - match serde_json::from_str(body).map(|mut wrapper: DataWrapper| { - wrapper.set_http_status_code(status_code); // Set the HTTP status code - wrapper // Return the modified wrapper - }) { - Ok(result) => Ok(result), - Err(_) => { - eprintln!("Error parsing HTTP response body: {}", body); - let mut wrapper: DataWrapper = DataWrapper::new(); - wrapper.error_body = Some(body.to_string()); - wrapper.set_http_status_code(status_code); - Ok(wrapper) - } - } + if status_code == 204 || body.is_empty() { + let mut wrapper: DataWrapper = DataWrapper::new(); + wrapper.set_http_status_code(status_code); + return Ok(wrapper); } + serde_json::from_str(body).map(|mut wrapper: DataWrapper| { + wrapper.set_http_status_code(status_code); + wrapper + }) } } diff --git a/src/graph_data_wrapper.rs b/src/graph_data_wrapper.rs index ee5b283..6853d67 100644 --- a/src/graph_data_wrapper.rs +++ b/src/graph_data_wrapper.rs @@ -67,40 +67,18 @@ impl DataWrapperDeserializ for GraphDataWrapper { fn deserialize_and_set_status(body: &str, status_code: u16) -> Result { - if status_code >= 200 && status_code < 300 { - if status_code == 204 || body.is_empty() { - return Ok(Self { - nodes: None, - relations: None, - error_body: None, - http_status_code: Some(status_code), - }); - } - serde_json::from_str(body).map(|mut wrapper: GraphDataWrapper| { - wrapper.set_http_status_code(status_code); - wrapper - }) - } else { - eprintln!( - "HTTP request failed with status code {}: {}", - status_code, body - ); - match serde_json::from_str(body).map(|mut wrapper: GraphDataWrapper| { - wrapper.set_http_status_code(status_code); - wrapper - }) { - Ok(result) => Ok(result), - Err(_) => { - eprintln!("Error parsing HTTP response body: {}", body); - Ok(GraphDataWrapper { - nodes: None, - relations: None, - error_body: Some(body.to_string()), - http_status_code: Some(status_code), - }) - } - } + if status_code == 204 || body.is_empty() { + return Ok(Self { + nodes: None, + relations: None, + error_body: None, + http_status_code: Some(status_code), + }); } + serde_json::from_str(body).map(|mut wrapper: GraphDataWrapper| { + wrapper.set_http_status_code(status_code); + wrapper + }) } } diff --git a/src/http.rs b/src/http.rs index 0c1f625..933d614 100644 --- a/src/http.rs +++ b/src/http.rs @@ -136,40 +136,23 @@ impl fmt::Display for ResponseError { } } -pub async fn process_response(response: Response, path: &str) -> Result +pub(crate) async fn process_response(response: Response) -> Result where T: DeserializeOwned + DataWrapperDeserialization, { let status = response.status(); if (200..300).contains(&status.as_u16()) { - // Read the response body and attempt to deserialize - let body = response.text().await.map_err(|err| { - eprintln!("Failed to read response body: {err}",); - ResponseError { - status, - message: err.to_string(), - content_type: None, - } - })?; - - let max_chars = 2000; - let truncated_body = &body[..body.len().min(max_chars)]; - println!("Response body for path: {}\n{}", path, &truncated_body); // Debug output - - // Conditionally apply custom or default logic - let result: T = T::deserialize_and_set_status(&body, status.as_u16()).map_err(|err| { - eprintln!("Failed to deserialize JSON: {err}",); - ResponseError { - status, - message: err.to_string(), - content_type: None, - } + let body = response.text().await.map_err(|err| ResponseError { + status, + message: err.to_string(), + content_type: None, })?; - - Ok(result) + T::deserialize_and_set_status(&body, status.as_u16()).map_err(|err| ResponseError { + status, + message: err.to_string(), + content_type: None, + }) } else { - let status = response.status(); - eprintln!("Request failed with status: {status}",); // Read the header before the body: `text()` consumes the response. let content_type = response .headers() diff --git a/src/resources/mod.rs b/src/resources/mod.rs index 2eca052..3eed3bd 100644 --- a/src/resources/mod.rs +++ b/src/resources/mod.rs @@ -10,7 +10,7 @@ use crate::generic::{ }; use crate::graph_data_wrapper::{GraphDataWrapper, GraphNode}; use crate::nodes::Node; -use crate::http::{process_response, ResponseError}; +use crate::http::ResponseError; use crate::relations::{EdgeProxy, RelForm, RelatedNode}; use crate::ApiService; use chrono::{DateTime, Utc}; diff --git a/src/timeseries/mod.rs b/src/timeseries/mod.rs index dba329c..e34231f 100644 --- a/src/timeseries/mod.rs +++ b/src/timeseries/mod.rs @@ -14,7 +14,7 @@ use crate::generic::{ }; use crate::filters::NodeFilter; use crate::relations::RelatedNode; -use crate::http::{process_response, ResponseError}; +use crate::http::ResponseError; use crate::serde_helper::is_zero; use crate::ApiService; use chrono::{DateTime, Utc}; @@ -392,7 +392,6 @@ impl TimeSeriesService { if total_datapoints > MAX_DATAPOINTS_PER_REQUEST { while total_datapoints > MAX_DATAPOINTS_PER_REQUEST { - println!("Total datapoints left: {}", total_datapoints); // Divide the request into multiple batch requests let mut new_json: DataWrapper> = DataWrapper::new(); @@ -409,7 +408,6 @@ impl TimeSeriesService { let batch_size: usize = MAX_DATAPOINTS_PER_REQUEST / active_timeseries_with_datapoints.len(); - println!("Current Batch size: {}", batch_size); if orig_dp_collection.datapoints.len() > batch_size { let chunk: Vec = orig_dp_collection.datapoints.drain(..batch_size).collect(); @@ -420,7 +418,6 @@ impl TimeSeriesService { .iter() .position(|&x| x == orig_dp_collection.hash()) { - println!("Remove datacollection: {}", orig_dp_collection.to_string()); active_timeseries_with_datapoints.remove(pos); } } else { @@ -434,18 +431,8 @@ impl TimeSeriesService { let moved_points = new_dp_collection.datapoints.len(); new_json.add_item(new_dp_collection); total_datapoints = total_datapoints - moved_points; - println!("Total datapoints left: {}", total_datapoints); } - let mut new_total_datapoints: usize = 0; - for dp_collection in new_json.get_items().iter() { - new_total_datapoints += dp_collection.datapoints.len(); - } - println!( - "Sending insert datapoints request with {} datapoints.", - new_total_datapoints - ); - new_request_bodies.push(new_json); } } @@ -468,12 +455,6 @@ impl TimeSeriesService { while let Some(result) = sends.next().await { result?; } - - total_datapoints = 0; - for dp_collection in json.get_items().iter() { - total_datapoints += dp_collection.datapoints.len(); - } - println!("Final request: Total datapoints left: {}", total_datapoints); self.execute_post_request::, _>(path, json) .await }