Skip to content

Commit 9cf6cb7

Browse files
TheLastCicadaTheLastCicada
committed
fix: add skipOnUnsynced option to reconcileOrganization to prevent data loss
Bugbot flagged a High Severity regression on 6d41b71: reconcileOrganization now silently returns (no throw) when a store is unsynced or the subscribe call fails, but the user-facing resyncOrganization controller (POST /organizations/resync and POST /v2/organizations/resync) calls reconcileOrganization and then unconditionally: 1. Resets registryHash / registry_hash to '0' 2. Destroys ModelKeys rows (V1) / AuditV2 rows (V2) for the orgUid Before this PR, reconcileOrganization would either complete or throw; the throw would hit the controller's catch, roll back the transaction, and return 400 without destroying data. After 6d41b71, a transient datalayer hiccup (subscribe returns false, sync-status RPC fails, etc.) causes the resync endpoint to "succeed" with silent data deletion. Fix: add a { skipOnUnsynced } option to both V1 and V2 reconcileOrganization. - Default (false): throw on any subscribe-failed / store-unsynced / status-check-failed condition. Restores pre-6e76235 behavior for request-path callers. - true: silent skip with an INFO log. Preserves the sync-blocking regression fix for background task callers (validate-organization-table-and-subscriptions*.js). Background task callers updated to pass { skipOnUnsynced: true }. The two resync controllers are unchanged — they continue to call reconcileOrganization with the (now throwing) default behavior, which hits their existing catch block and rolls back the transaction. Adds six contract tests covering the new option in simulator mode (signature acceptance, no-option back-compat, no signature errors for either value). Production throw/skip behavior remains covered by live integration tests; simulator mode cannot reach the pre-check block (wrapped in !USE_SIMULATOR for V2, guarded at the top of the function for V1).
1 parent 6d41b71 commit 9cf6cb7

5 files changed

Lines changed: 220 additions & 57 deletions

File tree

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

