From f1df6c0a46713b846a96917acc5b91fe4e39ceb8 Mon Sep 17 00:00:00 2001
From: Alex Hall <alex.mojaki@gmail.com>
Date: Tue, 12 Jul 2022 16:23:03 +0200
Subject: [PATCH] (core) Prevent logging pointless errors about attachments and
 data size on shutdown

Summary: As suggested in https://grist.slack.com/archives/CR8HZ4P9V/p1652365399661569?thread_ts=1652364998.893169&cid=CR8HZ4P9V, check if DocStorage is initialized before trying to use it when shutting down, to avoid noisy logging of errors about removing attachments and updating data size.

Test Plan: Tested manually that errors early in loadDoc caused logging of errors about attachments/data size before the changed, but not after.

Reviewers: dsagal

Reviewed By: dsagal

Differential Revision: https://phab.getgrist.com/D3522
---
 app/server/lib/ActiveDoc.ts  | 34 +++++++++++++++++++---------------
 app/server/lib/DocStorage.ts |  4 ++++
 2 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/app/server/lib/ActiveDoc.ts b/app/server/lib/ActiveDoc.ts
index b0bfc044..22347b33 100644
--- a/app/server/lib/ActiveDoc.ts
+++ b/app/server/lib/ActiveDoc.ts
@@ -485,24 +485,28 @@ export class ActiveDoc extends EventEmitter {
       // We'll defer syncing usage until everything is calculated.
       const usageOptions = {syncUsageToDatabase: false, broadcastUsageToClients: false};
 
-      // Remove expired attachments, i.e. attachments that were soft deleted a while ago. This
-      // needs to happen periodically, and doing it here means we can guarantee that it happens
-      // even if the doc is only ever opened briefly, without having to slow down startup.
-      const removeAttachmentsPromise = this.removeUnusedAttachments(true, usageOptions);
+      // This cleanup requires docStorage, which may have failed to start up.
+      // We don't want to log pointless errors in that case.
+      if (this.docStorage.isInitialized()) {
+        // Remove expired attachments, i.e. attachments that were soft deleted a while ago. This
+        // needs to happen periodically, and doing it here means we can guarantee that it happens
+        // even if the doc is only ever opened briefly, without having to slow down startup.
+        const removeAttachmentsPromise = this.removeUnusedAttachments(true, usageOptions);
 
-      // Update data size; we'll be syncing both it and attachments size to the database soon.
-      const updateDataSizePromise = this._updateDataSize(usageOptions);
+        // Update data size; we'll be syncing both it and attachments size to the database soon.
+        const updateDataSizePromise = this._updateDataSize(usageOptions);
 
-      try {
-        await removeAttachmentsPromise;
-      } catch (e) {
-        this._log.error(docSession, "Failed to remove expired attachments", e);
-      }
+        try {
+          await removeAttachmentsPromise;
+        } catch (e) {
+          this._log.error(docSession, "Failed to remove expired attachments", e);
+        }
 
-      try {
-        await updateDataSizePromise;
-      } catch (e) {
-        this._log.error(docSession, "Failed to update data size", e);
+        try {
+          await updateDataSizePromise;
+        } catch (e) {
+          this._log.error(docSession, "Failed to update data size", e);
+        }
       }
 
       this._syncDocUsageToDatabase(true);
diff --git a/app/server/lib/DocStorage.ts b/app/server/lib/DocStorage.ts
index 52f97963..b6c06b7b 100644
--- a/app/server/lib/DocStorage.ts
+++ b/app/server/lib/DocStorage.ts
@@ -674,6 +674,10 @@ export class DocStorage implements ISQLiteDB, OnDemandStorage {
     // Note that we don't call _updateMetadata() as there are no metadata tables yet anyway.
   }
 
+  public isInitialized(): boolean {
+    return Boolean(this._db);
+  }
+
   /**
    * Initializes the database with proper settings.
    */