Skip to content

Conversation

@mbroadst
Copy link
Member

The default cursor iteration behavior is to block until either a cursor is dead (has a cursor id of 0), or a non-empty batch is returned from the server for a getMore command. tryNext allows users to iterate a cursor optionally returning null if the getMore returns an empty batch (likely on a tailable + awaitData cursor).

NODE-2917

@mbroadst mbroadst requested review from emadum and nbbeeken November 25, 2020 14:47
The default cursor iteration behavior is to block until either a
cursor is dead (has a cursor id of 0), or a non-empty batch is
returned from the server for a `getMore` command. `tryNext` allows
users to iterate a cursor optionally returning `null` if the
`getMore` returns an empty batch (likely on a tailable + awaitData
cursor).

NODE-2917

next(cursor, callback);
if (cursor[kDocuments].length === 0 && blocking === false) {
return callback(undefined, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a specific reason to use null over undefined? I suppose it makes more sense to communicate a non-existent object, so I'm not taking issue with it just wondering if there was some thinking behind it

Although maybe one reason to use undefined is so the result doesn't respond true to: typeof result === 'object'.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used null here for symmetry with our semantics around a "dead" cursor. When a cursor returns a zero cursor id, we return an explicit null to the user. I think this might have been left over from days when a Cursor inherited Readable since ending a stream is accomplished by readable.push(null), but either way the convention exists so I stuck with it

@mbroadst mbroadst merged commit 43c94b6 into master Nov 30, 2020
@mbroadst mbroadst deleted the NODE-2917/try-next branch November 30, 2020 14:33
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.

4 participants