From 5cd860a398fda85e8bf1885376e9f6c1603936d3 Mon Sep 17 00:00:00 2001
From: abraunegg <alex.braunegg@gmail.com>
Date: Sat, 31 Aug 2019 05:27:49 +1000
Subject: [PATCH] Fix handling of 5xx responses from OneDrive when uploading
 via a session (Issue #632) (#631)

* Fix handling of 5xx responses from OneDrive when performing a session upload
* Switch to same checks when doing non session upload so that OneDrive exceptions are thrown correctly
* Remove a 'throw e' when curl times out
* Remove a needless 'throw e' when a session upload cannot be found
* Set the upload failed flag when OneDrive errors on session uploads
---
 src/onedrive.d | 23 +++++++++++++++++++++--
 src/sync.d     |  9 ++++++++-
 src/upload.d   | 16 ++++++++++++----
 3 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/src/onedrive.d b/src/onedrive.d
index f3962ab4..2099cbf4 100644
--- a/src/onedrive.d
+++ b/src/onedrive.d
@@ -364,7 +364,7 @@ final class OneDriveApi
 		http.contentLength = offsetSize;
 		auto response = perform();
 		// TODO: retry on 5xx errors
-		checkHttpCode();
+		checkHttpCode(response);
 		return response;
 	}
 
@@ -620,7 +620,6 @@ final class OneDriveApi
 		} catch (CurlException e) {
 			// Potentially Timeout was reached on handle error
 			log.error("\nThere was a problem in accessing the Microsoft OneDrive service - Internet connectivity issue?\n");
-			throw e;
 		}
 		
 		JSONValue json;
@@ -680,6 +679,7 @@ final class OneDriveApi
 	
 		switch(http.statusLine.code)
 		{
+			//  0 - OK ... HTTP2 version of 200 OK
 			case 0:
 				break;
 			//	200 - OK
@@ -776,6 +776,25 @@ final class OneDriveApi
 	{
 		switch(http.statusLine.code)
 		{
+			//  0 - OK ... HTTP2 version of 200 OK
+			case 0:
+				break;
+			//	200 - OK
+			case 200:
+				// No Log .. 
+				break;
+			//	201 - Created OK
+			//  202 - Accepted
+			//	204 - Deleted OK
+			case 201,202,204:
+				// No actions, but log if verbose logging
+				//log.vlog("OneDrive Response: '", http.statusLine.code, " - ", http.statusLine.reason, "'");
+				break;
+				
+			// 302 - resource found and available at another location, redirect
+			case 302:
+				break;
+			
 			// 400 - Bad Request
 			case 400:
 				// Bad Request .. how should we act?
diff --git a/src/sync.d b/src/sync.d
index bb95ca76..87236fd8 100644
--- a/src/sync.d
+++ b/src/sync.d
@@ -2417,7 +2417,8 @@ final class SyncEngine
 								} else {
 									// response is not valid JSON, an error was returned from OneDrive
 									log.fileOnly("Uploading new file ", path, " ... error");
-									writeln(" error");
+									writeln("error");
+									uploadFailed = true;
 									return;
 								}
 							} else {
@@ -2434,6 +2435,7 @@ final class SyncEngine
 					
 						if (e.httpStatusCode >= 500) {
 							// OneDrive returned a 'HTTP 5xx Server Side Error' - gracefully handling error - error message already logged
+							uploadFailed = true;
 							return;
 						}
 					}
@@ -2709,14 +2711,19 @@ final class SyncEngine
 						// fileDetailsFromOneDrive is not valid JSON, an error was returned from OneDrive
 						log.error("ERROR: An error was returned from OneDrive and the resulting response is not a valid JSON object");
 						log.error("ERROR: Increase logging verbosity to assist determining why.");
+						uploadFailed = true;
+						return;
 					}
 				} else {
 					// Skip file - too large
 					log.log("Skipping uploading this new file as it exceeds the maximum size allowed by OneDrive: ", path);
+					uploadFailed = true;
+					return;
 				}
 			}
 		} else {
 			log.log("Skipping uploading this new file as parent path is not in the database: ", path);
+			uploadFailed = true;
 			return;
 		}
 	}
diff --git a/src/upload.d b/src/upload.d
index a2a67d64..f2f86ab9 100644
--- a/src/upload.d
+++ b/src/upload.d
@@ -89,8 +89,6 @@ struct UploadSession
 						if (e.httpStatusCode == 400) {
 							log.vlog("Upload session not found");
 							return false;
-						} else {
-							throw e;
 						}
 					}
 					
@@ -178,14 +176,24 @@ struct UploadSession
 						fragSize,
 						fileSize
 					);
+				} catch (OneDriveException e) {
+					// there was an error remove session file
+					if (exists(sessionFilePath)) {
+						remove(sessionFilePath);
+					}
+					return response;
+				}
+				// fragment uploaded without issue
+				if (response.type() == JSONType.object){
 					offset += fragmentSize;
 					if (offset >= fileSize) break;
 					// update the session details
 					session["expirationDateTime"] = response["expirationDateTime"];
 					session["nextExpectedRanges"] = response["nextExpectedRanges"];
 					save();
-				} catch (OneDriveException e) {
-					// there was an error remove session file
+				} else {
+					// not a JSON object
+					log.vlog("File upload session failed - invalid response from OneDrive");
 					if (exists(sessionFilePath)) {
 						remove(sessionFilePath);
 					}