Skip to content

Commit

Permalink
RSDK-2407 - Fix gRPC error logging (#40)
Browse files Browse the repository at this point in the history
  • Loading branch information
stuqdog authored Apr 5, 2023
1 parent f9af5a0 commit c10e5c1
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 40 deletions.
32 changes: 7 additions & 25 deletions src/rpc/base_channel.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use anyhow::Result;
use std::{
fmt::Debug,
panic::{catch_unwind, set_hook, AssertUnwindSafe},
sync::{
atomic::{AtomicBool, AtomicPtr, Ordering},
Arc,
Expand All @@ -27,6 +26,12 @@ impl Debug for WebRTCBaseChannel {
}
}

impl Drop for WebRTCBaseChannel {
fn drop(&mut self) {
log::debug!("Dropping base channel {self:?}");
}
}

impl WebRTCBaseChannel {
pub(crate) async fn new(
peer_connection: Arc<RTCPeerConnection>,
Expand Down Expand Up @@ -62,16 +67,14 @@ impl WebRTCBaseChannel {

let c = Arc::downgrade(&channel);
dc.on_error(Box::new(move |err: webrtc::Error| {
log::error!("Data channel error: {err}");
let c = match c.upgrade() {
Some(c) => c,
None => return Box::pin(async {}),
};
Box::pin(async move {
let mut err = Some(anyhow::Error::from(err));
c.closed_reason.store(&mut err, Ordering::Release);
if let Err(e) = c.close_sync() {
log::error!("error closing channel: {e}")
}
})
}));

Expand All @@ -93,27 +96,6 @@ impl WebRTCBaseChannel {
.map_err(anyhow::Error::from)
}

// `close` with blocking. Should only be used in contexts where an async close is disallowed
pub(crate) fn close_sync(&self) -> Result<()> {
set_hook(Box::new(|info| {
log::error!(
"Unable to close base_channel gracefully. This may be because of an attempt to connect to a robot that isn't online. Error message: {}",
info.to_string()
)
}));
let safe_self = AssertUnwindSafe(self);
match catch_unwind(|| {
let rt = tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.unwrap();
rt.block_on(async move { safe_self.close().await })
}) {
Ok(res) => res,
Err(_) => Err(anyhow::anyhow!("Unable to close base channel gracefully")),
}
}

/// Returns whether or not the channel is closed
#[allow(dead_code)]
pub fn is_closed(&self) -> bool {
Expand Down
6 changes: 0 additions & 6 deletions src/rpc/client_channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ impl Debug for WebRTCClientChannel {

impl Drop for WebRTCClientChannel {
fn drop(&mut self) {
let bc = self.base_channel.clone();
if !bc.is_closed() {
if let Err(e) = bc.close_sync() {
log::error!("Error closing base channel: {e}")
}
};
log::debug!("Dropping client channel {:?}", &self);
}
}
Expand Down
15 changes: 6 additions & 9 deletions src/rpc/client_stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ impl WebRTCClientStream {
Ok(())
}

async fn process_trailers(&mut self, trailers: ResponseTrailers) -> Result<()> {
async fn process_trailers(&mut self, trailers: ResponseTrailers) {
let trailers_to_send = trailers_from_proto(trailers.clone());
if let Err(e) = self
.base_stream
Expand All @@ -69,14 +69,11 @@ impl WebRTCClientStream {
}
};

self.base_stream.close_with_recv_error(&mut err.as_ref());
match err {
None => Ok(()),
Some(err) => {
log::error!("Error processing trailers: {err}");
Err(err)
}
if let Some(e) = &err {
log::debug!("received gRPC error: {e}");
}

self.base_stream.close_with_recv_error(&mut err.as_ref())
}

// processes response.
Expand Down Expand Up @@ -114,7 +111,7 @@ impl WebRTCClientStream {
self.process_message(message.to_owned()).await
}

Some(Type::Trailers(trailers)) => self.process_trailers(trailers.to_owned()).await,
Some(Type::Trailers(trailers)) => Ok(self.process_trailers(trailers.to_owned()).await),
None => Ok(()),
}
}
Expand Down

0 comments on commit c10e5c1

Please sign in to comment.