Implement Sync for AzureError

[nbigaouette]

I'm trying to bootstrap a simple project talking to an azure blob storage. To simplify error handling I want to use the anyhow crate.

Unfortunately I cannot make both crates work. anyhow complains that the AzureError is not sync:

error[E0277]: `(dyn std::error::Error + std::marker::Send + 'static)` cannot be shared between threads safely
  --> end2end/src/main.rs:42:72
   |
42 |     let client = Client::new("your_storage_account", "your_master_key")?;
   |                                                                        ^ `(dyn std::error::Error + std::marker::Send + 'static)` cannot be shared between threads safely
   |
   = help: the trait `std::marker::Sync` is not implemented for `(dyn std::error::Error + std::marker::Send + 'static)`
   = note: required because of the requirements on the impl of `std::marker::Sync` for `std::ptr::Unique<(dyn std::error::Error + std::marker::Send + 'static)>`
   = note: required because it appears within the type `std::boxed::Box<(dyn std::error::Error + std::marker::Send + 'static)>`
   = note: required because it appears within the type `std::option::Option<std::boxed::Box<(dyn std::error::Error + std::marker::Send + 'static)>>`
   = note: required because it appears within the type `error_chain::State`
   = note: required because it appears within the type `serde_xml_rs::error::Error`
   = note: required because it appears within the type `azure_sdk_core::errors::AzureError`
   = note: required because of the requirements on the impl of `std::convert::From<azure_sdk_core::errors::AzureError>` for `anyhow::Error`
   = note: required by `std::convert::From::from`

where main the signature is fn main() -> anyhow::Result<()>.

Looking around anyhow github issue, it seems to want a Sync error to be able to work in async code: dtolnay/anyhow#81

And according to dtolnay/anyhow#72, errors should be Sync (see comment dtolnay/anyhow#72 (comment))

Could AzureError be made Sync?

Looking at the compiler error, is seems error_chain is an indirect dependency (through serde_xml), which could potentially block this... Any though?

Thanks!

[MindFlavor]

Your guess was right, now AzureError is both Send and Sync 🍾🍾🍾 !

I've added some pseudo unit tests to check this:

AzureSDKForRust/azure_sdk_core/src/errors.rs

Lines 439 to 480 in 4ab4529

	#[cfg(test)]
	mod test {
	use super::*;
	fn send_fn<T>(_t: T)
	where
	T: Send,
	{
	}
	fn sync_fn<T>(_t: T)
	where
	T: Sync,
	{
	}
	#[test]
	fn test_azure_error_send() {
	let e = AzureError::HeaderNotFound("Content-Length".to_owned());
	send_fn(e);
	}
	#[test]
	fn test_azure_error_sync() {
	let e = AzureError::HeaderNotFound("Content-Length".to_owned());
	sync_fn(e);
	}
	// This does not compile
	//#[test]
	//fn test_not_send() {
	// let a = std::rc::Rc::new(100);
	// send_fn(a);
	//}
	// This does not compile
	//#[test]
	//fn test_not_sync() {
	// let a = std::cell::Cell::new(500);
	// sync_fn(a);
	//}
	}

This way if something not Send and/or Sync slips through the cracks in the future we will be notified.

[nbigaouette]

Great! Thanks a lot!

For the tests that fail to compile, there is a feature of doc tests that could be useful: https://doc.rust-lang.org/edition-guide/rust-2018/rustdoc/documentation-tests-can-now-compile-fail.html
That would require moving the "non-compilable tests" to doc-tests, but it might be possible.