From 11da23c17ae6a6cb976395fc72eeb0921db2ad49 Mon Sep 17 00:00:00 2001 From: meowjesty Date: Tue, 15 Oct 2024 16:35:22 -0300 Subject: [PATCH] improve error for dns getaddrinfo --- mirrord/agent/src/dns.rs | 8 ++++++-- mirrord/layer/src/error.rs | 4 ++-- mirrord/layer/src/socket/ops.rs | 2 +- mirrord/protocol/src/error.rs | 19 ++++++++++++++++--- 4 files changed, 25 insertions(+), 8 deletions(-) diff --git a/mirrord/agent/src/dns.rs b/mirrord/agent/src/dns.rs index ddae278930e..71e8b55e8c4 100644 --- a/mirrord/agent/src/dns.rs +++ b/mirrord/agent/src/dns.rs @@ -87,7 +87,7 @@ impl DnsWorker { let hosts_path = etc_path.join("hosts"); // TODO(alex) [high] 1: Return an io error here so we can inspect it in mirrord. - let resolv_conf = fs::read(resolv_conf_path).await?; + let resolv_conf = fs::read("/meow/meow").await?; let hosts_conf = fs::read(hosts_path).await?; let (config, mut options) = parse_resolv_conf(resolv_conf)?; @@ -112,12 +112,16 @@ impl DnsWorker { } /// Handles the given [`DnsCommand`] in a separate [`tokio::task`]. + #[tracing::instrument(level = Level::DEBUG, skip(self))] fn handle_message(&self, message: DnsCommand) { let etc_path = self.etc_path.clone(); let timeout = self.timeout; let attempts = self.attempts; let lookup_future = async move { - let result = Self::do_lookup(etc_path, message.request.node, attempts, timeout).await; + let result = Self::do_lookup(etc_path, message.request.node, attempts, timeout) + .await + .inspect_err(|fail| tracing::error!(?fail, "DNS lookup failed!")); + tracing::debug!(?result, "DNS RESULT!"); if let Err(result) = message.response_tx.send(result) { tracing::error!(?result, "Failed to send query response"); } diff --git a/mirrord/layer/src/error.rs b/mirrord/layer/src/error.rs index 100010860af..cf6422e0778 100644 --- a/mirrord/layer/src/error.rs +++ b/mirrord/layer/src/error.rs @@ -8,7 +8,7 @@ use mirrord_protocol::{ResponseError, SerializationError}; #[cfg(target_os = "macos")] use mirrord_sip::SipError; use thiserror::Error; -use tracing::{error, info}; +use tracing::{error, info, Level}; use crate::{graceful_exit, proxy_connection::ProxyError}; @@ -41,7 +41,7 @@ mod ignore_codes { /// These errors are converted to [`libc`] error codes, and are also used to [`set_errno`]. #[derive(Error, Debug)] pub(crate) enum HookError { - #[error("mirrord-layer: Failed while getting a response!")] + #[error(transparent)] ResponseError(#[from] ResponseError), #[error("mirrord-layer: DNS does not resolve!")] diff --git a/mirrord/layer/src/socket/ops.rs b/mirrord/layer/src/socket/ops.rs index 8457063ef37..180c76aa605 100644 --- a/mirrord/layer/src/socket/ops.rs +++ b/mirrord/layer/src/socket/ops.rs @@ -872,7 +872,7 @@ pub(super) fn dup(fd: c_int, dup_fd: i32) -> Result<(), /// # Note /// /// This function updates the mapping in [`REMOTE_DNS_REVERSE_MAPPING`]. -#[mirrord_layer_macro::instrument(level = Level::TRACE, ret)] +#[mirrord_layer_macro::instrument(level = Level::DEBUG, ret, err)] pub(super) fn remote_getaddrinfo(node: String) -> HookResult> { let addr_info_list = common::make_proxy_request_with_response(GetAddrInfoRequest { node })?.0?; diff --git a/mirrord/protocol/src/error.rs b/mirrord/protocol/src/error.rs index 1fc767a5f9e..05b342708ca 100644 --- a/mirrord/protocol/src/error.rs +++ b/mirrord/protocol/src/error.rs @@ -8,7 +8,7 @@ use std::{ use bincode::{Decode, Encode}; use hickory_resolver::error::{ResolveError, ResolveErrorKind}; use thiserror::Error; -use tracing::warn; +use tracing::{warn, Level}; use crate::{ outgoing::SocketAddress, @@ -44,7 +44,7 @@ pub enum ResponseError { #[error("IO failed for remote operation with `{0}!")] RemoteIO(RemoteIOError), - #[error("IO failed for remote operation with `{0}!")] + #[error(transparent)] DnsLookup(DnsLookupError), #[error("Remote operation failed with `{0}`")] @@ -158,6 +158,7 @@ pub struct DnsLookupError { } impl From for ResponseError { + #[tracing::instrument(level = Level::DEBUG, ret)] fn from(io_error: io::Error) -> Self { Self::RemoteIO(RemoteIOError { raw_os_error: io_error.raw_os_error(), @@ -167,6 +168,7 @@ impl From for ResponseError { } impl From for ResponseError { + #[tracing::instrument(level = Level::DEBUG, ret)] fn from(fail: ResolveError) -> Self { match fail.kind().to_owned() { ResolveErrorKind::Io(io_fail) => io_fail.into(), @@ -233,6 +235,8 @@ pub enum ResolveErrorKindInternal { Timeout, // Unknown is for uncovered cases (enum is non-exhaustive) Unknown, + NotFound, + PermissionDenied, } impl From for ErrorKindInternal { @@ -285,6 +289,7 @@ impl From for ErrorKindInternal { impl From for ResolveErrorKindInternal { fn from(error_kind: ResolveErrorKind) -> Self { + println!("{error_kind:?}"); match error_kind { ResolveErrorKind::Message(message) => { ResolveErrorKindInternal::Message(message.to_string()) @@ -296,8 +301,16 @@ impl From for ResolveErrorKindInternal { } ResolveErrorKind::Proto(_) => ResolveErrorKindInternal::Proto, ResolveErrorKind::Timeout => ResolveErrorKindInternal::Timeout, + ResolveErrorKind::Io(fail) => match fail.kind() { + io::ErrorKind::NotFound => ResolveErrorKindInternal::NotFound, + io::ErrorKind::PermissionDenied => ResolveErrorKindInternal::PermissionDenied, + other => { + warn!(?other, "unknown IO error"); + ResolveErrorKindInternal::Unknown + } + }, _ => { - warn!("unknown error kind: {:?}", error_kind); + warn!(?error_kind, "unknown error kind"); ResolveErrorKindInternal::Unknown } }