AsyncResultSingle shouldn't emit null

[Mikey-Burns]

One of the requirements in RxJava 2 is that you cannot emit null as a value anymore. However, methods such as io.vertx.reactivex.servicediscovery.ServiceDiscovery#rxGetRecord(Function) emit null if they don't find a matching record, because AsyncResultSingle directly returns the resulting value from the AsyncResult. This leads to weird behavior where the following test would fail

@Test(timeout = TIMEOUT)
public void nullExampleTest(TestContext context) {
  final Async async = context.async();
  ServiceDiscovery.create(Vertx.vertx(), new ServiceDiscoveryOptions()
      .setBackendConfiguration(new JsonObject()
          .put("backend-name", DefaultServiceDiscoveryBackend.class.getName())
      )
  )
      .rxGetRecord(record -> false)
      .doOnSuccess(System.out::println)
//        .map(record -> record)
      .subscribe(
          emit -> context.fail("Should have thrown an NPE"),
          error -> {
            context.assertEquals(NullPointerException.class, error.getClass());
            async.complete();
          }
      );
}

because the Single will emit null. However, if you uncomment the .map(record -> record), it will throw an NPE because you cannot map a null item in RxJava 2.

I believe the solution is to modify AsyncResultSingle from this

try {
  observer.onSuccess(ar.result());
} catch (Exception err) {
  log.error("Unexpected error", err);
}

to check for null

try {
  Objects.requireNonNull(ar.result(), "The mapper function returned a null value.");
  observer.onSuccess(ar.result());
} catch (Exception err) {
  log.error("Unexpected error", err);
}

I can put up a PR for this if you agree that it's a valid fix.

[tsegismont]

Thanks for the report. The Rxified API is generated from the standard API. I guess the problem is that getRecord result should be annotated with Nullable. Then Vert.x Codegen would generate a Maybe return instead of Single. Can you confirm @cescoffier ?

[Mikey-Burns]

@tsegismont Returning Maybe instead of Single would be even better. I was assuming that switching from Single to Maybe was unlikely, so this would be an improvement over the current state.

[vietj]

https://github.com/vert-x3/vertx-service-discovery/issues/86

[gaol]

I think the issue exists in vertx-mongo-client as well, like:

MongoClient findOne(String collection, JsonObject query, @Nullable JsonObject fields, Handler<AsyncResult> resultHandler);

The generated rx version is: Single<JsonObject> rxFindOne(String collection, JsonObject query, JsonObject fields), return as Maybe<JsonObject> should be more appropriate because of same reason as above. :)

[tsegismont]

Can you file an issue on this project?

[gaol]

Sure, done: https://github.com/vert-x3/vertx-mongo-client/issues/142

[tsegismont]

Thank you