Remove GRIST_SKIP_REDIS_CHECKSUM_MISMATCH

Skipping the redis checksum mismatch is now generalized. A warning is logged when we see a mismatch.
gristlabs · Jul 10, 2024 · c3bd9dd · c3bd9dd
1 parent d336293
commit c3bd9dd
Show file tree

Hide file tree

Showing 2 changed files with 4 additions and 20 deletions.
diff --git a/app/server/lib/ExternalStorage.ts b/app/server/lib/ExternalStorage.ts
@@ -1,5 +1,4 @@
 import {ObjMetadata, ObjSnapshot, ObjSnapshotWithMetadata} from 'app/common/DocSnapshot';
-import {isAffirmative} from 'app/common/gutil';
 import log from 'app/server/lib/log';
 import {createTmpDir} from 'app/server/lib/uploads';
 
@@ -239,16 +238,7 @@ export class ChecksummedExternalStorage implements ExternalStorage {
             const message = `ext ${this.label} download: data for ${fromKey} has wrong checksum:` +
               ` ${checksum} (expected ${expectedChecksum})`;
 
-            // If GRIST_SKIP_REDIS_CHECKSUM_MISMATCH is set, issue a warning only and continue,
-            // rather than issuing an error and failing.
-            // This flag is experimental and should be removed once we are
-            // confident that the checksums verification is useless.
-            if (isAffirmative(process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH)) {
-              log.warn(message);
-            } else {
-              log.error(message);
-              return undefined;
-            }
+            log.warn(message);
           }
         }
 

diff --git a/test/server/lib/HostedStorageManager.ts b/test/server/lib/HostedStorageManager.ts
@@ -494,18 +494,12 @@ describe('HostedStorageManager', function() {
         await setRedisChecksum(docId, 'nobble');
         await store.removeAll();
 
-        // With GRIST_SKIP_REDIS_CHECKSUM_MISMATCH set, the fetch should work
-        process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH = 'true';
+        const warnSpy = sandbox.spy(log, 'warn');
         await store.run(async () => {
           await assert.isFulfilled(store.docManager.fetchDoc(docSession, docId));
+          assert.isTrue(warnSpy.calledWithMatch('has wrong checksum'), 'a warning should have been logged');
         });
-
-        // By default, the fetch should eventually errors.
-        delete process.env.GRIST_SKIP_REDIS_CHECKSUM_MISMATCH;
-        await store.run(async () => {
-          await assert.isRejected(store.docManager.fetchDoc(docSession, docId),
-                                  /operation failed to become consistent/);
-        });
+        warnSpy.restore();
 
         // Check we get the document back on fresh start if checksum is correct.
         await setRedisChecksum(docId, checksum);