0

I'm adapting some sample code from what3words for accessing their API via their Java SDK. It uses RXJava.

The sample code is:

Observable.fromCallable(() -> wrapper.convertTo3wa(new Coordinates(51.2423, -0.12423)).execute())
        .subscribeOn(Schedulers.io())
        .observeOn(AndroidSchedulers.mainThread())
        .subscribe(result -> {
            if (result.isSuccessful()) {
                Log.i("MainActivity", String.format("3 word address: %s", result.getWords()));
            } else {
                Log.e("MainActivity", result.getError().getMessage());
            }
        });

First of all. this gives a deprecation warning when building and a IDE warning (Result of 'Observable.subscribe()' is ignored).

To resolve this first issue I have added Disposable myDisposable = in front of the Observable. Is this correct? (See below for where it is added)

Next I need to add a timeout so that I can show a warning etc if the request times out. To do this I have added .timeout(5000, TimeUnit.MILLISECONDS) to the builder.

This works, but the way timeouts seem to work on Observables is that they throw an exception and I cannot figure out how to catch and handle that exception.

What I have right now is:

Disposable myDisposable = Observable.fromCallable(() -> wrapper.convertTo3wa(new Coordinates(51.2423, -0.12423)).execute())
        .subscribeOn(Schedulers.io())
        .observeOn(AndroidSchedulers.mainThread())
        .timeout(5000, TimeUnit.MILLISECONDS)
        .subscribe(result -> {
            if (result.isSuccessful()) {
                Log.i("MainActivity", String.format("3 word address: %s", result.getWords()));
            } else {
                Log.e("MainActivity", result.getError().getMessage());
            }
        });

This builds and runs fine, and the API/deprecation warning is not shown, BUT when no network is available this correctly times out and throws the unhandled exception.

So, the code seems to be correct, but how on earth do add the exception handling to catch the timeout TimeoutException that is thrown?

I've tried numerous things, including: adding a try-catch clause around the whole Observable - this warns that TimeoutException is not thrown by the code in the `try; and adding an error handler.

Adding the error handler has got me closest, and so the code below is as far as I have got:

Disposable myDisposable = Observable.fromCallable(() -> wrapper.convertTo3wa(new Coordinates(51.2423, -0.12423)).execute())
        .subscribeOn(Schedulers.io())
        .observeOn(AndroidSchedulers.mainThread())
        .timeout(5000, TimeUnit.MILLISECONDS)
        .subscribe(result -> {
            if (result.isSuccessful()) {
                Log.i("MainActivity", String.format("3 word address: %s", result.getWords()));
            } else {
                Log.e("MainActivity", result.getError().getMessage());
            }
         }, error -> {
             runOnUiThread(new Runnable() {
                 @Override
                 public void run() {
                     myTextView.setText(R.string.network_not_available);
                 }
             });
         });

This catches the Timeout correctly and updates my UI without error, however when the network is restored it seems that the Observable might be trying to return and a null pointer exception is thrown.

(Update, this NPE might actually be being thrown sometimes after a short time whether the network is restored or not... but it is always thrown when the network restores.)

I get FATAL EXCEPTION: RxCachedThreadScheduler-1 and java.lang.NullPointerException: Callable returned a null value. Null values are generally not allowed in 3.x operators and sources.

Do I need to destroy the Observable or something to prevent the NPE?

Fat Monk
  • 2,077
  • 1
  • 26
  • 59
  • Use a try-catch to handle the exception – Abhimanyu Aug 19 '21 at 17:15
  • Have tried `try-catch` around the `Observable` and it says that TimeoutException 8is not thrown by the `try`. – Fat Monk Aug 19 '21 at 17:39
  • Please check if this helps, https://blog.mindorks.com/error-handling-in-rxjava – Abhimanyu Aug 19 '21 at 17:42
  • @Abhimanyu It looks like that link should help, but I'm actually just even more confused after reading it... none of the examples seem to fit my issue closely enough for me to adapt them (my failing, I know!) – Fat Monk Aug 20 '21 at 14:31

2 Answers2

4

You need to add an onError handler to your subscribe call:

    .subscribe(result -> {
        if (result.isSuccessful()) {
            Log.i("MainActivity", String.format("3 word address: %s", result.getWords()));
        } else {
            Log.e("MainActivity", result.getError().getMessage());
        }
     },
     error -> {
         // handle error here
     });

When a an exception makes it to a subscribe call that does not have an onError handler, it will throw a OnErrorNotImplementedException, like this:

io.reactivex.exceptions.OnErrorNotImplementedException: The exception was not handled due to missing onError handler in the subscribe() method call. Further reading: https://github.com/ReactiveX/RxJava/wiki/Error-Handling | java.util.concurrent.TimeoutException: The source did not signal an event for 1 seconds and has been terminated.

Adding the onError handler will prevent that, and the onError handler will get called instead.

