Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-6845): ensure internal rejections are handled #4448

Merged
merged 17 commits into from
Mar 13, 2025

Conversation

nbbeeken
Copy link
Contributor

@nbbeeken nbbeeken commented Mar 4, 2025

Description

What is changing?

  • we convert unhandled rejections to uncaught exceptions
Is there new documentation needed for these changes?

No

What is the motivation for this change?

Recent issues with KMS and unhandled rejections inspired better defaults for our test suite

Fix misc unhandled rejections under special conditions

We identified an issue with our test suite that suppressed catching unhandled rejections and surfacing them to us so we can ensure the driver handles any possible rejections. Luckily only 3 cases were identified and each was under a flagged or specialized code path that may not have been in use:

  • If the MongoClient was configured to use OIDC and an AbortSignal was aborted on cursor at the same time the client was reauthenticating, if the reauth process was rejected it would have been unhandled.
  • If timeoutMS was used and the timeout expired before an operation reached the server selection step the operation would throw the expected timeout error but a promise representing the timeout would also raise an unhandled rejection.
  • If a change stream was closed while processing a change event it was possible for the "change stream is closed" error to be emitted as an error event and reject an internal promise representing fetching the "next" change.

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@nbbeeken nbbeeken requested a review from a team as a code owner March 4, 2025 22:13
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch 2 times, most recently from ade61c3 to 0a0bd03 Compare March 4, 2025 23:17
@nbbeeken nbbeeken changed the title test(NODE-6804): implement more invalid KMS tests and set strict unhandled rejection flag test(NODE-6804): convert unhandled rejections into uncaught exceptions Mar 4, 2025
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch 2 times, most recently from 79328e3 to 63aa846 Compare March 4, 2025 23:21
@nbbeeken nbbeeken marked this pull request as draft March 5, 2025 18:40
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch 2 times, most recently from fd54823 to 3887239 Compare March 7, 2025 17:18
@nbbeeken nbbeeken marked this pull request as ready for review March 7, 2025 18:17
@nbbeeken nbbeeken changed the title test(NODE-6804): convert unhandled rejections into uncaught exceptions fix(NODE-6804): ensure internal rejections are handled Mar 7, 2025
@baileympearson baileympearson self-assigned this Mar 10, 2025
@baileympearson baileympearson added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Mar 10, 2025
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch from aa03bf2 to bffe34a Compare March 11, 2025 18:48
@nbbeeken nbbeeken changed the title fix(NODE-6804): ensure internal rejections are handled fix(NODE-6845): ensure internal rejections are handled Mar 11, 2025
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch from 61e27c3 to bffe34a Compare March 11, 2025 22:39
@nbbeeken nbbeeken changed the base branch from main to NODE-6844-defer-fixes March 11, 2025 22:44
@nbbeeken nbbeeken requested a review from baileympearson March 11, 2025 22:44
@nbbeeken nbbeeken force-pushed the NODE-6844-defer-fixes branch from 929562a to 93a24b2 Compare March 12, 2025 15:50
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch from bffe34a to b1e5f0c Compare March 12, 2025 15:51
Base automatically changed from NODE-6844-defer-fixes to main March 12, 2025 19:32
@nbbeeken nbbeeken force-pushed the NODE-6804-test-kms branch from b1e5f0c to 489ebf6 Compare March 12, 2025 19:35
@baileympearson baileympearson merged commit 06e941a into main Mar 13, 2025
30 checks passed
@baileympearson baileympearson deleted the NODE-6804-test-kms branch March 13, 2025 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Primary Review In Review with primary reviewer, not yet ready for team's eyes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants