From e44e6eea63f0f0e28602bb2f932622c3cb8b6d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20Berthaud-M=C3=BCller?= Date: Fri, 22 Apr 2022 20:43:24 +0200 Subject: [PATCH] change connector error management --- src/dns/client.rs | 2 +- src/dns/connector.rs | 18 ++++---- src/dns/dns_connector.rs | 90 +++++++++++++++++++++++++++++----------- src/dns/mod.rs | 2 +- src/models/errors.rs | 32 +++++++------- 5 files changed, 96 insertions(+), 48 deletions(-) diff --git a/src/dns/client.rs b/src/dns/client.rs index 6f072bd..7f77c73 100644 --- a/src/dns/client.rs +++ b/src/dns/client.rs @@ -50,7 +50,7 @@ impl<'r> FromRequest<'r> for DnsClient { let config = try_outcome!(request.guard::<&State>().await); match DnsClient::new(config.dns.server).await { Err(e) => { - println!("Failed to connect to DNS server {:#?}", e); + println!("Failed to connect to DNS server: {}", e); Outcome::Failure((Status::InternalServerError, ())) }, Ok(c) => Outcome::Success(c) diff --git a/src/dns/connector.rs b/src/dns/connector.rs index 70f75e8..00ddc6b 100644 --- a/src/dns/connector.rs +++ b/src/dns/connector.rs @@ -6,12 +6,10 @@ use crate::dns; // E.g.: DNS update + axfr, zone file read + write #[async_trait] pub trait RecordConnector { - type Error; - - async fn get_records(&mut self, zone: dns::Name, class: dns::DNSClass) -> Result, Self::Error>; - async fn add_records(&mut self, zone: dns::Name, class: dns::DNSClass, new_records: Vec) -> Result<(), Self::Error>; - async fn update_records(&mut self, zone: dns::Name, class: dns::DNSClass, old_records: Vec, new_records: Vec) -> Result<(), Self::Error>; - async fn delete_records(&mut self, zone: dns::Name, class: dns::DNSClass, records: Vec) -> Result<(), Self::Error>; + async fn get_records(&mut self, zone: dns::Name, class: dns::DNSClass) -> Result, Box>; + async fn add_records(&mut self, zone: dns::Name, class: dns::DNSClass, new_records: Vec) -> Result<(), Box>; + async fn update_records(&mut self, zone: dns::Name, class: dns::DNSClass, old_records: Vec, new_records: Vec) -> Result<(), Box>; + async fn delete_records(&mut self, zone: dns::Name, class: dns::DNSClass, records: Vec) -> Result<(), Box>; // delete_records } @@ -19,9 +17,13 @@ pub trait RecordConnector { // E.g.: Manage catalog zone, dynamically generate knot / bind / nsd config... #[async_trait] pub trait ZoneConnector { - type Error; // get_zones // add_zone // delete_zone - async fn zone_exists(&mut self, zone: dns::Name, class: dns::DNSClass) -> Result<(), Self::Error>; + async fn zone_exists(&mut self, zone: dns::Name, class: dns::DNSClass) -> Result<(), Box>; +} + +pub trait ConnectorError: std::fmt::Debug + std::fmt::Display { + fn is_proto_error(&self) -> bool; + fn zone_name(&self) -> Option; } \ No newline at end of file diff --git a/src/dns/dns_connector.rs b/src/dns/dns_connector.rs index aad60e5..d6db87f 100644 --- a/src/dns/dns_connector.rs +++ b/src/dns/dns_connector.rs @@ -6,7 +6,7 @@ use trust_dns_client::error::ClientError; use super::{Name, Record, RData}; use super::client::{ClientResponse, DnsClient}; -use super::connector::{RecordConnector, ZoneConnector}; +use super::connector::{RecordConnector, ZoneConnector, ConnectorError}; const MAX_PAYLOAD_LEN: u16 = 1232; @@ -33,23 +33,55 @@ impl DnsConnectorClient { } } +impl ConnectorError for DnsConnectorError { + fn zone_name(&self) -> Option { + if let DnsConnectorError::ResponceNotOk { code: _code, zone } = self { + Some(zone.clone()) + } else { + None + } + } + + fn is_proto_error(&self) -> bool { + return matches!(self, DnsConnectorError::ClientError(_)); + } +} + +impl std::fmt::Display for DnsConnectorError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DnsConnectorError::ClientError(e) => { + write!(f, "DNS client error: {}", e) + }, + DnsConnectorError::ResponceNotOk { code, zone } => { + write!(f, "Query for zone {} failed with code {}", zone, code) + } + } + + } +} + + #[async_trait] impl RecordConnector for DnsConnectorClient { - type Error = DnsConnectorError; + //type Error = DnsConnectorError; - async fn get_records(&mut self, zone: Name, class: DNSClass) -> Result, Self::Error> + async fn get_records(&mut self, zone: Name, class: DNSClass) -> Result, Box> { let response = { let query = self.client.query(zone.clone(), class, RecordType::AXFR); - query.await.map_err(|e| DnsConnectorError::ClientError(e))? + match query.await.map_err(|e| Box::new(DnsConnectorError::ClientError(e))) { + Err(e) => return Err(e), + Ok(v) => v, + } }; if response.response_code() != ResponseCode::NoError { - return Err(DnsConnectorError::ResponceNotOk { + return Err(Box::new(DnsConnectorError::ResponceNotOk { code: response.response_code(), zone: zone, - }); + })); } let answers = response.answers(); @@ -62,7 +94,7 @@ impl RecordConnector for DnsConnectorClient { Ok(records) } - async fn add_records(&mut self, zone: Name, class: DNSClass, new_records: Vec) -> Result<(), Self::Error> + async fn add_records(&mut self, zone: Name, class: DNSClass, new_records: Vec) -> Result<(), Box> { // Taken from trust_dns_client::op::update_message::append // The original function can not be used as is because it takes a RecordSet and not a Record list @@ -89,19 +121,22 @@ impl RecordConnector for DnsConnectorClient { edns.set_version(0); } - let response = ClientResponse(self.client.send(message)).await.map_err(|e| DnsConnectorError::ClientError(e))?; + let response = match ClientResponse(self.client.send(message)).await.map_err(|e| Box::new(DnsConnectorError::ClientError(e))) { + Err(e) => return Err(e), + Ok(v) => v, + }; if response.response_code() != ResponseCode::NoError { - return Err(DnsConnectorError::ResponceNotOk { + return Err(Box::new(DnsConnectorError::ResponceNotOk { code: response.response_code(), zone: zone, - }); + })); } Ok(()) } - async fn update_records(&mut self, zone: Name, class: DNSClass, old_records: Vec, new_records: Vec) -> Result<(), Self::Error> + async fn update_records(&mut self, zone: Name, class: DNSClass, old_records: Vec, new_records: Vec) -> Result<(), Box> { // Taken from trust_dns_client::op::update_message::compare_and_swap // The original function can not be used as is because it takes a RecordSet and not a Record list @@ -150,19 +185,22 @@ impl RecordConnector for DnsConnectorClient { edns.set_version(0); } - let response = ClientResponse(self.client.send(message)).await.map_err(|e| DnsConnectorError::ClientError(e))?; + let response = match ClientResponse(self.client.send(message)).await.map_err(|e| Box::new(DnsConnectorError::ClientError(e))) { + Err(e) => return Err(e), + Ok(v) => v, + }; if response.response_code() != ResponseCode::NoError { - return Err(DnsConnectorError::ResponceNotOk { + return Err(Box::new(DnsConnectorError::ResponceNotOk { code: response.response_code(), zone: zone, - }); + })); } Ok(()) } - async fn delete_records(&mut self, zone: Name, class: DNSClass, records: Vec) -> Result<(), Self::Error> + async fn delete_records(&mut self, zone: Name, class: DNSClass, records: Vec) -> Result<(), Box> { // for updates, the query section is used for the zone let mut zone_query = Query::new(); @@ -197,13 +235,16 @@ impl RecordConnector for DnsConnectorClient { edns.set_version(0); } - let response = ClientResponse(self.client.send(message)).await.map_err(|e| DnsConnectorError::ClientError(e))?; + let response = match ClientResponse(self.client.send(message)).await.map_err(|e| Box::new(DnsConnectorError::ClientError(e))) { + Err(e) => return Err(e), + Ok(v) => v, + }; if response.response_code() != ResponseCode::NoError { - return Err(DnsConnectorError::ResponceNotOk { + return Err(Box::new(DnsConnectorError::ResponceNotOk { code: response.response_code(), zone: zone, - }); + })); } Ok(()) @@ -214,20 +255,21 @@ impl RecordConnector for DnsConnectorClient { #[async_trait] impl ZoneConnector for DnsConnectorClient { - type Error = DnsConnectorError; - - async fn zone_exists(&mut self, zone: Name, class: DNSClass) -> Result<(), Self::Error> + async fn zone_exists(&mut self, zone: Name, class: DNSClass) -> Result<(), Box> { let response = { let query = self.client.query(zone.clone(), class, RecordType::SOA); - query.await.map_err(|e| DnsConnectorError::ClientError(e))? + match query.await.map_err(|e| Box::new(DnsConnectorError::ClientError(e))) { + Err(e) => return Err(e), + Ok(v) => v, + } }; if response.response_code() != ResponseCode::NoError { - return Err(DnsConnectorError::ResponceNotOk { + return Err(Box::new(DnsConnectorError::ResponceNotOk { code: response.response_code(), zone: zone, - }); + })); } Ok(()) diff --git a/src/dns/mod.rs b/src/dns/mod.rs index a5fc33d..6abeca3 100644 --- a/src/dns/mod.rs +++ b/src/dns/mod.rs @@ -12,6 +12,6 @@ pub use trust_dns_client::rr::{ pub use trust_dns_proto::rr::Name; // Reexport module types -pub use connector::{RecordConnector, ZoneConnector}; +pub use connector::{RecordConnector, ZoneConnector, ConnectorError}; pub use dns_connector::{DnsConnectorClient, DnsConnectorError}; pub use client::DnsClient; \ No newline at end of file diff --git a/src/models/errors.rs b/src/models/errors.rs index e286a6c..130d876 100644 --- a/src/models/errors.rs +++ b/src/models/errors.rs @@ -7,7 +7,7 @@ use rocket::serde::json::Json; use serde_json::Value; use djangohashers::{HasherError}; use diesel::result::Error as DieselError; -use crate::dns::DnsConnectorError; +use crate::dns::ConnectorError; use crate::models; #[derive(Debug)] @@ -100,20 +100,23 @@ impl From for ErrorResponse { } } -impl From for ErrorResponse { - fn from(e: DnsConnectorError) -> Self { - match e { - DnsConnectorError::ResponceNotOk { code, zone } => { - println!("Query for zone {} failed with code {}", zone, code); - - ErrorResponse::new( - Status::NotFound, - "Zone could not be found".into() - ).with_details(json!({ +impl From> for ErrorResponse { + fn from(e: Box) -> Self { + println!("{}", e); + if e.is_proto_error() { + return make_500(e); + } else { + let error = ErrorResponse::new( + Status::NotFound, + "Zone could not be found".into() + ); + if let Some(zone) = e.zone_name() { + return error.with_details(json!({ "zone_name": zone.to_utf8() - })) - }, - DnsConnectorError::ClientError(e) => make_500(e) + })); + } else { + return error; + } } } } @@ -163,6 +166,7 @@ impl From for (Status, ErrorResponse) { } } +// TODO: change for Display trait pub fn make_500(e: E) -> ErrorResponse { println!("Making 500 for Error: {:?}", e); ErrorResponse::new(Status::InternalServerError, "An unexpected error occured.".into())