From 3b18638a63389c214cefaaaedd5643b6881eb0aa Mon Sep 17 00:00:00 2001 From: abraunegg Date: Wed, 9 Dec 2020 14:18:16 +1100 Subject: [PATCH] Remove duplicate code for error output functions and enhance error logging output (#1170) * Remove duplicate code for error output functions and enhance error logging output --- src/monitor.d | 12 +--- src/onedrive.d | 48 ++++--------- src/sync.d | 191 +++++++++++++++++++++---------------------------- src/upload.d | 15 +--- src/util.d | 89 +++++++++++++++++++++++ 5 files changed, 190 insertions(+), 165 deletions(-) diff --git a/src/monitor.d b/src/monitor.d index 1dceea9e..06aac0d7 100644 --- a/src/monitor.d +++ b/src/monitor.d @@ -151,7 +151,7 @@ final class Monitor // catch any error which is generated } catch (std.file.FileException e) { // Standard filesystem error - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); return; } catch (Exception e) { // Issue #1154 handling @@ -166,7 +166,7 @@ final class Monitor exit(-1); } else { // some other error - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -388,12 +388,4 @@ final class Monitor } } } - - // Parse and display error message received from the local file system - private void displayFileSystemErrorMessage(string message) - { - log.error("ERROR: The local file system returned an error with the following message:"); - auto errorArray = splitLines(message); - log.error(" Error Message: ", errorArray[0]); - } } diff --git a/src/onedrive.d b/src/onedrive.d index 2c0bf056..190b3ec8 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -8,6 +8,7 @@ import core.thread, std.conv, std.math; import std.algorithm.searching; import progress; import config; +import util; static import log; shared bool debugResponse = false; private bool dryRun = false; @@ -402,7 +403,7 @@ final class OneDriveApi response = cast(char[]) read(responseUrl); } catch (OneDriveException e) { // exception generated - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return false; } @@ -535,7 +536,7 @@ final class OneDriveApi remove(saveToPath); } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); } } } @@ -553,7 +554,7 @@ final class OneDriveApi newPath.setAttributes(cfg.returnRequiredDirectoryPermisions()); } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); } } @@ -780,7 +781,7 @@ final class OneDriveApi response = post(tokenUrl, postData); } catch (OneDriveException e) { // an error was generated - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } if (response.type() == JSONType.object) { @@ -796,7 +797,7 @@ final class OneDriveApi cfg.refreshTokenFilePath.setAttributes(cfg.returnRequiredFilePermisions()); } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); } } if (printAccessToken) writeln("New access token: ", accessToken); @@ -939,7 +940,7 @@ final class OneDriveApi writeln(); // Reset onProgress to not display anything for next download done using exit scope } catch (CurlException e) { - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } // free progress bar memory p = null; @@ -949,7 +950,7 @@ final class OneDriveApi // try and catch any curl error http.perform(); } catch (CurlException e) { - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } } @@ -1081,10 +1082,11 @@ final class OneDriveApi log.error("ERROR: OneDrive returned an error with the following message:"); auto errorArray = splitLines(e.msg); string errorMessage = errorArray[0]; + string defaultTimeoutErrorMessage = " Error Message: There was a timeout in accessing the Microsoft OneDrive service - Internet connectivity issue?"; - if (canFind(errorMessage, "Couldn't connect to server on handle") || canFind(errorMessage, "Couldn't resolve host name on handle")) { + if (canFind(errorMessage, "Couldn't connect to server on handle") || canFind(errorMessage, "Couldn't resolve host name on handle") || canFind(errorMessage, "Timeout was reached on handle")) { // This is a curl timeout - log.error(" Error Message: There was a timeout in accessing the Microsoft OneDrive service - Internet connectivity issue?"); + log.error(defaultTimeoutErrorMessage); // or 408 request timeout // https://github.com/abraunegg/onedrive/issues/694 // Back off & retry with incremental delay @@ -1110,8 +1112,8 @@ final class OneDriveApi log.log("Internet connectivity to Microsoft OneDrive service has been restored"); retrySuccess = true; } catch (CurlException e) { - if (canFind(e.msg, "Couldn't connect to server on handle") || canFind(e.msg, "Couldn't resolve host name on handle")) { - log.error(" Error Message: There was a timeout in accessing the Microsoft OneDrive service - Internet connectivity issue?"); + if (canFind(e.msg, "Couldn't connect to server on handle") || canFind(e.msg, "Couldn't resolve host name on handle") || canFind(errorMessage, "Timeout was reached on handle")) { + log.error(defaultTimeoutErrorMessage); // Increment & loop around retryAttempts++; } @@ -1129,6 +1131,7 @@ final class OneDriveApi } else { // Some other error was returned log.error(" Error Message: ", errorMessage); + log.error(" Calling Function: ", getFunctionName!({})); } // return an empty JSON for handling return json; @@ -1365,29 +1368,6 @@ final class OneDriveApi } } } - - // Parse and display error message received from OneDrive - private void displayOneDriveErrorMessage(string message) { - log.error("\nERROR: OneDrive returned an error with the following message:"); - auto errorArray = splitLines(message); - log.error(" Error Message: ", errorArray[0]); - // Strip cause from error to leave a JSON - JSONValue errorMessage = parseJSON(replace(message, errorArray[0], "")); - // extra debug - log.vdebug("Raw Error Data: ", message); - log.vdebug("JSON Message: ", errorMessage); - if (errorMessage.type() == JSONType.object) { - log.error(" Error Reason: ", errorMessage["error_description"].str); - } - } - - // Parse and display error message received from the local file system - private void displayFileSystemErrorMessage(string message) - { - log.error("ERROR: The local file system returned an error with the following message:"); - auto errorArray = splitLines(message); - log.error(" Error Message: ", errorArray[0]); - } } unittest diff --git a/src/sync.d b/src/sync.d index dcf46d1e..46983c01 100644 --- a/src/sync.d +++ b/src/sync.d @@ -297,7 +297,7 @@ final class SyncEngine log.vdebug("oneDriveDetails = onedrive.getDefaultDrive() generated a OneDriveException"); if (e.httpStatusCode == 400) { // OneDrive responded with 400 error: Bad Request - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); // Check this if (cfg.getValueString("drive_id").length) { @@ -308,7 +308,7 @@ final class SyncEngine } if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); log.errorAndNotify("\nERROR: Check your configuration as your refresh_token may be empty or invalid. You may need to issue a --logout and re-authorise this client.\n"); // Must exit here exit(-1); @@ -324,7 +324,7 @@ final class SyncEngine } if (e.httpStatusCode >= 500) { // There was a HTTP 5xx Server Side Error - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); // Must exit here exit(-1); } @@ -337,7 +337,7 @@ final class SyncEngine log.vdebug("oneDriveRootDetails = onedrive.getDefaultRoot() generated a OneDriveException"); if (e.httpStatusCode == 400) { // OneDrive responded with 400 error: Bad Request - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); // Check this if (cfg.getValueString("drive_id").length) { log.error("\nERROR: Check your 'drive_id' entry in your configuration file as it may be incorrect\n"); @@ -347,7 +347,7 @@ final class SyncEngine } if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); log.errorAndNotify("\nERROR: Check your configuration as your refresh_token may be empty or invalid. You may need to issue a --logout and re-authorise this client.\n"); // Must exit here exit(-1); @@ -363,7 +363,7 @@ final class SyncEngine } if (e.httpStatusCode >= 500) { // There was a HTTP 5xx Server Side Error - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); // Must exit here exit(-1); } @@ -1327,7 +1327,7 @@ final class SyncEngine // HTTP request returned status code 500 (Internal Server Error) if (e.httpStatusCode == 500) { // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } @@ -1352,13 +1352,13 @@ final class SyncEngine // display what the error is log.vdebug("Query Error: changes = generateDeltaResponse(driveId, idToQuery) on re-try after delay"); // error was not a 504 this time - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } else { // Default operation if not 404, 410, 429, 500 or 504 errors // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -1368,13 +1368,14 @@ final class SyncEngine try { // Fetch the changes relative to the path id we want to query log.vdebug("Attempting query 'changes = onedrive.viewChangesByItemId(driveId, idToQuery, deltaLink)'"); - log.vdebug("driveId: ", driveId); - log.vdebug("idToQuery: ", idToQuery); - log.vdebug("deltaLink: ", deltaLink); + log.vdebug("driveId: ", driveId); + log.vdebug("idToQuery: ", idToQuery); + log.vdebug("Previous deltaLink: ", deltaLink); // changes with or without deltaLink changes = onedrive.viewChangesByItemId(driveId, idToQuery, deltaLink); if (changes.type() == JSONType.object) { log.vdebug("Query 'changes = onedrive.viewChangesByItemId(driveId, idToQuery, deltaLink)' performed successfully"); + log.vdebug("OneDrive API /delta response: ", changes); } } catch (OneDriveException e) { // OneDrive threw an error @@ -1407,7 +1408,7 @@ final class SyncEngine // HTTP request returned status code 500 (Internal Server Error) if (e.httpStatusCode == 500) { // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } @@ -1441,12 +1442,12 @@ final class SyncEngine log.vdebug("Query 'changes = onedrive.viewChangesByItemId(driveId, idToQuery, deltaLink)' performed successfully on re-try"); } catch (OneDriveException e) { // Tried 3 times, give up - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } else { // error was not a 504 this time - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -1459,7 +1460,7 @@ final class SyncEngine itemdb.setDeltaLink(driveId, idToQuery, emptyDeltaLink); } // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -1512,7 +1513,7 @@ final class SyncEngine // HTTP request returned status code 500 (Internal Server Error) if (e.httpStatusCode == 500) { // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } @@ -1546,19 +1547,19 @@ final class SyncEngine log.vdebug("Query 'changesAvailable = onedrive.viewChangesByItemId(driveId, idToQuery, deltaLinkAvailable)' performed successfully on re-try"); } catch (OneDriveException e) { // Tried 3 times, give up - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } else { // error was not a 504 this time - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } } else { // Default operation if not 404, 410, 429, 500 or 504 errors // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -1784,12 +1785,12 @@ final class SyncEngine log.vlog("Remote change discarded - item cannot be found"); } else { // not a 404 - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } } } else { // not a 404 or a 429 - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } } @@ -2455,7 +2456,7 @@ final class SyncEngine } } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); // flag that this failed downloadFailed = true; return; @@ -2517,7 +2518,7 @@ final class SyncEngine setTimes(newPath, newItem.mtime, newItem.mtime); } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); } } } @@ -2630,7 +2631,7 @@ final class SyncEngine retryAttempts++; } } else { - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } } } @@ -2665,7 +2666,7 @@ final class SyncEngine retryAttempts++; } } else { - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); } } } @@ -2673,13 +2674,13 @@ final class SyncEngine } catch (FileException e) { // There was a file system error // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); downloadFailed = true; return; } catch (std.exception.ErrnoException e) { // There was a file system error // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); downloadFailed = true; return; } @@ -2698,7 +2699,7 @@ final class SyncEngine setTimes(path, item.mtime, item.mtime); } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); } } else { // size error? @@ -2814,7 +2815,7 @@ final class SyncEngine rmdirRecurse(path); } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); } } } @@ -3365,14 +3366,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -3402,14 +3403,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -3460,14 +3461,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -3522,14 +3523,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -3858,7 +3859,7 @@ final class SyncEngine } } catch (FileException e) { // display the error message - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); return; } } else { @@ -4033,7 +4034,7 @@ final class SyncEngine } else { // some other error from OneDrive was returned - display what it is log.error("OneDrive generated an error when creating this path: ", path); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -4269,14 +4270,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4326,7 +4327,7 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4334,14 +4335,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4376,14 +4377,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4419,14 +4420,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4614,7 +4615,7 @@ final class SyncEngine // error uploading file // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4622,14 +4623,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4668,14 +4669,14 @@ final class SyncEngine // error uploading file // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4750,14 +4751,14 @@ final class SyncEngine // error uploading file // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4787,14 +4788,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4816,14 +4817,14 @@ final class SyncEngine } else { // display what the error is writeln("skipped."); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } } catch (FileException e) { // display the error message writeln("skipped."); - displayFileSystemErrorMessage(e.msg); + displayFileSystemErrorMessage(e.msg, getFunctionName!({})); uploadFailed = true; return; } @@ -4989,7 +4990,7 @@ final class SyncEngine } catch (OneDriveException e) { // display what the error is log.vdebug("A further error was generated when attempting a reverse delete of objects from OneDrive"); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -5017,7 +5018,7 @@ final class SyncEngine } catch (OneDriveException e) { // display what the error is log.vdebug("A further error was generated when attempting a reverse delete of objects from OneDrive"); - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -5025,7 +5026,7 @@ final class SyncEngine // Not a 403 response & OneDrive Business Account / O365 Shared Folder / Library log.vdebug("onedrive.deleteById generated an error response when attempting to delete object by item id"); // display what the error is - displayOneDriveErrorMessage(e.msg); + displayOneDriveErrorMessage(e.msg, getFunctionName!({})); return; } } @@ -5107,34 +5108,6 @@ final class SyncEngine } } - // Parse and display error message received from OneDrive - private void displayOneDriveErrorMessage(string message) - { - log.error("\nERROR: OneDrive returned an error with the following message:"); - auto errorArray = splitLines(message); - log.error(" Error Message: ", errorArray[0]); - // extract 'message' as the reason - JSONValue errorMessage = parseJSON(replace(message, errorArray[0], "")); - string errorReason = errorMessage["error"]["message"].str; - // display reason - if (errorReason.startsWith("