Skip to content

Commit cd1ff18

Browse files
ByronByron
committed
fix(cmn): upload() return value handling
Now deals with Cancellation and non-OK status codes correctly. Fixes #18
1 parent 29ee94b commit cd1ff18

5 files changed

Lines changed: 63 additions & 24 deletions

File tree

‎gen/groupsmigration1/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ match result {
100100
Result::HttpError(err) => println!("HTTPERROR: {:?}", err),
101101
Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"),
102102
Result::MissingToken => println!("OAuth2: Missing Token"),
103+
Result::Cancelled => println!("Operation cancelled by user"),
103104
Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size),
104105
Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"),
105106
Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field),

‎gen/groupsmigration1/src/cmn.rs‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,9 @@ pub enum Result<T = ()> {
228228
/// We required a Token, but didn't get one from the Authenticator
229229
MissingToken,
230230

231+
/// The delgate instructed to cancel the operation
232+
Cancelled,
233+
231234
/// An additional, free form field clashed with one of the built-in optional ones
232235
FieldClash(&'static str),
233236

@@ -536,12 +539,15 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A>
536539
}
537540
}
538541

539-
pub fn upload(&mut self) -> hyper::HttpResult<hyper::client::Response> {
542+
/// returns None if operation was cancelled by delegate, or the HttpResult.
543+
/// It can be that we return the result just because we didn't understand the status code -
544+
/// caller should check for status himself before assuming it's OK to use
545+
pub fn upload(&mut self) -> Option<hyper::HttpResult<hyper::client::Response>> {
540546
let mut start = match self.start_at {
541547
Some(s) => s,
542548
None => match self.query_transfer_status() {
543549
(Some(s), _) => s,
544-
(_, result) => return result
550+
(_, result) => return Some(result)
545551
}
546552
};
547553

@@ -565,32 +571,34 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A>
565571
};
566572
start += request_size;
567573
if self.delegate.cancel_chunk_upload(&range_header) {
568-
return Err(hyper::error::HttpError::HttpStatusError)
574+
return None
569575
}
570576
match self.client.post(self.url)
571577
.header(range_header)
572578
.header(ContentType(self.media_type.clone()))
573579
.header(UserAgent(self.user_agent.to_string()))
574580
.body(&mut section_reader)
575581
.send() {
576-
Ok(res) => {
582+
Ok(mut res) => {
577583
if res.status == StatusCode::PermanentRedirect {
578584
continue
579585
}
580-
if res.status != StatusCode::Ok {
581-
if let Retry::After(d) = self.delegate.http_failure(&res, None) {
586+
if !res.status.is_success() {
587+
let mut json_err = String::new();
588+
res.read_to_string(&mut json_err).unwrap();
589+
if let Retry::After(d) = self.delegate.http_failure(&res, serde::json::from_str(&json_err).ok()) {
582590
sleep(d);
583591
continue;
584592
}
585593
}
586-
return Ok(res)
594+
return Some(Ok(res))
587595
},
588596
Err(err) => {
589597
if let Retry::After(d) = self.delegate.http_error(&err) {
590598
sleep(d);
591599
continue;
592600
}
593-
return Err(err)
601+
return Some(Err(err))
594602
}
595603
}
596604
}

‎gen/groupsmigration1/src/lib.rs‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@
101101
//! Result::HttpError(err) => println!("HTTPERROR: {:?}", err),
102102
//! Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"),
103103
//! Result::MissingToken => println!("OAuth2: Missing Token"),
104+
//! Result::Cancelled => println!("Operation cancelled by user"),
104105
//! Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size),
105106
//! Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"),
106107
//! Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field),
@@ -266,6 +267,7 @@ impl Default for Scope {
266267
/// Result::HttpError(err) => println!("HTTPERROR: {:?}", err),
267268
/// Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"),
268269
/// Result::MissingToken => println!("OAuth2: Missing Token"),
270+
/// Result::Cancelled => println!("Operation cancelled by user"),
269271
/// Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size),
270272
/// Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"),
271273
/// 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
583585
if !res.status.is_success() {
584586
let mut json_err = String::new();
585587
res.read_to_string(&mut json_err).unwrap();
586-
let error_info: cmn::JsonServerError = json::from_str(&json_err).unwrap();
587-
if let oauth2::Retry::After(d) = dlg.http_failure(&res, Some(error_info)) {
588+
if let oauth2::Retry::After(d) = dlg.http_failure(&res, json::from_str(&json_err).ok()) {
588589
sleep(d);
589590
continue;
590591
}
@@ -618,11 +619,21 @@ impl<'a, C, NC, A> ArchiveInsertCall<'a, C, NC, A> where NC: hyper::net::Network
618619
}.upload()
619620
};
620621
match upload_result {
621-
Err(err) => {
622+
None => {
623+
dlg.finished(false);
624+
return Result::Cancelled
625+
}
626+
Some(Err(err)) => {
622627
dlg.finished(false);
623628
return Result::HttpError(err)
624629
}
625-
Ok(upload_result) => res = upload_result,
630+
Some(Ok(upload_result)) => {
631+
res = upload_result;
632+
if !res.status.is_success() {
633+
dlg.finished(false);
634+
return Result::Failure(res)
635+
}
636+
}
626637
}
627638
}
628639
let result_value = {

‎src/mako/lib/mbuild.mako‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ match result {
361361
Result::HttpError(err) => println!("HTTPERROR: {:?}", err),
362362
Result::MissingAPIKey => println!("Auth: Missing API Key - used if there are no scopes"),
363363
Result::MissingToken => println!("OAuth2: Missing Token"),
364+
Result::Cancelled => println!("Operation cancelled by user"),
364365
Result::UploadSizeLimitExceeded(size, max_size) => println!("Upload size too big: {} of {}", size, max_size),
365366
Result::Failure(_) => println!("General Failure (hyper::client::Response doesn't print)"),
366367
Result::FieldClash(clashed_field) => println!("You added custom parameter which is part of builder: {:?}", clashed_field),
@@ -760,8 +761,7 @@ else {
760761
if !res.status.is_success() {
761762
let mut json_err = String::new();
762763
res.read_to_string(&mut json_err).unwrap();
763-
let error_info: cmn::JsonServerError = json::from_str(&json_err).unwrap();
764-
if let oauth2::Retry::After(d) = dlg.http_failure(&res, Some(error_info)) {
764+
if let oauth2::Retry::After(d) = dlg.http_failure(&res, json::from_str(&json_err).ok()) {
765765
sleep(d);
766766
continue;
767767
}
@@ -792,14 +792,25 @@ else {
792792
}.upload()
793793
};
794794
match upload_result {
795-
Err(err) => {
795+
None => {
796+
${delegate_finish}(false);
797+
return Result::Cancelled
798+
}
799+
Some(Err(err)) => {
796800
## Do not ask the delgate again, as it was asked by the helper !
797801
${delegate_finish}(false);
798802
return Result::HttpError(err)
799803
}
800804
## Now the result contains the actual resource, if any ... it will be
801805
## decoded next
802-
Ok(upload_result) => res = upload_result,
806+
Some(Ok(upload_result)) => {
807+
res = upload_result;
808+
if !res.status.is_success() {
809+
## delegate was called in upload() already - don't tell him again
810+
${delegate_finish}(false);
811+
return Result::Failure(res)
812+
}
813+
}
803814
}
804815
}
805816
% endif

‎src/rust/cmn.rs‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,9 @@ pub enum Result<T = ()> {
226226
/// We required a Token, but didn't get one from the Authenticator
227227
MissingToken,
228228

229+
/// The delgate instructed to cancel the operation
230+
Cancelled,
231+
229232
/// An additional, free form field clashed with one of the built-in optional ones
230233
FieldClash(&'static str),
231234

@@ -534,12 +537,15 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A>
534537
}
535538
}
536539

537-
pub fn upload(&mut self) -> hyper::HttpResult<hyper::client::Response> {
540+
/// returns None if operation was cancelled by delegate, or the HttpResult.
541+
/// It can be that we return the result just because we didn't understand the status code -
542+
/// caller should check for status himself before assuming it's OK to use
543+
pub fn upload(&mut self) -> Option<hyper::HttpResult<hyper::client::Response>> {
538544
let mut start = match self.start_at {
539545
Some(s) => s,
540546
None => match self.query_transfer_status() {
541547
(Some(s), _) => s,
542-
(_, result) => return result
548+
(_, result) => return Some(result)
543549
}
544550
};
545551

@@ -563,32 +569,34 @@ impl<'a, NC, A> ResumableUploadHelper<'a, NC, A>
563569
};
564570
start += request_size;
565571
if self.delegate.cancel_chunk_upload(&range_header) {
566-
return Err(hyper::error::HttpError::HttpStatusError)
572+
return None
567573
}
568574
match self.client.post(self.url)
569575
.header(range_header)
570576
.header(ContentType(self.media_type.clone()))
571577
.header(UserAgent(self.user_agent.to_string()))
572578
.body(&mut section_reader)
573579
.send() {
574-
Ok(res) => {
580+
Ok(mut res) => {
575581
if res.status == StatusCode::PermanentRedirect {
576582
continue
577583
}
578-
if res.status != StatusCode::Ok {
579-
if let Retry::After(d) = self.delegate.http_failure(&res, None) {
584+
if !res.status.is_success() {
585+
let mut json_err = String::new();
586+
res.read_to_string(&mut json_err).unwrap();
587+
if let Retry::After(d) = self.delegate.http_failure(&res, serde::json::from_str(&json_err).ok()) {
580588
sleep(d);
581589
continue;
582590
}
583591
}
584-
return Ok(res)
592+
return Some(Ok(res))
585593
},
586594
Err(err) => {
587595
if let Retry::After(d) = self.delegate.http_error(&err) {
588596
sleep(d);
589597
continue;
590598
}
591-
return Err(err)
599+
return Some(Err(err))
592600
}
593601
}
594602
}

0 commit comments

Comments
 (0)
⚡