From 50ca9cc6cf81f8fe80450db615437598250d38ce Mon Sep 17 00:00:00 2001 From: vjrj Date: Thu, 30 Jul 2026 08:29:21 +0200 Subject: [PATCH] fix(subsUnion): stop treating a failed worker call as a valid empty union MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit storeUnion checked `typeof union === 'object'` as its "did this work" guard, but typeof null is also 'object' — so a worker call that failed (caught upstream, leaving union as its initial null) looked identical to a legitimate "zero valid subscriptions" result, and got silently stored as the string "null" instead of being treated as a failure. This is exactly what corrupted subs-public-union/subs-private-union on staging just now, while chasing the missing-turf-deps bug. storeUnion is now only called once calcUnionAsync has actually resolved; process() and incrementalAdd() catch failures themselves and skip storing anything, leaving the previous (still valid) union alone. --- imports/startup/server/subsUnion.js | 94 ++++++++++++++--------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/imports/startup/server/subsUnion.js b/imports/startup/server/subsUnion.js index 5828dc2..641b308 100644 --- a/imports/startup/server/subsUnion.js +++ b/imports/startup/server/subsUnion.js @@ -41,48 +41,50 @@ Meteor.startup(async () => { return valid; }; + // Only called once calcUnionAsync has actually resolved — union === null here + // means "zero valid subscriptions", a real result, not a failed computation. + // (A prior version guarded this with `typeof union === 'object'`, which is + // true for null too, so a failed worker call — caught upstream, leaving + // union as null — looked identical to a legitimate empty union and got + // silently stored as "null". Callers must catch calcUnionAsync failures + // themselves and skip calling this, instead of relying on this check.) const storeUnion = async (isPublic, union, count) => { const publicl = isPublic ? 'public' : 'private'; const Publicl = publicl.replace(/\b\w/g, l => l.toUpperCase()); - - if (typeof union === 'object') { - const bounds = union === null ? null : L.geoJSON(union).getBounds(); - const unionSet = { - $set: { - name: `subs-${publicl}-union`, - value: JSON.stringify(union), - isPublic, - description: `${Publicl} subscriptions union`, - type: 'string' - } - }; - const boundsSet = { - $set: { - name: `subs-${publicl}-union-bounds`, - value: JSON.stringify(bounds), - isPublic, - description: `${Publicl} subscriptions union bounds`, - type: 'string' - } - }; - const sizeSet = { - $set: { - name: 'subs-union-count', - value: count, - isPublic: false, - description: 'Subscriptions count', - type: 'number' - } - }; - // FIXME, take care of object size: - // https://stackoverflow.com/questions/10827812/what-is-the-length-maximum-for-a-string-data-type-in-mongodb-used-with-ruby - await SiteSettings.upsertAsync({ name: `subs-${publicl}-union` }, unionSet, { multi: false }); - await SiteSettings.upsertAsync({ name: `subs-${publicl}-union-bounds` }, boundsSet, { multi: false }); - await SiteSettings.upsertAsync({ name: 'subs-union-count' }, sizeSet, { multi: false }); - if (debug) console.log(`${Publicl} subscription union calculated`); - } else { - console.log('Subscription union failed!'); - } + const bounds = union === null ? null : L.geoJSON(union).getBounds(); + const unionSet = { + $set: { + name: `subs-${publicl}-union`, + value: JSON.stringify(union), + isPublic, + description: `${Publicl} subscriptions union`, + type: 'string' + } + }; + const boundsSet = { + $set: { + name: `subs-${publicl}-union-bounds`, + value: JSON.stringify(bounds), + isPublic, + description: `${Publicl} subscriptions union bounds`, + type: 'string' + } + }; + const sizeSet = { + $set: { + name: 'subs-union-count', + value: count, + isPublic: false, + description: 'Subscriptions count', + type: 'number' + } + }; + // FIXME, take care of object size: + // https://stackoverflow.com/questions/10827812/what-is-the-length-maximum-for-a-string-data-type-in-mongodb-used-with-ruby + await SiteSettings.upsertAsync({ name: `subs-${publicl}-union` }, unionSet, { multi: false }); + await SiteSettings.upsertAsync({ name: `subs-${publicl}-union-bounds` }, boundsSet, { multi: false }); + await SiteSettings.upsertAsync({ name: 'subs-union-count' }, sizeSet, { multi: false }); + if (debug) console.log(`${Publicl} subscription union calculated`); }; const process = async (isPublic) => { @@ -90,13 +92,12 @@ Meteor.startup(async () => { const decorate = isPublic ? addNoisy : noNoisy; const decorated = subscribers.filter(isValidSub).map(decorate); - let union = null; try { - union = await calcUnionAsync(decorated); + const union = await calcUnionAsync(decorated); + await storeUnion(isPublic, union, subscribers.length); } catch (e) { - console.error('subsUnion worker failed', e); + console.error('subsUnion worker failed, union left unchanged', e); } - await storeUnion(isPublic, union, subscribers.length); }; const recreate = async () => { await process(true); await process(false); }; @@ -138,17 +139,16 @@ Meteor.startup(async () => { const current = await SiteSettings.findOneAsync({ name: `subs-${publicl}-union` }); const baseUnion = current && current.value ? JSON.parse(current.value) : null; - let union = null; try { // eslint-disable-next-line no-await-in-loop - union = await calcUnionAsync([decorated], baseUnion); + const union = await calcUnionAsync([decorated], baseUnion); + // eslint-disable-next-line no-await-in-loop + await storeUnion(isPublic, union, countSubs); } catch (e) { console.error('subsUnion incremental worker failed, falling back to a full recreate', e); recomputePending = true; return; } - // eslint-disable-next-line no-await-in-loop - await storeUnion(isPublic, union, countSubs); } if (debug) console.log('Subs union updated incrementally'); };