[#1143] Do not report a Notice of Disconnection that cannot reach an already-closed client as an unhandled error

[vharseko]

Problem

The server can drop a connection with a Notice of Disconnection after the client has already closed its end. For example, AuthenticatedUsers.doPostResponse does this after a DELETE of the bound user. The notice then cannot be written, and the failure is reported as an uncaught RxJava error. The server prints an OnErrorNotImplementedException ... | java.io.EOFException stack trace. According to the code, on a worker thread it also logs ERR_UNCAUGHT_THREAD_EXCEPTION and raises an ALERT_TYPE_UNCAUGHT_EXCEPTION alert. This produces 7 to 11 traces per run in the build-docker-alpine benchmark log. See #1143.

Cause

LDAPServerFilter.ClientConnectionImpl.disconnect(ResultCode, String) subscribed to the notification with a bare .subscribe(). In RxJava 3.1.10, EmptyCompletableObserver.onError does not throw. It hands the error to RxJavaPlugins.onError, which prints it and passes it to the thread's uncaught-exception handler. So the try/catch (OnErrorNotImplementedException) around s.onError() in sendUnsolicitedNotification() (added in #555) never saw anything.

Change

• disconnect(ResultCode, String) subscribes with an error consumer that only traces the failure: a client that is already gone is an expected outcome. doAfterTerminate(connection.closeSilently()) is unchanged, so the connection is closed whether or not the notice was written.
• The dead try/catch and its import are removed.

Test

ConnectionFactoryTestCase.testDisconnectWithNotificationToClosedClientIsNotReportedAsUnhandledError closes the client and waits until the server context reports isClosed(). It then calls disconnect(BUSY, "busy") with a capturing RxJavaPlugins error handler installed, and asserts that nothing reached it. The write fails before disconnect() returns, so the assertion needs no wait.

• Without the fix (LDAPServerFilter from master): fails with expecting empty, but was:<[OnErrorNotImplementedException ... | java.io.EOFException]>.
• With the fix: passes. The whole opendj-grizzly suite (1040 tests) is green.

Fixes #1143

[maximthomas]

praise: The fix is on the subscribe that lost the error, and the test proves it.

• LDAPServerFilter.ClientConnectionImpl.disconnect(ResultCode, String) now subscribes with an error consumer that only traces the failure (opendj-grizzly/src/main/java/org/forgerock/opendj/grizzly/LDAPServerFilter.java:545-557). doAfterTerminate(connection.closeSilently()) still closes the connection whether or not the notice was written.
• testDisconnectWithNotificationToClosedClientIsNotReportedAsUnhandledError fails with the base LDAPServerFilter (OnErrorNotImplementedException ... | java.io.EOFException) and passes at the head (ConnectionFactoryTestCase 78/78). It restores the previous RxJavaPlugins error handler in finally.
• Under RxJava 3 the removed try/catch (OnErrorNotImplementedException) around s.onError could never catch anything, as the description explains. disconnect is the only production subscriber of sendUnsolicitedNotification, and it now handles the error, so removing the catch is safe

[maximthomas]

praise: The fix is on the subscribe that lost the error, and the test proves it.

• LDAPServerFilter.ClientConnectionImpl.disconnect(ResultCode, String) now subscribes with an error consumer that only traces the failure (opendj-grizzly/src/main/java/org/forgerock/opendj/grizzly/LDAPServerFilter.java:545-557). doAfterTerminate(connection.closeSilently()) still closes the connection whether or not the notice was written.
• testDisconnectWithNotificationToClosedClientIsNotReportedAsUnhandledError fails with the base LDAPServerFilter (OnErrorNotImplementedException ... | java.io.EOFException) and passes at the head (ConnectionFactoryTestCase 78/78). It restores the previous RxJavaPlugins error handler in finally.
• Under RxJava 3 the removed try/catch (OnErrorNotImplementedException) around s.onError could never catch anything, as the description explains. disconnect is the only production subscriber of sendUnsolicitedNotification, and it now handles the error, so removing the catch is safe