Skip to content

Commit 2c1326b

Browse files
TheLastCicadaTheLastCicada
committed
fix: guard importOrganization against subscribeToStoreOnDataLayer falsy return
Mirrors the pattern established in #1600 af06a65 (for governance sync) and 6d41b71 (for reconcileOrganization). subscribeToStoreOnDataLayer in src/datalayer/persistance.js returns false (without throwing) on the common failure paths: - No storeId provided - getSubscriptions() RPC failure (datalayer unreachable) Without the falsy-return check, the dominant datalayer-unreachable failure would slip past the try/catch, and the subsequent getDataLayerStoreSyncStatus call (which also fails when the store is not subscribed) would produce a misleading "not yet synced" log. Applies to both V1 and V2 importOrganization at two subscribe call sites each: the pre-existing orgUid subscribe at the top of the function, and the new singleton-store subscribe added in this PR. Both are adjacent in the same function; fixing only one would leave an internal inconsistency that a reviewer would flag.
1 parent 73d674b commit 2c1326b

2 files changed

Lines changed: 48 additions & 9 deletions

File tree

‎src/models/organizations/organizations.model.js‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -959,11 +959,23 @@ class Organization extends Model {
959959
// This ensures new org stores get subscribed on the first pass so they can
960960
// begin syncing, and subsequent runs will find them synced and proceed.
961961
if (!USE_SIMULATOR) {
962+
// subscribeToStoreOnDataLayer returns falsy on the common failure paths
963+
// (no storeId, getSubscriptions RPC failure) and also throws on
964+
// unexpected errors, so guard on both. Without the falsy-return check,
965+
// the dominant datalayer-unreachable failure slips past the try/catch
966+
// and the subsequent "not yet synced" skip log is misleading. Same
967+
// pattern as Governance.sync and reconcileOrganization.
962968
try {
963-
await datalayer.subscribeToStoreOnDataLayer(orgUid);
969+
const subscribed = await datalayer.subscribeToStoreOnDataLayer(orgUid);
970+
if (!subscribed) {
971+
logger.warn(
972+
`[v1]: Could not subscribe to org store ${orgUid}. Skipping import, will retry on next task run.`,
973+
);
974+
return;
975+
}
964976
} catch (error) {
965977
logger.warn(
966-
`[v1]: Could not subscribe to store for ${orgUid}, skipping import: ${error.message}`,
978+
`[v1]: Could not subscribe to org store ${orgUid}: ${error.message}. Skipping import, will retry on next task run.`,
967979
);
968980
return;
969981
}
@@ -1013,10 +1025,18 @@ class Organization extends Model {
10131025
}
10141026

10151027
try {
1016-
await datalayer.subscribeToStoreOnDataLayer(singletonStoreId);
1028+
const singletonSubscribed = await datalayer.subscribeToStoreOnDataLayer(
1029+
singletonStoreId,
1030+
);
1031+
if (!singletonSubscribed) {
1032+
logger.warn(
1033+
`[v1]: Could not subscribe to singleton store ${singletonStoreId} for org ${orgUid}. Skipping import, will retry on next task run.`,
1034+
);
1035+
return;
1036+
}
10171037
} catch (error) {
10181038
logger.warn(
1019-
`[v1]: Could not subscribe to singleton store ${singletonStoreId} for org ${orgUid}, skipping import: ${error.message}`,
1039+
`[v1]: Could not subscribe to singleton store ${singletonStoreId} for org ${orgUid}: ${error.message}. Skipping import, will retry on next task run.`,
10201040
);
10211041
return;
10221042
}

‎src/models/v2/organizations-v2.model.js‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1597,12 +1597,23 @@ class OrganizationsV2 extends Model {
15971597
// This ensures new org stores get subscribed on the first pass so they can
15981598
// begin syncing, and subsequent runs will find them synced and proceed.
15991599
if (!USE_SIMULATOR) {
1600+
// subscribeToStoreOnDataLayer returns falsy on the common failure paths
1601+
// (no storeId, getSubscriptions RPC failure) and also throws on
1602+
// unexpected errors, so guard on both. Without the falsy-return check,
1603+
// the dominant datalayer-unreachable failure slips past the try/catch
1604+
// and the subsequent "not yet synced" skip log is misleading. Same
1605+
// pattern as Governance.sync and reconcileOrganization.
16001606
try {
1601-
// Subscribe to the store if not already subscribed (no-op if already subscribed)
1602-
await datalayer.subscribeToStoreOnDataLayer(orgUid);
1607+
const subscribed = await datalayer.subscribeToStoreOnDataLayer(orgUid);
1608+
if (!subscribed) {
1609+
loggerV2.warn(
1610+
`[v2]: Could not subscribe to org store ${orgUid}. Skipping import, will retry on next task run.`,
1611+
);
1612+
return;
1613+
}
16031614
} catch (error) {
16041615
loggerV2.warn(
1605-
`[v2]: Could not subscribe to store for ${orgUid}, skipping import: ${error.message}`,
1616+
`[v2]: Could not subscribe to org store ${orgUid}: ${error.message}. Skipping import, will retry on next task run.`,
16061617
);
16071618
return;
16081619
}
@@ -1653,10 +1664,18 @@ class OrganizationsV2 extends Model {
16531664
}
16541665

16551666
try {
1656-
await datalayer.subscribeToStoreOnDataLayer(singletonStoreId);
1667+
const singletonSubscribed = await datalayer.subscribeToStoreOnDataLayer(
1668+
singletonStoreId,
1669+
);
1670+
if (!singletonSubscribed) {
1671+
loggerV2.warn(
1672+
`[v2]: Could not subscribe to singleton store ${singletonStoreId} for org ${orgUid}. Skipping import, will retry on next task run.`,
1673+
);
1674+
return;
1675+
}
16571676
} catch (error) {
16581677
loggerV2.warn(
1659-
`[v2]: Could not subscribe to singleton store ${singletonStoreId} for org ${orgUid}, skipping import: ${error.message}`,
1678+
`[v2]: Could not subscribe to singleton store ${singletonStoreId} for org ${orgUid}: ${error.message}. Skipping import, will retry on next task run.`,
16601679
);
16611680
return;
16621681
}

0 commit comments

Comments
 (0)