[JAVA-4322] Observable<T> toFuture() implicit returns a future with a null value Created: 30/Sep/21  Updated: 04/May/22

Status: Backlog
Project: Java Driver
Component/s: Scala
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Unknown
Reporter: Daniel Connelly Assignee: Ross Lawley
Resolution: Unresolved Votes: 1
Labels: external-user
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates JAVA-4313 Observable head should not return null Closed
Related
is related to JAVA-4303 Improve map/flatmap of Publisher[Void] Closed

 Description   

Background

While Integrating with the scala mongo driver we discovered https://github.com/mongodb/mongo-java-driver/blob/r4.3.3/driver-scala/src/main/scala/org/mongodb/scala/Observable.scala#L366-L376

the .head method hides a null underneath, as this is not documented nor is it defended against codebases can become vulnerable to null pointer exceptions or invalid responses in an API service, the fallback is to make use of .toFutureOption however as the risk isn't documented teams may only discover this when they hit it. 

If this is undefended against the code above continues to pass, Until someone tries to access something in the desired object. 

Solutions

1. ** Document the behaviour and offer more guidance in handling 

2. Throw NoSuchElementException instead of null.asInstanceOf

3. Create a custom Exception to indicate what the null pointer means (the query returned nothing.)

 



 Comments   
Comment by Ross Lawley [ 20/Oct/21 ]

Opening and marking as a future improvement - when the whole API no longer uses Observable<Void>.

Comment by Daniel Connelly [ 19/Oct/21 ]

Hi Ross, 

 

I don't fully understand the need for null, when you could throw an exception here which could be transformed by other publishers and bubbled up when not transformed and cause errors in the code instead of null. 

Comment by Ross Lawley [ 12/Oct/21 ]

Hi daniel.connelly@hotmail.co.uk,

Since 4.0, the Scala code now lives in the same repo as the Java driver and reactive streams driver, so that is why we now use the JAVA jira project.

Ross

Comment by Daniel Connelly [ 12/Oct/21 ]

I would like to update the ticket as well with the correct link to the null occurrence https://github.com/mongodb/mongo-scala-driver/blob/master/driver/src/main/scala/org/mongodb/scala/ObservableImplicits.scala#L347 

Comment by Daniel Connelly [ 12/Oct/21 ]

Hi Ross, 

Thanks for looking at this, just to confirm. as This is now a JAVA ticket, is the JAVA code automatically transformed into scala code? 

Daniel 

Comment by Ross Lawley [ 12/Oct/21 ]

Hi daniel.connelly@hotmail.co.uk and sunny.chotai@digital.hmrc.gov.uk,

Thanks for your inputs. I think that the issue is the difference in paradigms between reactive stream style observables (Publishers) and Futures. The Observable model has 0-n calls to onNext and then a final state - success or failure. Where as a Future only has success or failure states.

The fact that an Observable completes without calling onNext is not an error state and that is what requires the use of null.

I think this is more a side effect of the Observable[Void] API (JAVA-4303) and if changed to Observable[Unit] would produce more predictable Futures.

Ross

Comment by Sunny Chotai [ 01/Oct/21 ]

When .toFuture does not complete with a value, Future.Failed may be more appropriate than Future.successful(null). Especially given that .toFutureOption gives us Future.successful(None). ie if you use .toFuture over .toFutureOption then the value not being present is unexpected so this may be better represented using an exception in a Future.Failed.

Generated at Thu Feb 08 09:01:47 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.