AUT-5433: Wait for both session stores before calling back

The dual session store was previously calling back to `express-session` before the secondary store operation completed. This meant the response could be sent and the browser could fire the next request before the secondary write finished, creating a window where the stores had different data.
It also caused false positives in the consistency check, as the secondary read (during a `get()` call) would return data that had already been overwritten by the current request's `set()` call.

Moving all callbacks to after both store operations complete should reduce the chance of stale session data between the stores. This may cause a slight increase in latency during the transition period until DynamoDB becomes the primary and exclusive store. It would be good to try to keep the transition period reasonably short, but having enough confidence around the consistency of data across the two stores.

Author: sw-mattw

src/config/dual-session-store.ts

@@ -36,7 +36,6 @@ export class DualSessionStore extends Store {
           { sid, store: this.primaryLabel },
           "Session read from primary"
         );
-        cb(null, primarySession);
       }
 
       this.secondary.get(sid, (secondaryErr, secondarySession) => {
@@ -59,51 +58,92 @@ export class DualSessionStore extends Store {
             { err: secondaryErr, sid, store: this.secondaryLabel },
             "Secondary consistency check read failed"
           );
-          return;
+        } else {
+          logger.info(
+            { sid, store: this.secondaryLabel },
+            "Session read from secondary"
+          );
+          this.performConsistencyChecks(primarySession, secondarySession, sid);
         }
 
-        logger.info(
-          { sid, store: this.secondaryLabel },
-          "Session read from secondary"
-        );
-        this.performConsistencyChecks(primarySession, secondarySession, sid);
+        cb(null, primarySession);
       });
     });
   }
 
   set(sid: string, sess: SessionData, cb: (err?: any) => void): void {
-    this.primary.set(sid, sess, (err) => {
-      logger.info(
-        { sid, store: this.primaryLabel },
-        "Session written to primary"
-      );
-      cb(err);
-    });
+    this.primary.set(sid, sess, (primaryErr) => {
+      if (primaryErr) {
+        logger.warn(
+          { err: primaryErr, sid, store: this.primaryLabel },
+          "Primary session write failed"
+        );
+      } else {
+        logger.info(
+          { sid, store: this.primaryLabel },
+          "Session written to primary"
+        );
+      }
 
-    this.writeToSecondary("set", sid, sess);
-  }
+      this.secondary.set(sid, sess, (secondaryErr) => {
+        if (secondaryErr) {
+          logger.warn(
+            { err: secondaryErr, sid, store: this.secondaryLabel },
+            "Secondary session write failed"
+          );
+        } else {
+          logger.info(
+            { sid, store: this.secondaryLabel },
+            "Session written to secondary"
+          );
+        }
 
-  destroy(sid: string, cb: (err?: any) => void): void {
-    this.primary.destroy(sid, (err) => {
-      logger.info(
-        { sid, store: this.primaryLabel },
-        "Session destroyed in primary"
-      );
-      cb(err);
+        if (primaryErr && secondaryErr) {
+          cb(new DualStoreError(primaryErr, secondaryErr));
+        } else if (primaryErr) {
+          cb(primaryErr);
+        } else {
+          cb();
+        }
+      });
     });
+  }
 
-    this.secondary.destroy(sid, (secondaryErr) => {
-      if (secondaryErr) {
+  destroy(sid: string, cb: (err?: any) => void): void {
+    this.primary.destroy(sid, (primaryErr) => {
+      if (primaryErr) {
         logger.warn(
-          { err: secondaryErr, sid, store: this.secondaryLabel },
-          "Secondary session destroy failed"
+          { err: primaryErr, sid, store: this.primaryLabel },
+          "Primary session destroy failed"
+        );
+      } else {
+        logger.info(
+          { sid, store: this.primaryLabel },
+          "Session destroyed in primary"
         );
-        return;
       }
-      logger.info(
-        { sid, store: this.secondaryLabel },
-        "Session destroyed in secondary"
-      );
+
+      this.secondary.destroy(sid, (secondaryErr) => {
+        if (secondaryErr) {
+          logger.warn(
+            { err: secondaryErr, sid, store: this.secondaryLabel },
+            "Secondary session destroy failed"
+          );
+        } else {
+          logger.info(
+            { sid, store: this.secondaryLabel },
+            "Session destroyed in secondary"
+          );
+        }
+
+        if (primaryErr && secondaryErr) {
+          cb(new DualStoreError(primaryErr, secondaryErr));
+        } else if (primaryErr) {
+          cb(primaryErr);
+        } else {
+          cb();
+        }
+      });
     });
   }
 
@@ -113,36 +153,30 @@ export class DualSessionStore extends Store {
         { sid, store: this.primaryLabel },
         "Session touched in primary"
       );
-      cb();
-    });
 
-    this.writeToSecondary("touch", sid, sess);
+      this.touchSecondary(sid, sess, cb);
+    });
   }
 
-  private writeToSecondary(
-    operation: "set" | "touch",
-    sid: string,
-    sess: SessionData
-  ): void {
+  private touchSecondary(sid: string, sess: SessionData, cb: () => void): void {
     try {
-      this.secondary[operation](sid, sess, (secondaryErr) => {
-        if (secondaryErr) {
-          logger.warn(
-            { err: secondaryErr, sid, operation, store: this.secondaryLabel },
-            "Secondary session write failed"
+      if (this.secondary.touch) {
+        this.secondary.touch(sid, sess, () => {
+          logger.info(
+            { sid, store: this.secondaryLabel },
+            "Session touched in secondary"
           );
-          return;
-        }
-        logger.info(
-          { sid, operation, store: this.secondaryLabel },
-          "Session written to secondary"
-        );
-      });
+          cb();
+        });
+      } else {
+        cb();
+      }
     } catch (err) {
       logger.warn(
-        { err, sid, operation, store: this.secondaryLabel },
-        "Secondary session write threw unexpectedly"
+        { err, sid, store: this.secondaryLabel },
+        "Secondary session touch threw unexpectedly"
       );
+      cb();
     }
   }