Migrate security_keyvault to new azure_core::error scheme

[johnbatty]

Part of the mission to migrate crates to the new error scheme:
#771

[johnbatty]

@rylev, @cataggar Please could you review, as this is the first of these conversions that I've done.
Please also review/comment on my notes below, in case I have misunderstood anything!

Notes on the process, to help other crate migrations:

• Update Cargo.toml
  • Remove thiserror crate
  • Update azure_core version to 0.2
• Remove the thiserror error definitions
  • Tagged by #[derive(thiserror::Error...)]
• Include new azure_core::error definitions in each of the source files:
  • use azure_core::error::{Error, ErrorKind};
• Build... fixup errors... repeat (until it clean compiles)
• cargo test
• cargo clippy

Fixing up errors

Existing error types need to be replaced with the new azure_core::error errors. The new error type has a "kind" (azure_core::error::ErrorKind), plus a text description of the error/context with any helpful values included. The main work is

• (a) deciding what ErrorKind to use
• (b) creating a helpful error message describing the context and including appropriate values that will be useful to diagnose issues.

/// The kind of error
///
/// The classification of error is intentionally fairly coarse.
#[derive(Clone, Debug, PartialEq)]
pub enum ErrorKind {
    /// An HTTP status code that was not expected
    HttpResponse {
        status: u16,
        error_code: Option<String>,
    },
    /// An error performing IO
    Io,
    /// An error converting data
    DataConversion,
    /// An error getting an API credential token
    Credential,
    #[cfg(feature = "mock_transport_framework")]
    /// An error having to do with the mock framework
    MockFramework,
    /// A catch all for other kinds of errors
    Other,
}

I found .with_context(...) and .context(...) to be the main way to convert existing error types into new-style errors.

• .with_context(...) should be used if formatting the message via format!(...), as it allows the formatting to be passed via a closure which will only get executed if the error is required.
• .context(...) should be used if the message is simply a constant string literal.
  In some situations you may need to directly construct an error to return, which you can do via Error::with_message(kind, message)

Example replacements:

Using with_context(...)

// before:
        let vault_url = Url::parse(vault_url)?;
// after:
        let vault_url = Url::parse(vault_url).with_context(ErrorKind::DataConversion, || {
            format!("failed to parse vault url: {}", vault_url)
        })?;

Using context(...)

// before:
        let client = KeyClient {
            vault_url,
            .token_credential
            .get_token(&self.endpoint)
            .await
            .map_err(|_| Error::Authorization)?;

// after:
        let client = KeyClient {
            vault_url,
            .token_credential
            .get_token(&self.endpoint)
            .await
            .context(ErrorKind::Credential, "get token failed")?;  // .map_err(...) replaced by .context(...)

Directly constructing errors:

// before:
        if let Some(err) = body_deserialized.get("error") {
            let msg = err.get("message").ok_or(Error::UnparsableError)?;
            Err(Error::General(msg.to_string()))
        }
// after:
        if let Some(err) = body_deserialized.get("error") {
            let msg = err.get("message").ok_or_else(|| {
                Error::with_message(
                    ErrorKind::DataConversion,
                    format!(
                        "failed to read message field from error response. uri:{} body:{}",
                        uri, body
                    ),
                )
            })?;
            Err(Error::with_message(
                ErrorKind::Other,
                format!("post response error: {}", msg),
            ))

[johnbatty]

Aside... I noticed that there are a large number of unwrap() calls in this crate, which we should replace with more graceful error handling. I replaced a few, but when I realised how many there were I thought that perhaps I should deal with as a separate issue. Many of these are related to send/receive operations, so perhaps this code will just be replaced when we switch to the pipeline architecture...?

Example:

        let resp = reqwest::Client::new()
            .patch(&uri)
            .bearer_auth(self.token.as_ref().unwrap().token.secret())
            .header("Content-Type", "application/json")
            .body(body)
            .send()
            .await
            .unwrap();

Thoughts?

[cataggar]

Looking good. Can you review #781? It has a change that will help improve this PR.