fix: guard reconcileOrganization against subscribeToStoreOnDataLayer falsy return

TheLastCicada · TheLastCicada · commit 6d41b7155b85 · 2026-04-23T13:50:33.000-07:00
Bugbot flagged an inconsistency introduced by af06a65: governance sync now correctly treats both a thrown exception and a falsy return from subscribeToStoreOnDataLayer as a failure, but V1/V2 reconcileOrganization still only guarded thrown exceptions. subscribeToStoreOnDataLayer in src/datalayer/persistance.js returns false (without throwing) on the two most common failure paths: - No storeId provided - getSubscriptions() RPC failure (datalayer unreachable) Without the falsy-return check, the dominant datalayer-unreachable failure slips past the try/catch, and the subsequent getDataLayerStoreSyncStatus call (which also fails when the store is not subscribed) produces a misleading "not yet synced" log instead of a clear "could not subscribe" message. Mirrors the subscribe-first pattern af06a65 added to governance sync. Both subscribe-first and the falsy-return guard are no-ops in simulator mode (wrapped in !USE_SIMULATOR for V2, skipped entirely for V1 via the function's top-level simulator guard).
diff --git a/src/models/organizations/organizations.model.js b/src/models/organizations/organizations.model.js
@@ -834,11 +834,23 @@ class Organization extends Model {
     // before entering the blocking subscription flow.  If either the org store or
     // its derived singleton store is not yet synced, return early so the periodic
     // task retries on the next run rather than waiting up to 10 minutes.
+    //
+    // subscribeToStoreOnDataLayer returns falsy on the common failure paths
+    // (no storeId, getSubscriptions RPC failure) and also throws on unexpected
+    // errors, so guard on both.  Without the falsy-return check, the dominant
+    // datalayer-unreachable failure would slip past the try/catch and the
+    // subsequent "not yet synced" skip log would be misleading.
     try {
-      await datalayer.subscribeToStoreOnDataLayer(orgUid);
+      const subscribed = await datalayer.subscribeToStoreOnDataLayer(orgUid);
+      if (!subscribed) {
+        logger.warn(
+          `[v1]: reconcileOrganization: could not subscribe to org store ${orgUid}. Skipping reconcile, will retry on next task run.`,
+        );
+        return;
+      }
     } catch (error) {
       logger.warn(
-        `[v1]: reconcileOrganization: could not subscribe to org store ${orgUid}, skipping reconcile: ${error.message}`,
+        `[v1]: reconcileOrganization: could not subscribe to org store ${orgUid}: ${error.message}. Skipping reconcile, will retry on next task run.`,
       );
       return;
     }
diff --git a/src/models/v2/organizations-v2.model.js b/src/models/v2/organizations-v2.model.js
@@ -1857,12 +1857,24 @@ class OrganizationsV2 extends Model {
 
     // Subscribe to org store first so it begins syncing, then check sync status
     // before entering the blocking subscription flow.
+    //
+    // subscribeToStoreOnDataLayer returns falsy on the common failure paths
+    // (no storeId, getSubscriptions RPC failure) and also throws on unexpected
+    // errors, so guard on both.  Without the falsy-return check, the dominant
+    // datalayer-unreachable failure would slip past the try/catch and the
+    // subsequent "not yet synced" skip log would be misleading.
     if (!USE_SIMULATOR) {
       try {
-        await datalayer.subscribeToStoreOnDataLayer(org_uid);
+        const subscribed = await datalayer.subscribeToStoreOnDataLayer(org_uid);
+        if (!subscribed) {
+          loggerV2.warn(
+            `[v2]: reconcileOrganization: could not subscribe to org store ${org_uid}. Skipping reconcile, will retry on next task run.`,
+          );
+          return;
+        }
       } catch (error) {
         loggerV2.warn(
-          `[v2]: reconcileOrganization: could not subscribe to org store ${org_uid}, skipping reconcile: ${error.message}`,
+          `[v2]: reconcileOrganization: could not subscribe to org store ${org_uid}: ${error.message}. Skipping reconcile, will retry on next task run.`,
         );
         return;
       }
