From d8a047b4c36e400f71a5cc02e1313c56c4c7b70f Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Wed, 29 Jul 2026 16:01:25 +0200 Subject: [PATCH 1/2] fix(mongoose): tolerate same-name index conflicts at boot so migrations can repair them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A schema index change that alters the options of an existing same-name index (e.g. adding a partialFilterExpression) made the app unable to boot against an already-deployed database: awaitIndexBuilds() propagated the MongoDB conflict rejection (code 85 IndexOptionsConflict / 86 IndexKeySpecsConflict) before the migration runner — which runs right after it in bootstrap and owns the drop/recreate repair — could execute. The repair was gated behind the very defect it repairs. This deliberately narrows the #3990 fail-fast contract: conflict codes 85/86 describe environment state (a legacy index on a deployed database), not a code bug. They are now logged at error level — model, driver error, and best-effort declared (schema) vs live (collection) specs — and boot continues serving on the stale live index so the migration can reconcile. Every other rejection (e.g. an invalid index declaration) still propagates and fails boot exactly as before. Closes #4004 --- lib/services/mongoose.js | 75 +++++++++++- .../mongoose.awaitIndexBuilds.unit.tests.js | 115 ++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) diff --git a/lib/services/mongoose.js b/lib/services/mongoose.js index ca0ec871e..6486e8839 100644 --- a/lib/services/mongoose.js +++ b/lib/services/mongoose.js @@ -26,6 +26,59 @@ const resolveDebug = (cfg = config) => Boolean(cfg?.db?.debug) && configHelper.i */ const DEFAULT_AWAIT_INDEX_BUILDS_TIMEOUT_MS = 60000; +/** + * MongoDB server error codes for "an index with this name already exists with + * a different definition" (#4004): + * - 85 `IndexOptionsConflict` — same name, different options (e.g. a schema + * declaration gaining a `partialFilterExpression` that the deployed + * database's index does not carry yet) + * - 86 `IndexKeySpecsConflict` — same name, different key spec + * Both describe a state of the ENVIRONMENT (a legacy index on an + * already-deployed database), not a defect in the code — the repair for them + * is a drop/recreate migration, which runs AFTER {@link awaitIndexBuilds} in + * the boot sequence (lib/app.js#bootstrap). + */ +const INDEX_CONFLICT_CODES = new Set([85, 86]); +const INDEX_CONFLICT_CODE_NAMES = new Set(['IndexOptionsConflict', 'IndexKeySpecsConflict']); + +/** + * @desc Whether an index-build rejection is a same-name conflict with a + * pre-existing index (see {@link INDEX_CONFLICT_CODES}). + * @param {Error} err - rejection from `Model#init()` + * @returns {boolean} true when the error is an index-name conflict + */ +const isIndexConflictError = (err) => INDEX_CONFLICT_CODES.has(err?.code) || INDEX_CONFLICT_CODE_NAMES.has(err?.codeName); + +/** + * @desc Log a tolerated same-name index conflict LOUDLY: the model, the driver + * error (whose message names the requested and existing index), and — best + * effort — the full declared (schema) vs live (collection) index specs, so an + * operator can see exactly which definition the database still holds. Never + * throws: spec enumeration failures are logged and swallowed, since this runs + * on a boot path that must proceed to the migration runner (#4004). + * @param {string} modelName - name of the model whose index build conflicted + * @param {Error} err - the conflict rejection from `Model#init()` + * @returns {Promise} resolves once the conflict has been logged + */ +const logIndexConflict = async (modelName, err) => { + logger.error( + chalk.red( + `Index build CONFLICT on model '${modelName}' (${err?.codeName || err?.code}): ${err?.message} — ` + + 'boot CONTINUES with the LIVE (stale) index so the migration runner can reconcile it; ' + + 'the schema-declared spec is NOT applied until a migration drops/recreates this index.', + ), + ); + try { + const model = mongoose.model(modelName); + const declared = model.schema.indexes(); + const live = await model.collection.listIndexes().toArray(); + logger.error(chalk.red(` declared (schema): ${JSON.stringify(declared)}`)); + logger.error(chalk.red(` live (collection): ${JSON.stringify(live)}`)); + } catch (specErr) { + logger.error(chalk.red(` could not enumerate declared/live index specs: ${specErr?.message}`)); + } +}; + /** * Load all mongoose related models */ @@ -98,7 +151,18 @@ const connect = async () => { * already proceeded past this call. * A genuine rejection that happens BEFORE the timeout (the common case — an * invalid index declaration fails fast) still propagates and rejects this - * call, exactly as before. + * call, with ONE deliberate exception (#4004): a same-name index CONFLICT + * (codes 85/86 — the live collection already holds an index under the + * declared name with a different definition) is logged at error level and + * TOLERATED. That state is environment state, not a code bug: the migration + * that reconciles it runs AFTER this call (lib/app.js#bootstrap), so + * rejecting here would gate the repair behind the very defect it repairs — + * the app would crash-loop with no path forward short of manual index + * surgery. Serving with the stale live index (writes keep obeying the OLD + * constraint until the migration lands) beats not serving at all. NOTE: a + * conflict aborts that model's remaining index builds too (createIndexes is + * sequential per model), which the log calls out — the migration reconciling + * the conflict re-converges the rest via autoIndex on the next boot. * @param {object} [cfg=config] - application configuration object * @returns {Promise} Resolves once every registered model's index builds * finish, the configured timeout elapses, or immediately when disabled. @@ -127,6 +191,15 @@ const awaitIndexBuilds = async (cfg = config) => { .then((result) => { pending.delete(name); return result; + }) + .catch(async (err) => { + pending.delete(name); + // Same-name index conflict with a pre-existing (legacy) index: log + // loudly and keep booting so the migration runner — which runs right + // after this in bootstrap — can reconcile it (#4004). Every other + // rejection (e.g. an invalid index declaration) still propagates. + if (!isIndexConflictError(err)) throw err; + await logIndexConflict(name, err); }), ), ); diff --git a/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js b/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js index a005c03cd..f489e75bb 100644 --- a/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js +++ b/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js @@ -232,4 +232,119 @@ describe('mongoose service — awaitIndexBuilds:', () => { expect(localMongoose.modelNames).not.toHaveBeenCalled(); expect(localMongoose.model).not.toHaveBeenCalled(); }); + + /** + * #4004 — a schema-declared index whose options differ from a PRE-EXISTING + * same-name index (MongoDB codes 85 IndexOptionsConflict / 86 + * IndexKeySpecsConflict) must NOT fail startup: the migration that + * reconciles the conflict runs AFTER awaitIndexBuilds() in bootstrap, so + * rejecting here gates the repair behind the very defect it repairs. The + * conflict must instead be logged at error level, naming the model and both + * the declared (schema) and live (collection) specs. Every other rejection + * keeps propagating (covered above). These paths need their own logger mock + * (to capture error calls), so they set up an isolated module registry like + * the timeout tests. + */ + const setupWithConflictingModel = async ({ initError, listIndexes }) => { + jest.resetModules(); + jest.unstable_mockModule('../../../config/index.js', () => ({ + default: { db: { uri: 'mongodb://127.0.0.1:27017/NodeTest', options: {} }, files: { mongooseModels: [] } }, + })); + const error = jest.fn(); + jest.unstable_mockModule('../logger.js', () => ({ + default: { info: jest.fn(), error, warn: jest.fn() }, + })); + + const localMongoose = { + modelNames: jest.fn(() => ['User', 'BillingUsage']), + model: jest.fn((name) => ({ + init: name === 'BillingUsage' ? jest.fn().mockRejectedValue(initError) : jest.fn().mockResolvedValue(undefined), + schema: { + indexes: jest.fn(() => [ + [{ organizationId: 1, month: 1 }, { unique: true, partialFilterExpression: { legacyPeriod: { $exists: true } } }], + ]), + }, + collection: { listIndexes: jest.fn(() => ({ toArray: listIndexes })) }, + })), + connect: jest.fn(), + set: jest.fn(), + }; + jest.unstable_mockModule('mongoose', () => ({ default: localMongoose })); + + const mod = await import('../mongoose.js'); + return { awaitIndexBuilds: mod.default.awaitIndexBuilds, error }; + }; + + test('same-name index conflict (code 85, IndexOptionsConflict) does NOT reject — boot continues, error names model + both specs', async () => { + const conflict = Object.assign(new Error('An existing index has the same name as the requested index.'), { + code: 85, + codeName: 'IndexOptionsConflict', + }); + const { awaitIndexBuilds: run, error } = await setupWithConflictingModel({ + initError: conflict, + listIndexes: jest.fn().mockResolvedValue([{ v: 2, key: { organizationId: 1, month: 1 }, name: 'organizationId_1_month_1', unique: true }]), + }); + + await expect(run()).resolves.toBeUndefined(); + + const logged = error.mock.calls.map((c) => String(c[0])).join('\n'); + expect(logged).toEqual(expect.stringContaining('BillingUsage')); + expect(logged).toEqual(expect.stringContaining('IndexOptionsConflict')); + expect(logged).toEqual(expect.stringContaining('declared (schema)')); + expect(logged).toEqual(expect.stringContaining('partialFilterExpression')); + expect(logged).toEqual(expect.stringContaining('live (collection)')); + expect(logged).toEqual(expect.stringContaining('organizationId_1_month_1')); + }); + + test('same-name key-spec conflict identified by codeName only (IndexKeySpecsConflict, no numeric code) is tolerated too', async () => { + const conflict = Object.assign(new Error('Index must have unique name.'), { codeName: 'IndexKeySpecsConflict' }); + const { awaitIndexBuilds: run, error } = await setupWithConflictingModel({ + initError: conflict, + listIndexes: jest.fn().mockResolvedValue([]), + }); + + await expect(run()).resolves.toBeUndefined(); + expect(error).toHaveBeenCalled(); + }); + + test('conflict tolerance survives a failing spec enumeration (listIndexes throws) — still resolves, still logs', async () => { + const conflict = Object.assign(new Error('An existing index has the same name as the requested index.'), { code: 85 }); + const { awaitIndexBuilds: run, error } = await setupWithConflictingModel({ + initError: conflict, + listIndexes: jest.fn().mockRejectedValue(new Error('not authorized on listIndexes')), + }); + + await expect(run()).resolves.toBeUndefined(); + const logged = error.mock.calls.map((c) => String(c[0])).join('\n'); + expect(logged).toEqual(expect.stringContaining('could not enumerate declared/live index specs')); + }); + + test('tolerance is scoped to conflicts: a non-conflict rejection on another model still propagates', async () => { + jest.resetModules(); + jest.unstable_mockModule('../../../config/index.js', () => ({ + default: { db: { uri: 'mongodb://127.0.0.1:27017/NodeTest', options: {} }, files: { mongooseModels: [] } }, + })); + jest.unstable_mockModule('../logger.js', () => ({ + default: { info: jest.fn(), error: jest.fn(), warn: jest.fn() }, + })); + + const conflict = Object.assign(new Error('An existing index has the same name as the requested index.'), { code: 85 }); + const localMongoose = { + modelNames: jest.fn(() => ['User', 'BillingUsage']), + model: jest.fn((name) => ({ + init: + name === 'BillingUsage' + ? jest.fn().mockRejectedValue(conflict) + : jest.fn().mockRejectedValue(new Error('unsupported partial filter operator')), + schema: { indexes: jest.fn(() => []) }, + collection: { listIndexes: jest.fn(() => ({ toArray: jest.fn().mockResolvedValue([]) })) }, + })), + connect: jest.fn(), + set: jest.fn(), + }; + jest.unstable_mockModule('mongoose', () => ({ default: localMongoose })); + + const mod = await import('../mongoose.js'); + await expect(mod.default.awaitIndexBuilds()).rejects.toThrow('unsupported partial filter operator'); + }); }); From b545bb249ea1b7accca5ee5ef1b595218c05ce74 Mon Sep 17 00:00:00 2001 From: Pierre Brisorgueil Date: Wed, 29 Jul 2026 16:28:15 +0200 Subject: [PATCH 2/2] fix(mongoose): pass raw error + preserve declared spec on live-fetch failure Addresses CodeRabbit review on PR #4005: - logIndexConflict now passes the raw driver error as logger.error's second arg (matches this file's own convention at the timeout log a few lines below, and the codebase-wide two-arg pattern) - log the declared (schema) spec immediately after computing it, before awaiting listIndexes(), so a live-fetch failure doesn't discard an already-available declared spec - JSDoc for the new setupWithConflictingModel test helper --- lib/services/mongoose.js | 3 ++- lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/services/mongoose.js b/lib/services/mongoose.js index 6486e8839..27869339c 100644 --- a/lib/services/mongoose.js +++ b/lib/services/mongoose.js @@ -67,12 +67,13 @@ const logIndexConflict = async (modelName, err) => { 'boot CONTINUES with the LIVE (stale) index so the migration runner can reconcile it; ' + 'the schema-declared spec is NOT applied until a migration drops/recreates this index.', ), + err, ); try { const model = mongoose.model(modelName); const declared = model.schema.indexes(); - const live = await model.collection.listIndexes().toArray(); logger.error(chalk.red(` declared (schema): ${JSON.stringify(declared)}`)); + const live = await model.collection.listIndexes().toArray(); logger.error(chalk.red(` live (collection): ${JSON.stringify(live)}`)); } catch (specErr) { logger.error(chalk.red(` could not enumerate declared/live index specs: ${specErr?.message}`)); diff --git a/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js b/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js index f489e75bb..1c1dd7dcd 100644 --- a/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js +++ b/lib/services/tests/mongoose.awaitIndexBuilds.unit.tests.js @@ -244,6 +244,11 @@ describe('mongoose service — awaitIndexBuilds:', () => { * keeps propagating (covered above). These paths need their own logger mock * (to capture error calls), so they set up an isolated module registry like * the timeout tests. + * @param {object} opts - scenario configuration + * @param {Error} opts.initError - rejection reason for the `BillingUsage` model's `init()` + * @param {Function} opts.listIndexes - mock `toArray()` implementation for `collection.listIndexes()` + * @returns {Promise<{awaitIndexBuilds: Function, error: import('@jest/globals').Mock}>} the isolated + * `awaitIndexBuilds` under test and the mocked `logger.error` */ const setupWithConflictingModel = async ({ initError, listIndexes }) => { jest.resetModules();