This is an automated email from the ASF dual-hosted git repository. xuanwo pushed a commit to branch dropbox-refresh-token in repository https://gitbox.apache.org/repos/asf/incubator-opendal.git
commit 1d045b548b96a9a2ae9551292cb4dc34c5360986 Author: Xuanwo <[email protected]> AuthorDate: Fri Jul 7 11:56:27 2023 +0800 Refactor error handling Signed-off-by: Xuanwo <[email protected]> --- core/src/services/dropbox/backend.rs | 56 ++++++++++++++++++++++- core/src/services/dropbox/error.rs | 83 ++++++++++++++++----------------- core/src/services/dropbox/mod.rs | 1 - core/src/services/dropbox/response.rs | 86 ----------------------------------- 4 files changed, 93 insertions(+), 133 deletions(-) diff --git a/core/src/services/dropbox/backend.rs b/core/src/services/dropbox/backend.rs index f0ae8cde0..4af8f01b6 100644 --- a/core/src/services/dropbox/backend.rs +++ b/core/src/services/dropbox/backend.rs @@ -20,10 +20,10 @@ use std::sync::Arc; use async_trait::async_trait; use http::StatusCode; +use serde::Deserialize; use super::core::DropboxCore; use super::error::parse_error; -use super::response::DropboxMetadataResponse; use super::writer::DropboxWriter; use crate::raw::*; use crate::*; @@ -163,3 +163,57 @@ impl Accessor for DropboxBackend { } } } + +#[derive(Default, Debug, Deserialize)] +#[serde(default)] +pub struct DropboxMetadataResponse { + #[serde(rename(deserialize = ".tag"))] + pub tag: String, + pub client_modified: String, + pub content_hash: Option<String>, + pub file_lock_info: Option<DropboxMetadataFileLockInfo>, + pub has_explicit_shared_members: Option<bool>, + pub id: String, + pub is_downloadable: Option<bool>, + pub name: String, + pub path_display: String, + pub path_lower: String, + pub property_groups: Option<Vec<DropboxMetadataPropertyGroup>>, + pub rev: Option<String>, + pub server_modified: Option<String>, + pub sharing_info: Option<DropboxMetadataSharingInfo>, + pub size: Option<u64>, +} + +#[derive(Default, Debug, Deserialize)] +#[serde(default)] +pub struct DropboxMetadataFileLockInfo { + pub created: Option<String>, + pub is_lockholder: bool, + pub lockholder_name: Option<String>, +} + +#[derive(Default, Debug, Deserialize)] +#[serde(default)] +pub struct DropboxMetadataPropertyGroup { + pub fields: Vec<DropboxMetadataPropertyGroupField>, + pub template_id: String, +} + +#[derive(Default, Debug, Deserialize)] +#[serde(default)] +pub struct DropboxMetadataPropertyGroupField { + pub name: String, + pub value: String, +} + +#[derive(Default, Debug, Deserialize)] +#[serde(default)] +pub struct DropboxMetadataSharingInfo { + pub modified_by: Option<String>, + pub parent_shared_folder_id: Option<String>, + pub read_only: Option<bool>, + pub shared_folder_id: Option<String>, + pub traverse_only: Option<bool>, + pub no_access: Option<bool>, +} diff --git a/core/src/services/dropbox/error.rs b/core/src/services/dropbox/error.rs index 5e5831b15..57e3c177b 100644 --- a/core/src/services/dropbox/error.rs +++ b/core/src/services/dropbox/error.rs @@ -17,19 +17,26 @@ use http::Response; use http::StatusCode; +use http::Uri; +use serde::Deserialize; -use super::response::DropboxErrorResponse; use crate::raw::*; use crate::Error; use crate::ErrorKind; use crate::Result; +#[derive(Default, Debug, Deserialize)] +#[serde(default)] +pub struct DropboxErrorResponse { + error_summary: String, +} + /// Parse error response into Error. pub async fn parse_error(resp: Response<IncomingAsyncBody>) -> Result<Error> { let (parts, body) = resp.into_parts(); let bs = body.bytes().await?; - let (kind, retryable) = match parts.status { + let (mut kind, mut retryable) = match parts.status { StatusCode::NOT_FOUND => (ErrorKind::NotFound, false), StatusCode::FORBIDDEN => (ErrorKind::PermissionDenied, false), StatusCode::INTERNAL_SERVER_ERROR @@ -39,53 +46,39 @@ pub async fn parse_error(resp: Response<IncomingAsyncBody>) -> Result<Error> { _ => (ErrorKind::Unexpected, false), }; - let dropbox_error = - serde_json::from_slice::<DropboxErrorResponse>(&bs).map_err(new_json_deserialize_error); - match dropbox_error { - Ok(dropbox_error) => { - // We cannot get the error type from the response header when the status code is 409. - // Because Dropbox API v2 will put error summary in the response body, - // we need to parse it to get the correct error type and then error kind. - // See https://www.dropbox.com/developers/documentation/http/documentation#error-handling - let error_summary = dropbox_error.error_summary.as_str(); + let (message, dropbox_err) = serde_json::from_slice::<DropboxErrorResponse>(&bs) + .map(|dropbox_err| (format!("{dropbox_err:?}"), Some(dropbox_err))) + .unwrap_or_else(|_| (String::from_utf8_lossy(&bs).into_owned(), None)); - let mut err = Error::new( - match parts.status { - // 409 Conflict means that Endpoint-specific error. - // Look to the JSON response body for the specifics of the error. - StatusCode::CONFLICT => { - if error_summary.contains("path/not_found") - || error_summary.contains("path_lookup/not_found") - { - ErrorKind::NotFound - } else if error_summary.contains("path/conflict") { - ErrorKind::AlreadyExists - } else { - ErrorKind::Unexpected - } - } - // Otherwise, we can get the error type from the response status code. - _ => kind, - }, - error_summary, - ) - .with_context("response", format!("{parts:?}")); + if let Some(dropbox_err) = dropbox_err { + (kind, retryable) = + parse_dropbox_error_summary(&dropbox_err.error_summary).unwrap_or((kind, retryable)); + } - if retryable { - err = err.set_temporary(); - } + let mut err = Error::new(kind, &message).with_context("response", format!("{parts:?}")); - Ok(err) - } - Err(_err) => { - let mut err = Error::new(kind, &String::from_utf8_lossy(&bs)) - .with_context("response", format!("{parts:?}")); + if retryable { + err = err.set_temporary(); + } - if retryable { - err = err.set_temporary(); - } + if let Some(uri) = parts.extensions.get::<Uri>() { + err = err.with_context("uri", uri.to_string()); + } + + Ok(err) +} - Ok(err) - } +/// We cannot get the error type from the response header when the status code is 409. +/// Because Dropbox API v2 will put error summary in the response body, +/// we need to parse it to get the correct error type and then error kind. +/// +/// See <https://www.dropbox.com/developers/documentation/http/documentation#error-handling> +pub fn parse_dropbox_error_summary(summary: &str) -> Option<(ErrorKind, bool)> { + if summary.starts_with("path/not_found") || summary.starts_with("path_lookup/not_found") { + Some((ErrorKind::NotFound, false)) + } else if summary.starts_with("path/conflict") { + Some((ErrorKind::AlreadyExists, false)) + } else { + None } } diff --git a/core/src/services/dropbox/mod.rs b/core/src/services/dropbox/mod.rs index 048b7d74d..239974a75 100644 --- a/core/src/services/dropbox/mod.rs +++ b/core/src/services/dropbox/mod.rs @@ -19,7 +19,6 @@ mod backend; mod builder; mod core; mod error; -mod response; mod writer; pub use builder::DropboxBuilder as Dropbox; diff --git a/core/src/services/dropbox/response.rs b/core/src/services/dropbox/response.rs deleted file mode 100644 index 2e5c15a81..000000000 --- a/core/src/services/dropbox/response.rs +++ /dev/null @@ -1,86 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. - -use serde::Deserialize; - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxErrorResponse { - pub error_summary: String, - pub error: DropboxErrorDetail, -} - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxErrorDetail { - #[serde(rename(deserialize = ".tag"))] - pub tag: String, -} - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxMetadataResponse { - #[serde(rename(deserialize = ".tag"))] - pub tag: String, - pub client_modified: String, - pub content_hash: Option<String>, - pub file_lock_info: Option<DropboxMetadataFileLockInfo>, - pub has_explicit_shared_members: Option<bool>, - pub id: String, - pub is_downloadable: Option<bool>, - pub name: String, - pub path_display: String, - pub path_lower: String, - pub property_groups: Option<Vec<DropboxMetadataPropertyGroup>>, - pub rev: Option<String>, - pub server_modified: Option<String>, - pub sharing_info: Option<DropboxMetadataSharingInfo>, - pub size: Option<u64>, -} - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxMetadataFileLockInfo { - pub created: Option<String>, - pub is_lockholder: bool, - pub lockholder_name: Option<String>, -} - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxMetadataPropertyGroup { - pub fields: Vec<DropboxMetadataPropertyGroupField>, - pub template_id: String, -} - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxMetadataPropertyGroupField { - pub name: String, - pub value: String, -} - -#[derive(Default, Debug, Deserialize)] -#[serde(default)] -pub struct DropboxMetadataSharingInfo { - pub modified_by: Option<String>, - pub parent_shared_folder_id: Option<String>, - pub read_only: Option<bool>, - pub shared_folder_id: Option<String>, - pub traverse_only: Option<bool>, - pub no_access: Option<bool>, -}
