Conversation
- close(): serialize concurrent calls through a shared `_closing` promise so the second call awaits the actual native teardown instead of returning ahead of it. - open(): after `await this._connecting`, throw a clean "client closed during open" if `close()` nulled `db` mid-flight, replacing the noisy TypeError surfaced through outer catch. - emit(): wrap the default `console.error` call in try/catch so a hostile or broken console (test mocks, EPIPE, APM wrappers) cannot break the fail-silent contract. Tests: client.js gains race + concurrent-close cases; observability.js gains a survey that exercises every public method against a throwing console.error.
- each(): make `[Symbol.asyncIterator]()` open its own native cursor
via a per-iteration local; track all open cursors in a Set on the
outer closure so `Symbol.asyncDispose` still closes everything in
scope. Two parallel `for await` loops on the same `each(...)` value
now each see the full result set instead of one trampling the
other into a `Cannot use a session that has ended` emit.
- count({}): short-circuit to `estimatedDocumentCount`, ~20x faster
on large collections. JSDoc notes the estimated path may drift on
sharded collections with orphans or after unclean shutdown.
- save(): replace path now uses `?? 0` for upsertedCount /
modifiedCount / matchedCount to defend against a partial driver
response producing NaN > 0 = false.
Tests: each.js gains parallel + sequential reuse cases; index.js
pins the estimatedDocumentCount short-circuit with a mock survey.
Adds Streaming reads, Indexes, Positional updates, AbortSignal
sections; Symbol.asyncDispose example next to close(); refreshes
the Collection methods table with each / createIndex /
ensureIndexes / signal columns.
Notes the count({}) → estimatedDocumentCount short-circuit and
the empty-filter-vs-aborted-signal corner case explicitly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1de0a9f3b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The simple `if (this._closing) return this._closing` early-return dropped legitimate close requests that arrive after a reopen. Sequence: close() #1 starts (resets state, awaits native teardown) → another operation triggers open(), which installs a fresh native client on `this.client` → close() #2 hits the early-return, awaits the first close, and exits without closing the newly opened client. Fix: each close() now creates its own task that (a) awaits the previous in-flight close, (b) snapshots `this.client` and bails if another close already nulled it, (c) otherwise resets state and calls native close. Concurrent closes without reopen still share one native close (the second snapshot sees null and bails). Closes that race a reopen each close their own client. Adds a regression test that primes a connection, starts close() #1, forces a reopen via collection.count() (which sync-installs a new native client), then close() #2 — both client A and client B must close exactly once.
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
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.
Summary
Pre-release fixes for v6.1.0 based on a multi-angle audit of the v6.0.0 → main delta.
close()andemit()reliability: serialize concurrentclose()through_closingso the second call awaits the actual native teardown; throw a cleanclient closed during openinstead of an internalTypeErrorwhenopen()racesclose(); wrap defaultconsole.errorintry/catchso a hostile or broken console (test mocks, EPIPE, APM wrappers) cannot break the fail-silent contract.each()shared-cursor closure fixed via a factory model:[Symbol.asyncIterator]()opens its own native cursor, whileSymbol.asyncDisposestill closes everything in scope through a Set tracked on the outer closure. Two parallelfor awaitloops on the sameeach(...)value now each see the full result set instead of one trampling the other into aCannot use a session that has endedemit.count({})speedup: short-circuits toestimatedDocumentCount, ~20× faster on large collections. Notes the estimated path may drift on sharded collections with orphans / after unclean shutdown.save()defensive guard: replace path uses?? 0forupsertedCount/modifiedCount/matchedCountto defend against a partial driver response producingNaN > 0 = false.Test plan