fix(mongoose): await model index builds at startup + repair the never-created billing month index - #3993
Open
PierreBrisorgueil wants to merge 4 commits into
Open
fix(mongoose): await model index builds at startup + repair the never-created billing month index#3993PierreBrisorgueil wants to merge 4 commits into
PierreBrisorgueil wants to merge 4 commits into
Conversation
mongoose.connect() resolving does not mean indexes exist yet: autoIndex builds run in the background and startMongoose() never awaited them. On a brand-new database the first writes could land inside that build window, turning a unique-index idempotency guard (e.g. a duplicate upsert normally caught as E11000) into a no-op that creates a second, distinct document instead. Add mongooseService.awaitIndexBuilds(), calling Model#init() on every registered model, and await it in startMongoose() right after connect() so the app only reports ready once every model's indexes are built. This also surfaces index-creation errors (e.g. an unsupported partialFilterExpression operator) that a fire-and-forget autoIndex was silently swallowing on the model's unlistened 'index' event. Respects the effective autoIndex option: when it resolves falsy, init() still just resolves without forcing a build.
#3990) The (organizationId, month) unique index on legacy (non-meter) usage documents declared partialFilterExpression { weekKey: { $exists: false } } — MongoDB does not support $exists:false (or $ne) inside a partial filter, so the index silently never built on any database. Combined with the previous mongoose.connect()-doesn't-await-indexes gap, this guard has never actually enforced uniqueness anywhere. The legacy month-keyed path (BillingUsageRepository.increment/get/reset) is still live — it backs the meterMode:false (default) usage counters read via BillingUsageService.get() in the billing controller and quota service. Flip the condition to a supported positive check: a new `legacyPeriod` boolean, set only by increment()'s $setOnInsert on newly created legacy documents (never by the meter-mode paths, which key by weekKey), lets the index filter on `{ legacyPeriod: { $exists: true } }` instead. Migration 20260727120000 is the authoritative creator on already-deployed databases: it backfills legacyPeriod onto existing legacy documents, aborts on any pre-existing duplicate (organizationId, month) pair, and installs the corrected index — mirroring the same $exists/$ne pattern already fixed for the membership and email indexes. Adds boot-time index-readiness and migration coverage on a real Mongo: Model#init() surfaces the invalid filter, both unique indexes exist with the exact spec after boot, and meter/legacy replay stays a single document immediately after boot (no index-build window to race).
…MIGRATIONS entry (#3990) - migration: precheck legacy (organizationId, month) duplicates via a plain query FIRST (zero writes), then drop/backfill/recreate the partial index — safe now that boot awaits index builds before migrations.run(), so the index may already be live-and-empty when up() runs - mongoose: awaitIndexBuilds() is now bounded by config.db.awaitIndexBuilds (default 60s timeout) — on timeout, boot continues in a degraded state with a loud warning instead of hanging forever; false disables the wait - MIGRATIONS.md: document the boot-semantics change, the new config knob, and a pre-deploy duplicate-data audit snippet for legacy billing usage
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
WalkthroughBoot now waits for Mongoose index builds with configurable timeout behavior. Billing usage gains a ChangesIndex readiness and billing usage uniqueness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Application
participant MongooseService
participant BillingUsageModels
participant BillingMigration
participant BillingCollection
Application->>MongooseService: connect and await index builds
MongooseService->>BillingUsageModels: initialize model indexes
BillingUsageModels-->>MongooseService: index builds complete
Application->>BillingMigration: run migration
BillingMigration->>BillingCollection: audit duplicates and replace index
BillingMigration->>BillingCollection: backfill legacyPeriod and create unique index
BillingCollection-->>BillingMigration: migration result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3993 +/- ##
==========================================
+ Coverage 93.30% 93.42% +0.11%
==========================================
Files 170 170
Lines 5664 5688 +24
Branches 1821 1826 +5
==========================================
+ Hits 5285 5314 +29
+ Misses 306 304 -2
+ Partials 73 70 -3
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/services/mongoose.js`:
- Line 110: Update the timeoutMs resolution in the mongoose index-build await
logic to coerce numeric string overrides, including Layer-4 environment values,
before validation. Accept only finite positive numbers and otherwise retain
DEFAULT_AWAIT_INDEX_BUILDS_TIMEOUT_MS; preserve existing numeric values and
avoid silently accepting zero, negative, or non-numeric inputs.
In
`@modules/billing/migrations/20260727120000-fix-usage-month-index-partial-filter.js`:
- Around line 122-154: Update the migration flow around existing,
backfillResult, and the final createIndex to detect an already-live index with
the exact target specification and skip dropping/recreating it when the backfill
matches no documents. Otherwise retain the current replacement flow, and wrap
createIndex failures—especially E11000—with the same actionable remediation
guidance used by the pre-check.
- Around line 12-13: Update the JSDoc return annotation for the index predicate
described near the ix parameter from `@return` to `@returns`, preserving its
existing boolean description and behavior.
In
`@modules/billing/tests/billing.usage.monthIndexPartialFilter.migration.integration.tests.js`:
- Around line 188-192: Update the week-index restore logic around weekKeyIndexes
to preserve and replay each complete captured index descriptor rather than
reconstructing only key, name, unique, and sparse; retain the original index
options, including partialFilterExpression and collation, when recreating
indexes so the subsequent syncIndexes test sees the unchanged definitions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f10bb318-bc2d-449e-a3ca-9fe55ef789c9
📒 Files selected for processing (12)
MIGRATIONS.mdconfig/defaults/development.config.jslib/app.jslib/services/mongoose.jslib/services/tests/mongoose.awaitIndexBuilds.unit.tests.jsmodules/billing/migrations/20260727120000-fix-usage-month-index-partial-filter.jsmodules/billing/models/billing.usage.model.mongoose.jsmodules/billing/models/billing.usage.schema.jsmodules/billing/repositories/billing.usage.repository.jsmodules/billing/tests/billing.usage.bootIndexReady.integration.tests.jsmodules/billing/tests/billing.usage.monthIndexPartialFilter.migration.integration.tests.jsmodules/billing/tests/billing.usage.repository.unit.tests.js
…hful test (#3990) CodeRabbit convergence pass on PR #3993: - lib/services/mongoose.js: awaitIndexBuilds() timeoutMs now coerces via Number(...) instead of Number.isFinite on the raw value, so a Layer-4 DEVKIT_NODE_* env override (always a string) is honored; non-positive values fall back to the default instead of racing to a ~0ms timeout. - migration 20260727120000: skip the drop->backfill->recreate window entirely once the index already matches the target spec and nothing is left to backfill (steady-state fast path); the final createIndex now catches a raced E11000 (concurrent old-instance write during a rolling deploy) and converts it into the same actionable abort as the upfront pre-check instead of a bare driver error. Documented the residual window + maintenance-window guidance honestly in the header and MIGRATIONS.md. - test restore of the (organizationId, weekKey) index now replays the full captured descriptor instead of a hand-picked key/name/unique/ sparse subset, so any other option round-trips and can't silently diverge from the schema declaration. - @return -> @returns JSDoc nit. Added regression coverage: numeric-string/invalid timeoutMs coercion (mongoose unit), skip-window fast path + E11000 abort conversion (migration integration).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What — Two boot/index correctness fixes:
startMongoose()now awaits every model's index build (Model.init()) before the app reports ready — bounded by a newdb.awaitIndexBuildsconfig ({timeoutMs: 60000}default; timeout → loud warn naming pending models, boot continues;falsedisables). Unique-index-based idempotency (e.g. BillingUsage weekKey replay protection via caught E11000) previously had a fresh-database window where the first writes landed before the index existed — a duplicate upsert then created a second document instead of no-oping.{organizationId, month}unique index declaredpartialFilterExpression: {weekKey: {$exists: false}}— unsupported by MongoDB, so the index has silently never been created. Replaced with alegacyPerioddiscriminator ($setOnInsert on the legacy path only) +{legacyPeriod: {$exists: true}}filter, Zod mirror updated, and migration20260727120000-*for deployed databases: zero-write duplicate pre-check (aborts loud) → drop-if-present → backfill → recreate. MIGRATIONS.md documents the boot-semantics change + the pre-deploy duplicate audit snippet.Review: Phase-0 gate pass 1 BLOCK (3 findings: migration ordering defeated by boot-built index · unbounded boot wait · missing MIGRATIONS entry) → all fixed in
f50516fa; pass 2 OK-with-nits, residual crash-window filed as 3992. Suites: unit 167/2268, integration 41/516, lint clean.Related: #3991 (compound-sparse weekKey index defect, discovered during this work, separate).
Closes #3990
Summary by CodeRabbit
Bug Fixes
Improvements