[Java] Observe error to avoid crash #22016
Conversation
| closeSubject.doOnError((e) -> {}); | ||
| closeSubject.onError(new RuntimeException(t)); | ||
| onClose.invoke(null, t.getMessage()); | ||
| checkStartFailure(); |
There was a problem hiding this comment.
Is there a chance that checkStartFailure() ever gets called before the Completable is returned from start() meaning we'd need to do the same ting for startSubject?
There was a problem hiding this comment.
I think that is technically possible, but very unlikely in practice.
I'm not sure how the global error handler works in that case, because the startSubject will be observed by someone, they just haven't registered the handler by the time onError is called.
There was a problem hiding this comment.
If it's possible, we should handle it.
I'm not sure how the global error handler works in that case, because the startSubject will be observed by someone, they just haven't registered the handler by the time onError is called.
We could test this by intentionally faulting the startSubject before returning. I expect it would also be crashing. How would the runtime even know the startSubject will be observed by someone in the future?
There was a problem hiding this comment.
Read the docs for CompletableSubject:
If there were no CompletableObservers subscribed to this CompletableSubject when the onError() was called, the global error handler is not invoked.
This also implies that the "fix" in this PR might not be correct, and the error was happening through some other means. The docs also mention that if onError is called multiple times then it will call the global handler. onError should only ever be called once here, but I'm beginning to think it was somehow triggered multiple times.
|
Let's see if it helps :) |
| closeSubject.onComplete(); | ||
| try { | ||
| closeLock.lock(); | ||
| closeSubject.onComplete(); |
There was a problem hiding this comment.
Should we check that the closeSubject hasn't already been completed with an error here?
There was a problem hiding this comment.
Calling onComplete should noop if the subject is already completed either with an error or without an error.
Fixes #20187
Thanks @jaragones for testing out a fix since I could never repro this.