Rework OneDrive generating a 412 'Precondition Failed' Error

* Rework the 412 fix so that the OneDrive client gracefully handles the
error & retries the metadata update without the eTag/cTag causing the
issue.
* Fix the double logging to console of 'upload' and 'download' items
This commit is contained in:
abraunegg 2018-05-03 16:21:53 +10:00
parent 90dec8d6f1
commit 0b5dc66507
3 changed files with 81 additions and 59 deletions

View file

@ -21,6 +21,12 @@ void log(T...)(T args)
logfileWriteLine(args); logfileWriteLine(args);
} }
void fileOnly(T...)(T args)
{
// Write to log file only
logfileWriteLine(args);
}
void vlog(T...)(T args) void vlog(T...)(T args)
{ {
if (verbose) { if (verbose) {

View file

@ -478,59 +478,62 @@ final class OneDriveApi
switch(http.statusLine.code) switch(http.statusLine.code)
{ {
// 200 - OK
// case 1,2,3,4: case 200:
// No Log ..
// 200 - OK break;
// 201 - Created OK // 201 - Created OK
// 202 - Accepted // 202 - Accepted
// 204 - Deleted OK // 204 - Deleted OK
case 200,201,202,204: case 201,202,204:
// No actions, but log if verbose logging // No actions, but log if verbose logging
log.vlog("OneDrive Response: '", http.statusLine.code, " - ", http.statusLine.reason, "'"); log.vlog("OneDrive Response: '", http.statusLine.code, " - ", http.statusLine.reason, "'");
break; break;
// 400 - Bad Request // 400 - Bad Request
case 400: case 400:
// Bad Request .. how should we act? // Bad Request .. how should we act?
log.vlog("OneDrive returned a 'HTTP 400 - Bad Request' - gracefully handling error"); log.vlog("OneDrive returned a 'HTTP 400 - Bad Request' - gracefully handling error");
break; break;
// Item not found // Item not found
case 404: case 404:
// Item was not found - do not throw an exception // Item was not found - do not throw an exception
log.vlog("OneDrive returned a 'HTTP 404 - Item not found' - gracefully handling error"); log.vlog("OneDrive returned a 'HTTP 404 - Item not found' - gracefully handling error");
break; break;
// 409 - Conflict // 409 - Conflict
case 409: case 409:
// Conflict handling .. how should we act? This only really gets triggered if we are using --local-first & we remove items.db as the DB thinks the file is not uploaded but it is // Conflict handling .. how should we act? This only really gets triggered if we are using --local-first & we remove items.db as the DB thinks the file is not uploaded but it is
log.vlog("OneDrive returned a 'HTTP 409 - Conflict' - gracefully handling error"); log.vlog("OneDrive returned a 'HTTP 409 - Conflict' - gracefully handling error");
break; break;
// 412 - Precondition Failed // 412 - Precondition Failed
case 412: case 412:
// A precondition provided in the request (such as an if-match header) does not match the resource's current state. // A precondition provided in the request (such as an if-match header) does not match the resource's current state.
log.vlog("OneDrive returned a 'HTTP 412 - Precondition Failed' - gracefully handling error"); log.vlog("OneDrive returned a 'HTTP 412 - Precondition Failed' - gracefully handling error");
break; break;
// 415 - Unsupported Media Type // 415 - Unsupported Media Type
case 415: case 415:
// Unsupported Media Type ... sometimes triggered on image files, especially PNG // Unsupported Media Type ... sometimes triggered on image files, especially PNG
log.vlog("OneDrive returned a 'HTTP 415 - Unsupported Media Type' - gracefully handling error"); log.vlog("OneDrive returned a 'HTTP 415 - Unsupported Media Type' - gracefully handling error");
break; break;
// 500 - Internal Server Error // Server side (OneDrive) Errors
// 502 - Bad Gateway // 500 - Internal Server Error
// 503 - Service Unavailable // 502 - Bad Gateway
case 500,502,503: // 503 - Service Unavailable
// No actions // 504 - Gateway Timeout (Issue #320)
break; case 500,502,503,504:
// No actions
log.vlog("OneDrive returned a 'HTTP 5xx Server Side Error' - gracefully handling error");
break;
// "else" // "else"
default: default:
throw new OneDriveException(http.statusLine.code, http.statusLine.reason); throw new OneDriveException(http.statusLine.code, http.statusLine.reason);
break; break;
} }
} }

View file

@ -404,8 +404,7 @@ final class SyncEngine
private void applyDifference(JSONValue driveItem, string driveId, bool isRoot) private void applyDifference(JSONValue driveItem, string driveId, bool isRoot)
{ {
Item item = makeItem(driveItem); Item item = makeItem(driveItem);
//log.vlog("Processing item to apply differences");
if (isItemRoot(driveItem) || !item.parentId || isRoot) { if (isItemRoot(driveItem) || !item.parentId || isRoot) {
item.parentId = null; // ensures that it has no parent item.parentId = null; // ensures that it has no parent
item.driveId = driveId; // HACK: makeItem() cannot set the driveId propery of the root item.driveId = driveId; // HACK: makeItem() cannot set the driveId propery of the root
@ -567,7 +566,7 @@ final class SyncEngine
onedrive.downloadById(item.driveId, item.id, path); onedrive.downloadById(item.driveId, item.id, path);
setTimes(path, item.mtime, item.mtime); setTimes(path, item.mtime, item.mtime);
writeln(" done."); writeln(" done.");
log.log("Downloading ", path, "... done."); log.fileOnly("Downloading ", path, "... done.");
} }
// returns true if the given item corresponds to the local one // returns true if the given item corresponds to the local one
@ -758,7 +757,7 @@ final class SyncEngine
writeln(""); writeln("");
response = session.upload(path, item.driveId, item.parentId, baseName(path), eTag); response = session.upload(path, item.driveId, item.parentId, baseName(path), eTag);
} }
log.log("Uploading file ", path, "... done."); log.fileOnly("Uploading file ", path, "... done.");
// saveItem(response); redundant // saveItem(response); redundant
// use the cTag instead of the eTag because Onedrive may update the metadata of files AFTER they have been uploaded // use the cTag instead of the eTag because Onedrive may update the metadata of files AFTER they have been uploaded
eTag = response["cTag"].str; eTag = response["cTag"].str;
@ -950,7 +949,9 @@ final class SyncEngine
writeln(""); writeln("");
response = session.upload(path, parent.driveId, parent.id, baseName(path)); response = session.upload(path, parent.driveId, parent.id, baseName(path));
} }
log.log("Uploading file ", path, "... done."); log.fileOnly("Uploading file ", path, "... done.");
// Update the item's metadata on OneDrive
string id = response["id"].str; string id = response["id"].str;
string cTag = response["cTag"].str; string cTag = response["cTag"].str;
SysTime mtime = timeLastModified(path).toUTC(); SysTime mtime = timeLastModified(path).toUTC();
@ -979,7 +980,7 @@ final class SyncEngine
writeln(""); writeln("");
response = session.upload(path, parent.driveId, parent.id, baseName(path)); response = session.upload(path, parent.driveId, parent.id, baseName(path));
} }
log.log("Uploading file ", path, "... done."); log.fileOnly("Uploading file ", path, "... done.");
string id = response["id"].str; string id = response["id"].str;
string cTag = response["cTag"].str; string cTag = response["cTag"].str;
SysTime mtime = timeLastModified(path).toUTC(); SysTime mtime = timeLastModified(path).toUTC();
@ -1027,7 +1028,19 @@ final class SyncEngine
"lastModifiedDateTime": mtime.toISOExtString() "lastModifiedDateTime": mtime.toISOExtString()
]) ])
]; ];
auto response = onedrive.updateById(driveId, id, data, eTag);
JSONValue response;
try {
response = onedrive.updateById(driveId, id, data, eTag);
} catch (OneDriveException e) {
if (e.httpStatusCode == 412) {
// OneDrive threw a 412 error, most likely: ETag does not match current item's value
// Retry without eTag
log.vlog("OneDrive returned a 'HTTP 412 - Precondition Failed' - gracefully handling error");
string nullTag = null;
response = onedrive.updateById(driveId, id, data, nullTag);
}
}
saveItem(response); saveItem(response);
} }