diff options
author | Florian Klink <flokli@flokli.de> | 2024-03-08T21·04+0200 |
---|---|---|
committer | clbot <clbot@tvl.fyi> | 2024-03-09T05·48+0000 |
commit | bd1def3ec4dce82fb069815ab5de29f359bae78d (patch) | |
tree | ab4089988dfcebf9adea2a95f8119223a9542e28 /tvix | |
parent | 4e78de739327c33f37dad510875065d1ed57e2e8 (diff) |
fix(tvix/store/grpc/pathinfo): skip_all fields, handle errors r/7661
request only contains the outer metadata wrapping, and that's not too interesting: > Request { metadata: MetadataMap { headers: {"content-type": > "application/grpc", "user-agent": "grpc-go/1.60.1", "te": "trailers", > "grpc-accept-encoding": "gzip"} }, message: Streaming, extensions: > Extensions } Drop these fields for now, and rely on the underlying implementations to add instrumentation for the application-specific fields. Also, ensure we handle all error cases properly, and log them. We don't use `err` from instrument, as that'd also log an error on `Status::not_found`. Change-Id: Id1b983cb8b059c148c8a376f8802a1d28c59ba97 Reviewed-on: https://cl.tvl.fyi/c/depot/+/11103 Reviewed-by: Connor Brewster <cbrewster@hey.com> Tested-by: BuildkiteCI Autosubmit: flokli <flokli@flokli.de>
Diffstat (limited to 'tvix')
-rw-r--r-- | tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs | 35 |
1 files changed, 20 insertions, 15 deletions
diff --git a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs index a5c5776cd4ef..9f458182274d 100644 --- a/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs +++ b/tvix/store/src/proto/grpc_pathinfoservice_wrapper.rs @@ -27,7 +27,7 @@ where { type ListStream = BoxStream<'static, tonic::Result<proto::PathInfo, Status>>; - #[instrument(skip(self))] + #[instrument(skip_all)] async fn get( &self, request: Request<proto::GetPathInfoRequest>, @@ -43,7 +43,7 @@ where Ok(None) => Err(Status::not_found("PathInfo not found")), Ok(Some(path_info)) => Ok(Response::new(path_info)), Err(e) => { - warn!("failed to retrieve PathInfo: {}", e); + warn!(err = %e, "failed to get PathInfo"); Err(e.into()) } } @@ -51,7 +51,7 @@ where } } - #[instrument(skip(self))] + #[instrument(skip_all)] async fn put(&self, request: Request<proto::PathInfo>) -> Result<Response<proto::PathInfo>> { let path_info = request.into_inner(); @@ -60,13 +60,13 @@ where match self.inner.put(path_info).await { Ok(path_info_new) => Ok(Response::new(path_info_new)), Err(e) => { - warn!("failed to insert PathInfo: {}", e); + warn!(err = %e, "failed to put PathInfo"); Err(e.into()) } } } - #[instrument(skip(self))] + #[instrument(skip_all)] async fn calculate_nar( &self, request: Request<castorepb::Node>, @@ -74,21 +74,26 @@ where match request.into_inner().node { None => Err(Status::invalid_argument("no root node sent")), Some(root_node) => { - let (nar_size, nar_sha256) = self - .inner - .calculate_nar(&root_node) - .await - .expect("error during nar calculation"); // TODO: handle error + if let Err(e) = root_node.validate() { + warn!(err = %e, "invalid root node"); + Err(Status::invalid_argument("invalid root node"))? + } - Ok(Response::new(proto::CalculateNarResponse { - nar_size, - nar_sha256: nar_sha256.to_vec().into(), - })) + match self.inner.calculate_nar(&root_node).await { + Ok((nar_size, nar_sha256)) => Ok(Response::new(proto::CalculateNarResponse { + nar_size, + nar_sha256: nar_sha256.to_vec().into(), + })), + Err(e) => { + warn!(err = %e, "error during NAR calculation"); + Err(e.into()) + } + } } } } - #[instrument(skip(self))] + #[instrument(skip_all, err)] async fn list( &self, _request: Request<proto::ListPathInfoRequest>, |