From cd1ff18ba94966088a779b26347dc683f1f0c2d3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Sun, 22 Mar 2015 20:20:31 +0100 Subject: [PATCH] fix(cmn): upload() return value handling Now deals with Cancellation and non-OK status codes correctly. Fixes #18 --- gen/groupsmigration1/README.md | 1 + gen/groupsmigration1/src/cmn.rs | 24 ++++++++++++++++-------- gen/groupsmigration1/src/lib.rs | 19 +++++++++++++++---- src/mako/lib/mbuild.mako | 19 +++++++++++++++---- src/rust/cmn.rs | 24 ++++++++++++++++-------- 5 files changed, 63 insertions(+), 24 deletions(-) diff --git a/gen/groupsmigration1/README.md b/gen/groupsmigration1/README.md index 563da63508a..ddfb5a726d7 100644 --- a/gen/groupsmigration1/README.md +++ b/gen/groupsmigration1/README.md @@ -100,6 +100,7 @@ match result { Result::HttpError(err) => println!("HTTPERROR: {:?}", err), Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"), Result::MissingToken => println!("OAuth2: Missing Token"), + Result::Cancelled => println!("Operation cancelled by user"), Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size), Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"), Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field), diff --git a/gen/groupsmigration1/src/cmn.rs b/gen/groupsmigration1/src/cmn.rs index c4164bf18e8..aba4c1f9d12 100644 --- a/gen/groupsmigration1/src/cmn.rs +++ b/gen/groupsmigration1/src/cmn.rs @@ -228,6 +228,9 @@ pub enum Result { /// We required a Token, but didn't get one from the Authenticator MissingToken, + /// The delgate instructed to cancel the operation + Cancelled, + /// An additional, free form field clashed with one of the built-in optional ones FieldClash(&'static str), @@ -536,12 +539,15 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A> } } - pub fn upload(&mut self) -> hyper::HttpResult { + /// returns None if operation was cancelled by delegate, or the HttpResult. + /// It can be that we return the result just because we didn't understand the status code - + /// caller should check for status himself before assuming it's OK to use + pub fn upload(&mut self) -> Option> { let mut start = match self.start_at { Some(s) => s, None => match self.query_transfer_status() { (Some(s), _) => s, - (_, result) => return result + (_, result) => return Some(result) } }; @@ -565,7 +571,7 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A> }; start += request_size; if self.delegate.cancel_chunk_upload(&range_header) { - return Err(hyper::error::HttpError::HttpStatusError) + return None } match self.client.post(self.url) .header(range_header) @@ -573,24 +579,26 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A> .header(UserAgent(self.user_agent.to_string())) .body(&mut section_reader) .send() { - Ok(res) => { + Ok(mut res) => { if res.status == StatusCode::PermanentRedirect { continue } - if res.status != StatusCode::Ok { - if let Retry::After(d) = self.delegate.http_failure(&res, None) { + if !res.status.is_success() { + let mut json_err = String::new(); + res.read_to_string(&mut json_err).unwrap(); + if let Retry::After(d) = self.delegate.http_failure(&res, serde::json::from_str(&json_err).ok()) { sleep(d); continue; } } - return Ok(res) + return Some(Ok(res)) }, Err(err) => { if let Retry::After(d) = self.delegate.http_error(&err) { sleep(d); continue; } - return Err(err) + return Some(Err(err)) } } } diff --git a/gen/groupsmigration1/src/lib.rs b/gen/groupsmigration1/src/lib.rs index b58e140c128..6b6341f8121 100644 --- a/gen/groupsmigration1/src/lib.rs +++ b/gen/groupsmigration1/src/lib.rs @@ -101,6 +101,7 @@ //! Result::HttpError(err) => println!("HTTPERROR: {:?}", err), //! Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"), //! Result::MissingToken => println!("OAuth2: Missing Token"), +//! Result::Cancelled => println!("Operation cancelled by user"), //! Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size), //! Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"), //! Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field), @@ -266,6 +267,7 @@ impl Default for Scope { /// Result::HttpError(err) => println!("HTTPERROR: {:?}", err), /// Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"), /// Result::MissingToken => println!("OAuth2: Missing Token"), +/// Result::Cancelled => println!("Operation cancelled by user"), /// Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size), /// Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"), /// Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field), @@ -583,8 +585,7 @@ impl<'a, C, NC, A> ArchiveInsertCall<'a, C, NC, A> where NC: hyper::net::Network if !res.status.is_success() { let mut json_err = String::new(); res.read_to_string(&mut json_err).unwrap(); - let error_info: cmn::JsonServerError = json::from_str(&json_err).unwrap(); - if let oauth2::Retry::After(d) = dlg.http_failure(&res, Some(error_info)) { + if let oauth2::Retry::After(d) = dlg.http_failure(&res, json::from_str(&json_err).ok()) { sleep(d); continue; } @@ -618,11 +619,21 @@ impl<'a, C, NC, A> ArchiveInsertCall<'a, C, NC, A> where NC: hyper::net::Network }.upload() }; match upload_result { - Err(err) => { + None => { + dlg.finished(false); + return Result::Cancelled + } + Some(Err(err)) => { dlg.finished(false); return Result::HttpError(err) } - Ok(upload_result) => res = upload_result, + Some(Ok(upload_result)) => { + res = upload_result; + if !res.status.is_success() { + dlg.finished(false); + return Result::Failure(res) + } + } } } let result_value = { diff --git a/src/mako/lib/mbuild.mako b/src/mako/lib/mbuild.mako index 9213b7cea87..deb44e35e97 100644 --- a/src/mako/lib/mbuild.mako +++ b/src/mako/lib/mbuild.mako @@ -361,6 +361,7 @@ match result { Result::HttpError(err) => println!("HTTPERROR: {:?}", err), Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"), Result::MissingToken => println!("OAuth2: Missing Token"), + Result::Cancelled => println!("Operation cancelled by user"), Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size), Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"), Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field), @@ -760,8 +761,7 @@ else { if !res.status.is_success() { let mut json_err = String::new(); res.read_to_string(&mut json_err).unwrap(); - let error_info: cmn::JsonServerError = json::from_str(&json_err).unwrap(); - if let oauth2::Retry::After(d) = dlg.http_failure(&res, Some(error_info)) { + if let oauth2::Retry::After(d) = dlg.http_failure(&res, json::from_str(&json_err).ok()) { sleep(d); continue; } @@ -792,14 +792,25 @@ else { }.upload() }; match upload_result { - Err(err) => { + None => { + ${delegate_finish}(false); + return Result::Cancelled + } + Some(Err(err)) => { ## Do not ask the delgate again, as it was asked by the helper ! ${delegate_finish}(false); return Result::HttpError(err) } ## Now the result contains the actual resource, if any ... it will be ## decoded next - Ok(upload_result) => res = upload_result, + Some(Ok(upload_result)) => { + res = upload_result; + if !res.status.is_success() { + ## delegate was called in upload() already - don't tell him again + ${delegate_finish}(false); + return Result::Failure(res) + } + } } } % endif diff --git a/src/rust/cmn.rs b/src/rust/cmn.rs index ddb8bc6df9e..a005d3ca870 100644 --- a/src/rust/cmn.rs +++ b/src/rust/cmn.rs @@ -226,6 +226,9 @@ pub enum Result { /// We required a Token, but didn't get one from the Authenticator MissingToken, + /// The delgate instructed to cancel the operation + Cancelled, + /// An additional, free form field clashed with one of the built-in optional ones FieldClash(&'static str), @@ -534,12 +537,15 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A> } } - pub fn upload(&mut self) -> hyper::HttpResult { + /// returns None if operation was cancelled by delegate, or the HttpResult. + /// It can be that we return the result just because we didn't understand the status code - + /// caller should check for status himself before assuming it's OK to use + pub fn upload(&mut self) -> Option> { let mut start = match self.start_at { Some(s) => s, None => match self.query_transfer_status() { (Some(s), _) => s, - (_, result) => return result + (_, result) => return Some(result) } }; @@ -563,7 +569,7 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A> }; start += request_size; if self.delegate.cancel_chunk_upload(&range_header) { - return Err(hyper::error::HttpError::HttpStatusError) + return None } match self.client.post(self.url) .header(range_header) @@ -571,24 +577,26 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A> .header(UserAgent(self.user_agent.to_string())) .body(&mut section_reader) .send() { - Ok(res) => { + Ok(mut res) => { if res.status == StatusCode::PermanentRedirect { continue } - if res.status != StatusCode::Ok { - if let Retry::After(d) = self.delegate.http_failure(&res, None) { + if !res.status.is_success() { + let mut json_err = String::new(); + res.read_to_string(&mut json_err).unwrap(); + if let Retry::After(d) = self.delegate.http_failure(&res, serde::json::from_str(&json_err).ok()) { sleep(d); continue; } } - return Ok(res) + return Some(Ok(res)) }, Err(err) => { if let Retry::After(d) = self.delegate.http_error(&err) { sleep(d); continue; } - return Err(err) + return Some(Err(err)) } } }