diff --git a/objectstore-server/src/endpoints/batch.rs b/objectstore-server/src/endpoints/batch.rs index b62219fa..11ff1091 100644 --- a/objectstore-server/src/endpoints/batch.rs +++ b/objectstore-server/src/endpoints/batch.rs @@ -232,7 +232,10 @@ async fn got_to_part( .meter_stream(stream, context) .try_collect::() .await - .map_err(|e| ApiError::Service(e.into()))? + .map_err(|e| { + objectstore_log::error!(!!&e, "failed to collect payload stream"); + ApiError::Service(e.into()) + })? .freeze(); let mut metadata_headers = metadata.to_headers("").map_err(|err| { @@ -317,6 +320,9 @@ fn create_success_part( } fn create_error_part(idx: usize, error: &ApiError) -> Part { + // Capture the error explicitly, as it is not converted via `IntoResponse` here. + error.capture(); + let mut headers = HeaderMap::new(); insert_index_header(&mut headers, idx); insert_status_header(&mut headers, error.status()); diff --git a/objectstore-server/src/endpoints/common.rs b/objectstore-server/src/endpoints/common.rs index df33f90e..97468764 100644 --- a/objectstore-server/src/endpoints/common.rs +++ b/objectstore-server/src/endpoints/common.rs @@ -79,7 +79,6 @@ impl ApiError { ApiError::Batch(BatchError::LimitExceeded(_)) => StatusCode::PAYLOAD_TOO_LARGE, ApiError::Batch(BatchError::RateLimited) => StatusCode::TOO_MANY_REQUESTS, ApiError::Batch(BatchError::ResponseSerialization { .. }) => { - objectstore_log::error!(!!self, "error serializing batch response"); StatusCode::INTERNAL_SERVER_ERROR } @@ -89,10 +88,7 @@ impl ApiError { ApiError::Auth(AuthError::UnknownKey) => StatusCode::UNAUTHORIZED, ApiError::Auth(AuthError::UnsupportedPresignedMethod) => StatusCode::FORBIDDEN, ApiError::Auth(AuthError::NotPermitted) => StatusCode::FORBIDDEN, - ApiError::Auth(AuthError::InternalError(_)) => { - objectstore_log::error!(!!self, "auth system error"); - StatusCode::INTERNAL_SERVER_ERROR - } + ApiError::Auth(AuthError::InternalError(_)) => StatusCode::INTERNAL_SERVER_ERROR, ApiError::Service(ServiceError::Client(_)) => StatusCode::BAD_REQUEST, ApiError::Service(ServiceError::Metadata(_)) => StatusCode::BAD_REQUEST, @@ -102,21 +98,31 @@ impl ApiError { ApiError::Service(ServiceError::InvalidUploadId(_)) => StatusCode::BAD_REQUEST, ApiError::Service(ServiceError::AtCapacity) => StatusCode::TOO_MANY_REQUESTS, ApiError::Service(ServiceError::NotImplemented) => StatusCode::NOT_IMPLEMENTED, - ApiError::Service(_) => { - objectstore_log::error!(!!self, "error handling request"); - StatusCode::INTERNAL_SERVER_ERROR - } + ApiError::Service(_) => StatusCode::INTERNAL_SERVER_ERROR, - ApiError::Internal(_) => { - objectstore_log::error!(!!self, "internal error"); - StatusCode::INTERNAL_SERVER_ERROR - } + ApiError::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR, + } + } + + /// Reports this error to error tracking if it indicates a server fault (5xx status). + /// + /// Call this exactly once wherever an `ApiError` is serialized into a client-visible + /// response: standalone responses ([`IntoResponse`]) and batch response parts. + pub fn capture(&self) { + // Captured at the source in the service layer to prevent double-logging. + if matches!(self, ApiError::Service(_)) { + return; + } + + if self.status().is_server_error() { + objectstore_log::error!(!!self, "error handling request"); } } } impl IntoResponse for ApiError { fn into_response(self) -> Response { + self.capture(); let body = ApiErrorResponse::from_error(&self); (self.status(), Json(body)).into_response() } diff --git a/objectstore-service/src/concurrency.rs b/objectstore-service/src/concurrency.rs index a1602f5b..ac9bb416 100644 --- a/objectstore-service/src/concurrency.rs +++ b/objectstore-service/src/concurrency.rs @@ -193,7 +193,11 @@ where } .bind_hub(new_hub), ); - rx.await.map_err(|_| Error::Dropped)? + + rx.await.map_err(|_| { + objectstore_log::error!(!!&Error::Dropped, operation, "Task failed"); + Error::Dropped + })? } #[cfg(test)]