Skip to content

Commit

Permalink
Use anyhow::Result instead of Result<_, Box<dyn Error>> (#38)
Browse files Browse the repository at this point in the history
* Use anyhow::Result instead of Result<_, Box<dyn Error>>

You can't put the latter into the former, and the former can move
across thread boundaries while the latter can't.

I hit some of these errors while using anyhow in the PHP profiler.
Since ddcommon is using anyhow already, it seemed like we could
align here.
  • Loading branch information
morrisonlevi authored Aug 17, 2022
1 parent 9609654 commit 7eed32d
Show file tree
Hide file tree
Showing 7 changed files with 23 additions and 30 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 2 additions & 8 deletions ddcommon-ffi/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,14 @@ impl<T> From<alloc::vec::Vec<T>> for Vec<T> {
}
}

impl From<&dyn std::error::Error> for Vec<u8> {
fn from(err: &dyn std::error::Error) -> Self {
impl From<anyhow::Error> for Vec<u8> {
fn from(err: anyhow::Error) -> Self {
let mut vec = vec![];
write!(vec, "{}", err).expect("write to vec to always succeed");
Self::from(vec)
}
}

impl From<Box<dyn std::error::Error>> for Vec<u8> {
fn from(err: Box<dyn std::error::Error>) -> Self {
Self::from(&*err)
}
}

impl<'a, T> IntoIterator for &'a Vec<T> {
type Item = &'a T;
type IntoIter = core::slice::Iter<'a, T>;
Expand Down
1 change: 1 addition & 0 deletions ddprof-ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ license = "Apache-2.0"
crate-type = ["staticlib", "cdylib"]

[dependencies]
anyhow = "1.0"
chrono = "0.4"
datadog-profiling = { path = "../profiling", version = "0.7.0-rc.1" }
hyper = {version = "0.14", default-features = false}
Expand Down
14 changes: 5 additions & 9 deletions ddprof-ffi/src/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use ddcommon::tag::Tag;
use ddcommon_ffi::slice::{AsBytes, ByteSlice, CharSlice, Slice};
use exporter::ProfileExporter;
use std::borrow::Cow;
use std::error::Error;
use std::ptr::NonNull;
use std::str::FromStr;

Expand Down Expand Up @@ -82,19 +81,16 @@ pub extern "C" fn endpoint_agentless<'a>(
Endpoint::Agentless(site, api_key)
}

unsafe fn try_to_url(slice: CharSlice) -> Result<hyper::Uri, Box<dyn Error>> {
unsafe fn try_to_url(slice: CharSlice) -> anyhow::Result<hyper::Uri> {
let str: &str = slice.try_to_utf8()?;
#[cfg(unix)]
if let Some(path) = str.strip_prefix("unix://") {
return Ok(exporter::socket_path_to_uri(path.as_ref())?);
}
match hyper::Uri::from_str(str) {
Ok(url) => Ok(url),
Err(err) => Err(Box::new(err)),
}
Ok(hyper::Uri::from_str(str)?)
}

unsafe fn try_to_endpoint(endpoint: Endpoint) -> Result<exporter::Endpoint, Box<dyn Error>> {
unsafe fn try_to_endpoint(endpoint: Endpoint) -> anyhow::Result<exporter::Endpoint> {
// convert to utf8 losslessly -- URLs and API keys should all be ASCII, so
// a failed result is likely to be an error.
match endpoint {
Expand All @@ -120,7 +116,7 @@ pub extern "C" fn profile_exporter_new(
tags: Option<&ddcommon_ffi::Vec<Tag>>,
endpoint: Endpoint,
) -> NewProfileExporterResult {
match || -> Result<ProfileExporter, Box<dyn Error>> {
match || -> anyhow::Result<ProfileExporter> {
let family = unsafe { family.to_utf8_lossy() }.into_owned();
let converted_endpoint = unsafe { try_to_endpoint(endpoint)? };
let tags = tags.map(|tags| tags.iter().map(|tag| tag.clone().into_owned()).collect());
Expand Down Expand Up @@ -216,7 +212,7 @@ pub unsafe extern "C" fn profile_exporter_send(

let cancel_option = unwrap_cancellation_token(cancel);

match || -> Result<HttpStatus, Box<dyn std::error::Error>> {
match || -> anyhow::Result<HttpStatus> {
let response = exp_ptr.as_ref().send((*request_ptr).0, cancel_option)?;

Ok(HttpStatus(response.status().as_u16()))
Expand Down
5 changes: 3 additions & 2 deletions ddprof-ffi/src/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::Timespec;
use datadog_profiling::profile as profiles;
use ddcommon_ffi::slice::{AsBytes, CharSlice, Slice};
use std::convert::{TryFrom, TryInto};
use std::error::Error;
use std::str::Utf8Error;
use std::time::{Duration, SystemTime};

Expand Down Expand Up @@ -420,7 +419,9 @@ pub unsafe extern "C" fn ddog_Profile_serialize(
Some(x) if *x < 0 => None,
Some(x) => Some(Duration::from_nanos((*x) as u64)),
};
match || -> Result<_, Box<dyn Error>> { Ok(profile.serialize(end_time, duration)?) }() {
match || -> anyhow::Result<datadog_profiling::profile::EncodedProfile> {
Ok(profile.serialize(end_time, duration)?)
}() {
Ok(ok) => SerializeResult::Ok(ok.into()),
Err(err) => SerializeResult::Err(err.into()),
}
Expand Down
9 changes: 5 additions & 4 deletions profiling/src/exporter/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,14 @@ use ddcommon::connector::uds;
use ddcommon::Endpoint;

use http::Uri;
use std::{borrow::Cow, error::Error, str::FromStr};
use std::borrow::Cow;
use std::str::FromStr;

/// Creates an Endpoint for talking to the Datadog agent.
///
/// # Arguments
/// * `base_url` - has protocol, host, and port e.g. http://localhost:8126/
pub fn agent(base_url: Uri) -> Result<Endpoint, Box<dyn Error>> {
pub fn agent(base_url: Uri) -> anyhow::Result<Endpoint> {
let mut parts = base_url.into_parts();
let p_q = match parts.path_and_query {
None => None,
Expand All @@ -32,7 +33,7 @@ pub fn agent(base_url: Uri) -> Result<Endpoint, Box<dyn Error>> {
/// # Arguments
/// * `socket_path` - file system path to the socket
#[cfg(unix)]
pub fn agent_uds(path: &std::path::Path) -> Result<Endpoint, Box<dyn Error>> {
pub fn agent_uds(path: &std::path::Path) -> anyhow::Result<Endpoint> {
let base_url = uds::socket_path_to_uri(path)?;
agent(base_url)
}
Expand All @@ -46,7 +47,7 @@ pub fn agent_uds(path: &std::path::Path) -> Result<Endpoint, Box<dyn Error>> {
pub fn agentless<AsStrRef: AsRef<str>, IntoCow: Into<Cow<'static, str>>>(
site: AsStrRef,
api_key: IntoCow,
) -> Result<Endpoint, Box<dyn Error>> {
) -> anyhow::Result<Endpoint> {
let intake_url: String = format!("https://intake.profile.{}/v1/input", site.as_ref());

Ok(Endpoint {
Expand Down
13 changes: 6 additions & 7 deletions profiling/src/exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// This product includes software developed at Datadog (https://www.datadoghq.com/). Copyright 2021-Present Datadog, Inc.

use std::borrow::Cow;
use std::error::Error;
use std::future;
use std::io::Cursor;

Expand Down Expand Up @@ -83,7 +82,7 @@ impl Request {
self,
client: &HttpClient,
cancel: Option<&CancellationToken>,
) -> Result<hyper::Response<hyper::Body>, Box<dyn std::error::Error>> {
) -> anyhow::Result<hyper::Response<hyper::Body>> {
tokio::select! {
_ = async { match cancel {
Some(cancellation_token) => cancellation_token.cancelled().await,
Expand All @@ -108,7 +107,7 @@ impl ProfileExporter {
family: IntoCow,
tags: Option<Vec<Tag>>,
endpoint: Endpoint,
) -> Result<ProfileExporter, Box<dyn Error>> {
) -> anyhow::Result<ProfileExporter> {
Ok(Self {
exporter: Exporter::new()?,
endpoint,
Expand All @@ -125,7 +124,7 @@ impl ProfileExporter {
files: &[File],
additional_tags: Option<&Vec<Tag>>,
timeout: std::time::Duration,
) -> Result<Request, Box<dyn Error>> {
) -> anyhow::Result<Request> {
let mut form = multipart::Form::default();

form.add_text("version", "3");
Expand Down Expand Up @@ -163,7 +162,7 @@ impl ProfileExporter {
&self,
request: Request,
cancel: Option<&CancellationToken>,
) -> Result<HttpResponse, Box<dyn Error>> {
) -> anyhow::Result<HttpResponse> {
self.exporter
.runtime
.block_on(request.send(&self.exporter.client, cancel))
Expand All @@ -172,7 +171,7 @@ impl ProfileExporter {

impl Exporter {
/// Creates a new Exporter, initializing the TLS stack.
pub fn new() -> Result<Self, Box<dyn Error>> {
pub fn new() -> anyhow::Result<Self> {
// Set idle to 0, which prevents the pipe being broken every 2nd request
let client = hyper::Client::builder()
.pool_max_idle_per_host(0)
Expand All @@ -190,7 +189,7 @@ impl Exporter {
mut headers: hyper::header::HeaderMap,
body: &[u8],
timeout: std::time::Duration,
) -> Result<hyper::Response<hyper::Body>, Box<dyn std::error::Error>> {
) -> anyhow::Result<hyper::Response<hyper::Body>> {
self.runtime.block_on(async {
let mut request = hyper::Request::builder()
.method(http_method)
Expand Down

0 comments on commit 7eed32d

Please sign in to comment.