From d169dfc642c1beab48c4dac52d7fe5fc4f3c3c06 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Thu, 18 Aug 2022 09:14:13 +1000 Subject: [PATCH] Improve application logging output for error messages (#2100) * When enabling system logging to a log file, the actual ERROR line is forced to a new line in the application log. The reason for this is the \n prefix in the error message, which was in place so that when performing CLI logging or systemd logging, the error message would be displayed clearly. This change removes the \n from the actual error message, but inserts a newline before the error message is displayed (and also in some cases post error message) - thus keeping the application runtime look and feel, but improving the application log output. --- src/itemdb.d | 9 ++++++--- src/main.d | 7 +++++-- src/onedrive.d | 17 +++++++++++----- src/sync.d | 53 ++++++++++++++++++++++++++------------------------ src/util.d | 6 ++++-- 5 files changed, 55 insertions(+), 37 deletions(-) diff --git a/src/itemdb.d b/src/itemdb.d index 955e7d5b..72a097e1 100644 --- a/src/itemdb.d +++ b/src/itemdb.d @@ -52,11 +52,14 @@ final class ItemDatabase } catch (SqliteException e) { // An error was generated - what was the error? if (e.msg == "database is locked") { - log.error("\nERROR: onedrive application is already running - check system process list for active application instances"); + writeln(); + log.error("ERROR: onedrive application is already running - check system process list for active application instances"); log.vlog(" - Use 'sudo ps aufxw | grep onedrive' to potentially determine acive running process"); - write("\n"); + writeln(); } else { - log.error("\nERROR: An internal database error occurred: " ~ e.msg ~ "\n"); + writeln(); + log.error("ERROR: An internal database error occurred: " ~ e.msg); + writeln(); } exit(-1); } diff --git a/src/main.d b/src/main.d index 256c5d31..c56888b7 100644 --- a/src/main.d +++ b/src/main.d @@ -903,8 +903,11 @@ int main(string[] args) // if --synchronize && --monitor passed in, exit & display help as these conflict with each other if (cfg.getValueBool("synchronize") && cfg.getValueBool("monitor")) { - writeln("\nERROR: --synchronize and --monitor cannot be used together\n"); - writeln("Please use 'onedrive --help' for further assistance in regards to running this application.\n"); + writeln(); + writeln("ERROR: --synchronize and --monitor cannot be used together"); + writeln(); + writeln("Please use 'onedrive --help' for further assistance in regards to running this application."); + writeln(); // Use exit scopes to shutdown API return EXIT_FAILURE; } diff --git a/src/onedrive.d b/src/onedrive.d index f5278325..0f5df2df 100644 --- a/src/onedrive.d +++ b/src/onedrive.d @@ -1097,12 +1097,16 @@ final class OneDriveApi if ("scope" in response){ string effectiveScopes = response["scope"].str(); // Display the effective authentication scopes - writeln("\nEffective API Authentication Scopes: ", effectiveScopes); + writeln(); + writeln("Effective API Authentication Scopes: ", effectiveScopes); // if we have any write scopes, we need to tell the user to update an remove online prior authentication and exit application if (canFind(effectiveScopes, "Write")) { // effective scopes contain write scopes .. so not a read-only configuration - writeln("\nERROR: You have authentication scopes that allow write operations. You need to remove your existing application access consent"); - writeln("\nPlease login to https://account.live.com/consent/Manage and remove your existing application access consent\n"); + writeln(); + writeln("ERROR: You have authentication scopes that allow write operations. You need to remove your existing application access consent"); + writeln(); + writeln("Please login to https://account.live.com/consent/Manage and remove your existing application access consent"); + writeln(); // force exit shutdown(); exit(-1); @@ -1145,7 +1149,9 @@ final class OneDriveApi } catch (OneDriveException e) { if (e.httpStatusCode == 400 || e.httpStatusCode == 401) { // flag error and notify - log.errorAndNotify("\nERROR: Refresh token invalid, use --reauth to authorize the client again.\n"); + writeln(); + log.errorAndNotify("ERROR: Refresh token invalid, use --reauth to authorize the client again."); + writeln(); // set error message e.msg ~= "\nRefresh token invalid, use --reauth to authorize the client again"; } @@ -1493,7 +1499,8 @@ final class OneDriveApi retryAttempts++; 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")) { // no access to Internet - log.error("\nERROR: There was a timeout in accessing the Microsoft OneDrive service - Internet connectivity issue?"); + writeln(); + log.error("ERROR: There was a timeout in accessing the Microsoft OneDrive service - Internet connectivity issue?"); // what is the error reason to assis the user as what to check if (canFind(e.msg, "Couldn't connect to server on handle")) { log.log(" - Check HTTPS access or Firewall Rules"); diff --git a/src/sync.d b/src/sync.d index e3273f97..a03ecd69 100644 --- a/src/sync.d +++ b/src/sync.d @@ -312,7 +312,9 @@ final class SyncEngine // 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"); + writeln(); + log.error("ERROR: Check your 'drive_id' entry in your configuration file as it may be incorrect"); + writeln(); } // Must exit here onedrive.shutdown(); @@ -321,10 +323,7 @@ final class SyncEngine if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) 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 --reauth and re-authorise this client.\n"); - // Must exit here - onedrive.shutdown(); - exit(-1); + handleClientUnauthorised(); } if (e.httpStatusCode == 429) { // HTTP request returned status code 429 (Too Many Requests). We need to leverage the response Retry-After HTTP header to ensure minimum delay until the throttle is removed. @@ -354,7 +353,9 @@ final class SyncEngine 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"); + writeln(); + log.error("ERROR: Check your 'drive_id' entry in your configuration file as it may be incorrect"); + writeln(); } // Must exit here onedrive.shutdown(); @@ -363,10 +364,7 @@ final class SyncEngine if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) 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 --reauth and re-authorise this client.\n"); - // Must exit here - onedrive.shutdown(); - exit(-1); + handleClientUnauthorised(); } if (e.httpStatusCode == 429) { // HTTP request returned status code 429 (Too Many Requests). We need to leverage the response Retry-After HTTP header to ensure minimum delay until the throttle is removed. @@ -677,10 +675,7 @@ final class SyncEngine if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) 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 --reauth and re-authorise this client.\n"); - // Must exit here - onedrive.shutdown(); - exit(-1); + handleClientUnauthorised(); } if (e.httpStatusCode == 429) { // HTTP request returned status code 429 (Too Many Requests). We need to leverage the response Retry-After HTTP header to ensure minimum delay until the throttle is removed. @@ -873,10 +868,7 @@ final class SyncEngine if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) 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 --reauth and re-authorise this client.\n"); - // Must exit here - onedrive.shutdown(); - exit(-1); + handleClientUnauthorised(); } if (e.httpStatusCode == 429) { // HTTP request returned status code 429 (Too Many Requests). We need to leverage the response Retry-After HTTP header to ensure minimum delay until the throttle is removed. @@ -6102,7 +6094,8 @@ final class SyncEngine if ("id" in searchResult) idAvailable = true; // Display error details for this site data - log.error("\nERROR: SharePoint Site details not provided for: ", siteNameAvailable); + writeln(); + log.error("ERROR: SharePoint Site details not provided for: ", siteNameAvailable); log.error("ERROR: The SharePoint Site results returned from OneDrive API do not contain the required items to match. Please check your permissions with your site administrator."); log.error("ERROR: Your site security settings is preventing the following details from being accessed: 'displayName' or 'id'"); log.vlog(" - Is 'displayName' available = ", displayNameAvailable); @@ -6151,9 +6144,11 @@ final class SyncEngine // Was the intended target found? if(!found) { - log.error("\nERROR: The requested SharePoint site could not be found. Please check it's name and your permissions to access the site."); + writeln(); + log.error("ERROR: The requested SharePoint site could not be found. Please check it's name and your permissions to access the site."); // List all sites returned to assist user - log.log("\nThe following SharePoint site names were returned:"); + writeln(); + log.log("The following SharePoint site names were returned:"); foreach (searchResultEntry; siteSearchResults) { // list the display name that we use to match against the user query log.log(searchResultEntry); @@ -6904,10 +6899,7 @@ final class SyncEngine if (e.httpStatusCode == 401) { // HTTP request returned status code 401 (Unauthorized) 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 --reauth and re-authorise this client.\n"); - // Must exit here - onedrive.shutdown(); - exit(-1); + handleClientUnauthorised(); } if (e.httpStatusCode == 429) { // HTTP request returned status code 429 (Too Many Requests). We need to leverage the response Retry-After HTTP header to ensure minimum delay until the throttle is removed. @@ -6998,4 +6990,15 @@ final class SyncEngine // return calculated path as string return calculatedPath; } + + void handleClientUnauthorised() + { + // common code for handling when a client is unauthorised + writeln(); + log.errorAndNotify("ERROR: Check your configuration as your refresh_token may be empty or invalid. You may need to issue a --reauth and re-authorise this client."); + writeln(); + // Must exit here + onedrive.shutdown(); + exit(-1); + } } diff --git a/src/util.d b/src/util.d index 41f5d3a6..62ab75bb 100644 --- a/src/util.d +++ b/src/util.d @@ -269,7 +269,8 @@ bool containsASCIIHTMLCodes(string path) // Parse and display error message received from OneDrive void displayOneDriveErrorMessage(string message, string callingFunction) { - log.error("\nERROR: Microsoft OneDrive API returned an error with the following message:"); + writeln(); + log.error("ERROR: Microsoft OneDrive API returned an error with the following message:"); auto errorArray = splitLines(message); log.error(" Error Message: ", errorArray[0]); // Extract 'message' as the reason @@ -339,7 +340,8 @@ void displayOneDriveErrorMessage(string message, string callingFunction) // Parse and display error message received from the local file system void displayFileSystemErrorMessage(string message, string callingFunction) { - log.error("\nERROR: The local file system returned an error with the following message:"); + writeln(); + log.error("ERROR: The local file system returned an error with the following message:"); auto errorArray = splitLines(message); // What was the error message log.error(" Error Message: ", errorArray[0]);