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

Ensure BatchCursorFlux does not drop an error #1075

Merged
merged 8 commits into from
Feb 3, 2023
Merged

Conversation

rozza
Copy link
Member

@rozza rozza commented Feb 1, 2023

The BatchCursor class can be closed by the BatchCursorFlux class but there is no signalling for the BatchCursor onNext publisher.

This leads to a next() called after the cursor was closed error and that breaks the reactive streams spec as the error ends up being dropped.

JAVA-4849

The BatchCursor class can be closed by the BatchCursorFlux class
but there is no signalling for the BatchCursor onNext publisher.

This leads to a next() called after the cursor was closed error and
that breaks the reactive streams spec as the error ends up being dropped.

JAVA-4849
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

Does the test reproduce the error reported in JAVA-4849? It looks rather different, and I'm having trouble understanding exactly how this fix relates to the original report.

@rozza
Copy link
Member Author

rozza commented Feb 1, 2023

To add some more context - the "error" reported is a logging message similar to:

14:34:25.594 [Thread-17] ERROR reactor.core.publisher.Operators - Operator called default onErrorDropped
reactor.core.Exceptions$ErrorCallbackNotImplemented: com.mongodb.MongoException: next() called after the cursor was closed.
Caused by: com.mongodb.MongoException: next() called after the cursor was closed.

So the program works as expected it just logs an error as the default onErrorDropped is called.

The test ensures that after the fix no onErrorDropped hook is called.

https://projectreactor.io/docs/core/release/reference/#hooks-dropping

@rozza
Copy link
Member Author

rozza commented Feb 1, 2023

Updated to check the parent sink to see if its been cancelled. This should be a better flag to use here.

@rozza rozza requested a review from jyemin February 1, 2023 14:52
@rozza rozza requested a review from jyemin February 2, 2023 10:38
@jyemin jyemin self-requested a review February 2, 2023 14:24
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

LGTM

@rozza rozza merged commit 617432e into mongodb:master Feb 3, 2023
@rozza rozza deleted the JAVA-4849 branch February 3, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants