From 3fdff89ca7bb613b0549e7e340445371a22cffff Mon Sep 17 00:00:00 2001
From: Blake Niemyjski <bniemyjski@gmail.com>
Date: Tue, 28 Feb 2023 21:24:37 -0600
Subject: [PATCH] Fixes #53 Ensure storage api calls error are caught and
 logged

---
 .vscode/settings.json                         |  1 -
 .../core/src/configuration/SettingsManager.ts | 14 +++++++------
 packages/core/src/queue/DefaultEventQueue.ts  | 21 ++++++++++++-------
 3 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/.vscode/settings.json b/.vscode/settings.json
index 8c6cbc0a..03c97dbf 100644
--- a/.vscode/settings.json
+++ b/.vscode/settings.json
@@ -23,7 +23,6 @@
     "esbuild",
     "eslintignore",
     "jsdelivr",
-    "lage",
     "localstorage",
     "maxcdn",
     "ncaught",
diff --git a/packages/core/src/configuration/SettingsManager.ts b/packages/core/src/configuration/SettingsManager.ts
index 399c39db..436833c7 100644
--- a/packages/core/src/configuration/SettingsManager.ts
+++ b/packages/core/src/configuration/SettingsManager.ts
@@ -28,9 +28,9 @@ export class SettingsManager {
     }
 
     this._isUpdatingSettings = true;
-    const { log } = config.services;
-    try {
+    const { log, storage, submissionClient } = config.services;
 
+    try {
       const unableToUpdateMessage = "Unable to update settings";
       if (!config.isValid) {
         log.error(`${unableToUpdateMessage}: ApiKey is not set`);
@@ -39,7 +39,7 @@ export class SettingsManager {
 
       const version = config.settingsVersion;
       log.trace(`Checking for updated settings from: v${version}`);
-      const response = await config.services.submissionClient.getSettings(version);
+      const response = await submissionClient.getSettings(version);
 
       if (response.status === 304) {
         log.trace("Settings are up-to-date");
@@ -53,7 +53,7 @@ export class SettingsManager {
 
       config.applyServerSettings(response.data);
 
-      await config.services.storage.setItem(SettingsManager.SettingsKey, JSON.stringify(response.data));
+      await storage.setItem(SettingsManager.SettingsKey, JSON.stringify(response.data));
       log.trace(`Updated settings: v${response.data.version}`);
     } catch (ex) {
       log.error(`Error updating settings: ${ex instanceof Error ? ex.message : ex + ''}`);
@@ -63,10 +63,12 @@ export class SettingsManager {
   }
 
   private static async getSavedServerSettings(config: Configuration): Promise<ServerSettings> {
+    const { log, storage } = config.services;
     try {
-      const settings = await config.services.storage.getItem(SettingsManager.SettingsKey);
+      const settings = await storage.getItem(SettingsManager.SettingsKey);
       return settings && JSON.parse(settings) as ServerSettings || new ServerSettings({}, 0);
-    } catch {
+    } catch (ex) {
+      log.error(`Error getting saved settings: ${ex instanceof Error ? ex.message : ex + ''}`);
       return new ServerSettings({}, 0);
     }
   }
diff --git a/packages/core/src/queue/DefaultEventQueue.ts b/packages/core/src/queue/DefaultEventQueue.ts
index 1fa3a4b1..579c5729 100644
--- a/packages/core/src/queue/DefaultEventQueue.ts
+++ b/packages/core/src/queue/DefaultEventQueue.ts
@@ -257,8 +257,9 @@ export class DefaultEventQueue implements IEventQueue {
 
   private async loadEvents(): Promise<void> {
     if (this.config.usePersistedQueueStorage) {
+      const { log, storage } = this.config.services;
+
       try {
-        const storage = this.config.services.storage;
         const files: string[] = await storage.keys();
 
         for (const file of files) {
@@ -269,7 +270,7 @@ export class DefaultEventQueue implements IEventQueue {
           }
         }
       } catch (ex) {
-        this.config.services.log.error(`Error loading queue items from storage: ${ex instanceof Error ? ex.message : ex + ''}`)
+        log.error(`Error loading queue items from storage: ${ex instanceof Error ? ex.message : ex + ''}`)
       }
     }
   }
@@ -278,13 +279,17 @@ export class DefaultEventQueue implements IEventQueue {
     this._lastFileTimestamp = Math.max(Date.now(), this._lastFileTimestamp + 1);
     const file = `${this.QUEUE_PREFIX}${this._lastFileTimestamp}.json`;
 
-    const { storage, log } = this.config.services;
+    const { log, storage } = this.config.services;
     const useStorage: boolean = this.config.usePersistedQueueStorage;
     if (this._queue.push({ file, event }) > this.maxItems) {
       log.trace("Removing oldest queue entry: maxItems exceeded");
       const item = this._queue.shift();
       if (useStorage && item) {
-        await storage.removeItem(item.file);
+        try {
+          await storage.removeItem(item.file);
+        } catch (ex) {
+          log.error(`Error removing oldest queue entry from storage: ${ex instanceof Error ? ex.message : ex + ''}`)
+        }
       }
     }
 
@@ -292,7 +297,7 @@ export class DefaultEventQueue implements IEventQueue {
       try {
         await storage.setItem(file, JSON.stringify(event));
       } catch (ex) {
-        log.error(`Error saving queue item to storage: ${ex instanceof Error ? ex.message : ex + ''}`)
+        log.error(`Error saving queue entry to storage: ${ex instanceof Error ? ex.message : ex + ''}`)
       }
     }
 
@@ -302,11 +307,13 @@ export class DefaultEventQueue implements IEventQueue {
   private async removeEvents(items: EventQueueItem[]): Promise<void> {
     const files = items.map(i => i.file);
     if (this.config.usePersistedQueueStorage) {
+      const { log, storage } = this.config.services;
+
       for (const file of files) {
         try {
-          await this.config.services.storage.removeItem(file);
+          await storage.removeItem(file);
         } catch (ex) {
-          this.config.services.log.error(`Error removing queue item from storage: ${ex instanceof Error ? ex.message : ex + ''}`)
+          log.error(`Error removing queue item from storage: ${ex instanceof Error ? ex.message : ex + ''}`)
         }
       }
     }