dano
  • 91,354
  • 19
  • 222
  • 219
  • I tried that as well and it seems to work for throwing the error, but then when the network connection is restored a get a `null' exception (not NPE) from RXjava and can't figure out why - so I thought the `error` handler was incorrect (all I did in the error handle was run a `TextView.setText()` on the UI thread) – Fat Monk Aug 19 '21 at 17:42
  • Adding an onError handler is the correct thing. I'm not sure you mean by null exception, but I would say whatever is happening there is a separate problem. – dano Aug 19 '21 at 17:50
  • I'll replicate tomorrow and post more info once I recreate that "null not expected" message, or whatever it was that caused the crash when I added the `error` handler. By the way, can this also be done with a .doOnError { } clause in the builder rather than as a second clause in the .subscribe? – Fat Monk Aug 19 '21 at 17:54
  • @FatMonk No. If you let the exception reach `subscribe` it has to have an `onError` handler. However, you can use an operator that swallows exceptions (`onErrorResumeNext`, `onErrorReturn`, etc) along with or instead of `doOnError` if you want, which will keep the exception from reaching `subscribe`. – dano Aug 19 '21 at 18:04
  • I've updated the original question with more info about what happens when I add the `error -> {}` handler. – Fat Monk Aug 20 '21 at 10:14
  • @FatMonk `wrapper.convertTo3wa(new Coordinates(51.2423, -0.12423).execute()` must be returning null. It's the only `Callable` in your sample code, and the error is being thrown on an RxJava I/O scheduler thread, which is what you're subscribing on. – dano Aug 20 '21 at 13:57
  • I've added a null check to the `if (!result.isEmpty())` line as `if (result != null && !result.isEmpty)` but am still getting the crash so I assume the NPE is being thrown before we reach the .subscribe... So how can I catch this null and deal with it? – Fat Monk Aug 20 '21 at 14:12
  • You have to do it inside of the `fromCallable` method. If `convertTo3wa` returns null, just return an empty list instead. – dano Aug 20 '21 at 15:08
  • Have just managed to do exactly that combining both your advice and @PPartisan :-) Arrays/Lists in Java are not my strong point, but I think I have something working now without crashes. – Fat Monk Aug 20 '21 at 15:21
0

There's a few things going on here:

First of all. this gives a deprecation warning when building and a IDE warning (Result of 'Observable.subscribe()' is ignored).

subscribe() returns a Disposable. The idea is that when you're no longer interested in receiving the output of your observable, you call dispose() on the disposable and the work terminates. This can also prevent memory leaks.

As an example, imagine you have an Activity, and you start an Observable to run a long network query which finally posts something to the Activity UI. If the user navigates away before this task completes, or the Activity is otherwise destroyed, then you're no longer interested in its output because there is no longer a UI to post to. So you may call dispose() in onStop().

So, the code seems to be correct, but how on earth do add the exception handling to catch the timeout TimeoutException that is thrown?

Using the error block in subscribe is one option, but there are others. For example, if you wanted to keep using your Result class, you could use something like onErrorReturn(throwable -> Result.error(throwable)). Obviously I'm guessing what that class looks like:

.timeout(5000, TimeUnit.MILLISECONDS)
.onErrorReturn(throwable -> Result.errorWithMessage(R.string.network_not_available))
.subscribe(result -> {
  if (result.isSuccessful()) {
    Log.i("MainActivity", String.format("3 word address: %s", result.getWords()));
  } else {
    myTextView.setText(result.getErrorMessage());
  }
});

java.lang.NullPointerException: Callable returned a null value. Null values are generally not allowed in 3.x operators and sources.

This:

wrapper.convertTo3wa(new Coordinates(51.2423, -0.12423)).execute()

is returning null. You can do something like:

Observable.fromCallable(() -> {
  Result<?> out = wrapper.convertTo3wa(new Coordinates(51.2423, -0.12423)).execute();
  if(out == null)
    out = Result.error(/*Returned null*/);
  }
  return out;
}
PPartisan
  • 8,173
  • 4
  • 29
  • 48
  • I guess to dispose of the `Observable` I need toe xtend it's scope so that I can dispose of it in my `MainActivity.onPause()` or other lifecycle event? And re catching that null return from the `Observable`... I'm having trouble with your example - is `Result` a Kotlin class, because I am being asked by the IDE to import it from Kotlin? – Fat Monk Aug 20 '21 at 14:14
  • No, I'm just assuming you have a class called `Result` that you use to communicate success/error, it doesn't exist anywhere in the SDK. You'd need to cache the `Disposable` somewhere so you could dispose of it later in a separate callback, yes. – PPartisan Aug 20 '21 at 14:39
  • If you like I can write an example of how a `Result` class might look, but I assumed you already had something in place as you're calling `result.isSuccessful()` in your `subscribe()`. – PPartisan Aug 20 '21 at 14:49
  • I guess that class must be defined in the what3words SDK that the wrapper class comes from... I'll try digging there... – Fat Monk Aug 20 '21 at 14:52
  • OK, `result` is actually just a `List` where Language is defined in the w3w wrapper class... maybe I can work it out now... – Fat Monk Aug 20 '21 at 14:55