Skip to content

fix: preserve optional logger for search support probe#1097

Closed
Will-hxw wants to merge 1 commit intomongodb-js:ni/search-testsfrom
Will-hxw:fix/search-probe-logger-optional
Closed

fix: preserve optional logger for search support probe#1097
Will-hxw wants to merge 1 commit intomongodb-js:ni/search-testsfrom
Will-hxw:fix/search-probe-logger-optional

Conversation

@Will-hxw
Copy link
Copy Markdown

@Will-hxw Will-hxw commented Apr 24, 2026

Background

PR #1095 adds debug logging to the Atlas Search capability probe. That changes ConnectionStateConnected.isSearchSupported() from a no-argument public method to requiring a LoggerBase, which can break direct consumers even though the normal Session path passes a logger.

What Changed

  • Made the logger parameter optional for ConnectionStateConnected.isSearchSupported().
  • Fall back to the existing NullLogger when the method is called directly without a logger.
  • Updated the public API reports to show the optional parameter.
  • Added a unit test covering direct no-argument calls.

Not Changed

Risk

Low. Existing internal calls still pass the session logger. Direct calls without a logger keep working and simply use no-op logging during the first probe.

Validation

  • pnpm exec vitest --project unit-and-integration tests/unit/common/session.test.ts --run
  • pnpm run check:types
  • pnpm run check:api
  • pnpm run check:format
  • pnpm run check:lint
  • pnpm run check:dependencies
  • pnpm run check

Linked Issues

Supports #1095 / #1022.

@Will-hxw Will-hxw requested a review from a team as a code owner April 24, 2026 08:48
@Will-hxw Will-hxw requested review from blva and removed request for a team April 24, 2026 08:48
@nirinchev
Copy link
Copy Markdown
Collaborator

hey, thanks for that - I was debating with myself whether we should do it as an optional or required - I went for the latter because I figured this is only called internally. Do you have a use case for calling isSearchSupported()?

@blva blva requested review from nirinchev and removed request for blva April 24, 2026 18:12
@Will-hxw Will-hxw closed this by deleting the head repository Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants