fix(NODE-5374): do not apply cursor transform in Cursor.hasNext

[baileympearson]

Description

What is changing?

Primarily this PR fixes two bugs in our AbstractCursor.

• transform should not be applied when using hasNext.
• when transform throws, we should catch the error and propagate it to the user.

To facilitate this, the generic next<T> was refactored to use async-await as well.

The PR is broken down into commits which might be easier to view. The first commits are the async-await refactor, then the bug fixes.

Is there new documentation needed for these changes?

No.

What is the motivation for this change?

Release Highlight

Cursor.map transform bug fixes

The cursor API provides the ability to apply a map function to each document in the cursor:

const cursor = collection.find({ name: 'john doe' }).map(({ name }) => name); for await (const document of cursor) { console.error(document); // only prints the `name` field from each document }

Cursor iteration properly catches errors thrown from the transform applied in Cursor.map

Starting in version 4.0 of the driver, if the transform function throws an error, there are certain scenarios where the driver does not correctly catch this error and an uncaught exception is thrown:

const cursor = collection.find({ name: 'john doe' }).map(() => { throw new Error('oh no! error here'); //  }); await cursor.next(); // process crashes with uncaught error

This release adds logic to ensure that whenever we transform a cursor document, we handle any errors properly. Any errors thrown from a transform function are caught and returned to the user.

Cursor.hasNext no longer transforms documents if a transform has been applied to the cursor

Version 4.0 introduced a bug that would apply a transform function to documents in the cursor when the cursor was iterated using Cursor.hasNext(). When combined with Cursor.next(), this would result in transforming documents multiple times.

const cursor = collection.find({ name: 'john doe' }).map((document) => document.name); while (await cursor.hasNext()) { // this transforms the first document in the cursor once const doc = await cursor.next(); // the second document in the cursor is transformed again }

This release removes the transform logic from Cursor.hasNext, preventing cursor documents from being transformed twice when iterated using hasNext.

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

[durran]

I assume we are defaulting to true here as more methods would transform vs. methods that don't (hasNext)?

[baileympearson]

Yeah, that was my logic. I also considered two other options:

• providing no default
• combining blocking and transform into an options object and requiring all callsites to explicitly set them for clarity (i.e., next(this, { blocking: true, transform: false })

[nbbeeken]

I lean towards providing no default, for internal I prefer to make each call site define its inputs and because this isn't saving us a lot of refactors. I don't feel strongly though, feel free to resolve if we agree to leave it as is.