Lines changed: 45 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ class Organization extends Model {
789789
* @returns {Promise<void>}
790790
* @throws Error on failure. call in a try block
791791
*/
792-
static async reconcileOrganization(organization) {
792+
static async reconcileOrganization(organization, { skipOnUnsynced = false } = {}) {
793793
if (USE_SIMULATOR) {
794794
return;
795795
}
@@ -830,6 +830,19 @@ class Organization extends Model {
830830
}
831831
}
832832

833+
// Skip-or-throw helper: for transient "not ready" conditions, background
834+
// tasks want a silent skip (skipOnUnsynced=true), request-paths want a hard
835+
// failure (skipOnUnsynced=false, the default) so their downstream destructive
836+
// writes (e.g. POST /organizations/resync resetting registryHash and
837+
// destroying all data-model rows) don't run against stale state.
838+
const skipOrThrow = (message) => {
839+
if (skipOnUnsynced) {
840+
logger.info(`[v1]: reconcileOrganization: ${message}. Will retry on next task run.`);
841+
return;
842+
}
843+
throw new Error(`reconcileOrganization: ${message}`);
844+
};
845+
833846
// Subscribe to org store first so it begins syncing, then check sync status
834847
// before entering the blocking subscription flow. If either the org store or
835848
// its derived singleton store is not yet synced, return early so the periodic
@@ -840,51 +853,48 @@ class Organization extends Model {
840853
// errors, so guard on both. Without the falsy-return check, the dominant
841854
// datalayer-unreachable failure would slip past the try/catch and the
842855
// subsequent "not yet synced" skip log would be misleading.
856+
let subscribeErr;
843857
try {
844858
const subscribed = await datalayer.subscribeToStoreOnDataLayer(orgUid);
845859
if (!subscribed) {
846-
logger.warn(
847-
`[v1]: reconcileOrganization: could not subscribe to org store ${orgUid}. Skipping reconcile, will retry on next task run.`,
848-
);
849-
return;
860+
subscribeErr = `could not subscribe to org store ${orgUid}`;
850861
}
851862
} catch (error) {
852-
logger.warn(
853-
`[v1]: reconcileOrganization: could not subscribe to org store ${orgUid}: ${error.message}. Skipping reconcile, will retry on next task run.`,
854-
);
863+
subscribeErr = `could not subscribe to org store ${orgUid}: ${error.message}`;
864+
}
865+
if (subscribeErr) {
866+
skipOrThrow(subscribeErr);
855867
return;
856868
}
857869

870+
let orgStatusErr;
858871
try {
859872
const orgSyncStatus = await getDataLayerStoreSyncStatus(orgUid);
860873
if (!isDlStoreSynced(orgSyncStatus?.sync_status)) {
861-
logger.info(
862-
`[v1]: reconcileOrganization: org store ${orgUid} not yet synced, skipping reconcile. Will retry on next task run.`,
863-
);
864-
return;
874+
orgStatusErr = `org store ${orgUid} not yet synced`;
865875
}
866876
} catch (error) {
867-
logger.warn(
868-
`[v1]: reconcileOrganization: could not check sync status for org store ${orgUid}, skipping reconcile: ${error.message}`,
869-
);
877+
orgStatusErr = `could not check sync status for org store ${orgUid}: ${error.message}`;
878+
}
879+
if (orgStatusErr) {
880+
skipOrThrow(orgStatusErr);
870881
return;
871882
}
872883

873884
// Check the singleton (data model version) store before the blocking fetch inside
874885
// subscribeToOrganization so that an unsynced singleton doesn't stall the background task.
875886
if (dataModelVersionStoreId) {
887+
let singletonStatusErr;
876888
try {
877889
const singletonSyncStatus = await getDataLayerStoreSyncStatus(dataModelVersionStoreId);
878890
if (!isDlStoreSynced(singletonSyncStatus?.sync_status)) {
879-
logger.info(
880-
`[v1]: reconcileOrganization: singleton store ${dataModelVersionStoreId} for org ${orgUid} not yet synced, skipping reconcile. Will retry on next task run.`,
881-
);
882-
return;
891+
singletonStatusErr = `singleton store ${dataModelVersionStoreId} for org ${orgUid} not yet synced`;
883892
}
884893
} catch (error) {
885-
logger.warn(
886-
`[v1]: reconcileOrganization: could not check sync status for singleton store ${dataModelVersionStoreId}, skipping reconcile: ${error.message}`,
887-
);
894+
singletonStatusErr = `could not check sync status for singleton store ${dataModelVersionStoreId}: ${error.message}`;
895+
}
896+
if (singletonStatusErr) {
897+
skipOrThrow(singletonStatusErr);
888898
return;
889899
}
890900
}
@@ -1014,16 +1024,20 @@ class Organization extends Model {
10141024
* @returns {Promise<void>}
10151025
*/
10161026
static async importOrganization(orgUid, isHome = false) {
1017-
// Subscribe to the org store first, then check sync status.
1018-
// This ensures new org stores get subscribed on the first pass so they can
1019-
// begin syncing, and subsequent runs will find them synced and proceed.
1027+
// Subscribe-first, then check sync status. This ensures new org stores
1028+
// get subscribed on the first pass so they can begin syncing, and
1029+
// subsequent runs will find them synced and proceed.
10201030
if (!USE_SIMULATOR) {
1021-
try {
1022-
await datalayer.subscribeToStoreOnDataLayer(orgUid);
1023-
} catch (error) {
1024-
logger.warn(
1025-
`[v1]: Could not subscribe to store for ${orgUid}, skipping import: ${error.message}`,
1026-
);
1031+
if (
1032+
!(await subscribeOrSkip({
1033+
datalayer,
1034+
storeId: orgUid,
1035+
logger,
1036+
prefix: '[v1]: importOrganization',
1037+
label: 'org',
1038+
action: 'import',
1039+
}))
1040+
) {
10271041
return;
10281042
}
10291043

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

Lines changed: 44 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1835,11 +1835,24 @@ class OrganizationsV2 extends Model {
18351835
}
18361836

18371837
/**
1838-
* Reconcile organization - validate and update database with datalayer data
1838+
* Reconcile organization - validate and update database with datalayer data.
1839+
*
18391840
* @param {Object} organization - Organization record
1841+
* @param {Object} [options]
1842+
* @param {boolean} [options.skipOnUnsynced=false] - When true, silently
1843+
* return early if the org or singleton store is unsynced or if a subscribe
1844+
* call fails. Background task callers (validate-organization-table*) must
1845+
* pass `true` so a transient datalayer hiccup doesn't block task cadence.
1846+
* Request-path callers (e.g. POST /organizations/resync) MUST leave this
1847+
* `false`: their downstream code (e.g. resetting registry_hash,
1848+
* destroying audit records) depends on reconcile either completing or
1849+
* throwing, never silently skipping — otherwise a datalayer hiccup causes
1850+
* data deletion without reconciliation.
18401851
* @returns {Promise<void>}
1852+
* @throws When `skipOnUnsynced` is false and any of: the org store subscribe
1853+
* fails, the org or singleton store is unsynced, or a sync-status RPC fails.
18411854
*/
1842-
static async reconcileOrganization(organization) {
1855+
static async reconcileOrganization(organization, { skipOnUnsynced = false } = {}) {
18431856
const { org_uid, is_home, data_model_version_store_id } = organization;
18441857

18451858
loggerV2.info(`[v2]: Reconciling organization ${org_uid}`);
@@ -1855,6 +1868,17 @@ class OrganizationsV2 extends Model {
18551868
}
18561869
}
18571870

1871+
// Skip-or-throw helper: for transient "not ready" conditions, background
1872+
// tasks want a silent skip, request-paths want a hard failure so their
1873+
// downstream destructive writes don't run.
1874+
const skipOrThrow = (message) => {
1875+
if (skipOnUnsynced) {
1876+
loggerV2.info(`[v2]: reconcileOrganization: ${message}. Will retry on next task run.`);
1877+
return;
1878+
}
1879+
throw new Error(`reconcileOrganization: ${message}`);
1880+
};
1881+
18581882
// Subscribe to org store first so it begins syncing, then check sync status
18591883
// before entering the blocking subscription flow.
18601884
//
@@ -1864,49 +1888,46 @@ class OrganizationsV2 extends Model {
18641888
// datalayer-unreachable failure would slip past the try/catch and the
18651889
// subsequent "not yet synced" skip log would be misleading.
18661890
if (!USE_SIMULATOR) {
1891+
let subscribeErr;
18671892
try {
18681893
const subscribed = await datalayer.subscribeToStoreOnDataLayer(org_uid);
18691894
if (!subscribed) {
1870-
loggerV2.warn(
1871-
`[v2]: reconcileOrganization: could not subscribe to org store ${org_uid}. Skipping reconcile, will retry on next task run.`,
1872-
);
1873-
return;
1895+
subscribeErr = `could not subscribe to org store ${org_uid}`;
18741896
}
18751897
} catch (error) {
1876-
loggerV2.warn(
1877-
`[v2]: reconcileOrganization: could not subscribe to org store ${org_uid}: ${error.message}. Skipping reconcile, will retry on next task run.`,
1878-
);
1898+
subscribeErr = `could not subscribe to org store ${org_uid}: ${error.message}`;
1899+
}
1900+
if (subscribeErr) {
1901+
skipOrThrow(subscribeErr);
18791902
return;
18801903
}
18811904

1905+
let orgStatusErr;
18821906
try {
18831907
const orgSyncStatus = await datalayer.getDataLayerStoreSyncStatus(org_uid);
18841908
if (!isDlStoreSynced(orgSyncStatus?.sync_status)) {
1885-
loggerV2.info(
1886-
`[v2]: reconcileOrganization: org store ${org_uid} not yet synced, skipping reconcile. Will retry on next task run.`,
1887-
);
1888-
return;
1909+
orgStatusErr = `org store ${org_uid} not yet synced`;
18891910
}
18901911
} catch (error) {
1891-
loggerV2.warn(
1892-
`[v2]: reconcileOrganization: could not check sync status for org store ${org_uid}, skipping reconcile: ${error.message}`,
1893-
);
1912+
orgStatusErr = `could not check sync status for org store ${org_uid}: ${error.message}`;
1913+
}
1914+
if (orgStatusErr) {
1915+
skipOrThrow(orgStatusErr);
18941916
return;
18951917
}
18961918

18971919
if (data_model_version_store_id) {
1920+
let singletonStatusErr;
18981921
try {
18991922
const singletonSyncStatus = await datalayer.getDataLayerStoreSyncStatus(data_model_version_store_id);
19001923
if (!isDlStoreSynced(singletonSyncStatus?.sync_status)) {
1901-
loggerV2.info(
1902-
`[v2]: reconcileOrganization: singleton store ${data_model_version_store_id} for org ${org_uid} not yet synced, skipping reconcile. Will retry on next task run.`,
1903-
);
1904-
return;
1924+
singletonStatusErr = `singleton store ${data_model_version_store_id} for org ${org_uid} not yet synced`;
19051925
}
19061926
} catch (error) {
1907-
loggerV2.warn(
1908-
`[v2]: reconcileOrganization: could not check sync status for singleton store ${data_model_version_store_id}, skipping reconcile: ${error.message}`,
1909-
);
1927+
singletonStatusErr = `could not check sync status for singleton store ${data_model_version_store_id}: ${error.message}`;
1928+
}
1929+
if (singletonStatusErr) {
1930+
skipOrThrow(singletonStatusErr);
19101931
return;
19111932
}
19121933
}

‎src/tasks/validate-organization-table-and-subscriptions-v2.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ const task = new Task('validate-organization-table-v2', async () => {
3737
);
3838

3939
try {
40-
await OrganizationsV2.reconcileOrganization(organization);
40+
// Background task — skip (not throw) when the org/singleton store
41+
// is unsynced or a subscribe call fails, so a transient datalayer
42+
// hiccup doesn't spam ERROR logs or block task cadence.
43+
await OrganizationsV2.reconcileOrganization(organization, {
44+
skipOnUnsynced: true,
45+
});
4146
} catch (error) {
4247
loggerV2.error(
4348
`failed reconcile organization records and subscriptions for organization ${organization.org_uid}. Error: ${error.message}. `,

‎src/tasks/validate-organization-table-and-subscriptions.js‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,12 @@ const task = new Task('validate-organization-table', async () => {
3737
);
3838

3939
try {
40-
await Organization.reconcileOrganization(organization);
40+
// Background task — skip (not throw) when the org/singleton store
41+
// is unsynced or a subscribe call fails, so a transient datalayer
42+
// hiccup doesn't spam ERROR logs or block task cadence.
43+
await Organization.reconcileOrganization(organization, {
44+
skipOnUnsynced: true,
45+
});
4146
} catch (error) {
4247
logger.error(
4348
`failed reconcile organization records and subscriptions for organization ${organization.orgUid}. Error: ${error.message}. `,

0 commit comments

Comments
 (0)