TaskWrapper: Configurable callback if exception thrown!

[stefanofago73]

Listeners must be non-blocking and safe!

Before version 1.0.8, if something like this is passed as a listener:

(item, value)->{ throw new RuntimeException("Boom!"); }

There's no other solution than creating a new session, because the callbacks (CompletableFuture) weren't completing.

In version 1.0.8, callback invocations were introduced, but the "stuck-effect" is still present.
(Lines 276-278 and 310-313)

Perhaps it would be more effective to let the Client to decide what to do in the case of an unhandled exception.
This is also for debugging purposes or to develop a retry policy before deciding to actually block processing due to an unrecoverable situation.

This could be achieved with changes to the Task element and the handling of the TaskWrapper. Logically imply some "restrictions and guardrails"

Could it work?

[kevinherron]

Can you give me a little more detail about the issue you're describing? What listeners are you talking about? What gets stuck?

[stefanofago73]

Sorry... I'm talking about DataValueListener, initially tested on a Milo Client 1.0.3... If a DataValueListener throws an exception, and more values are ready, after dispatching the notifications, no more notifications arrive, but everything is alive... The lines I've reported are related to TaskQueue for the tag 1.0.8. If it is still confusing (sorry), I'll be back with a complete report. Thanks for the incredible Milo!

[kevinherron]

I think this particular issue is better solved by doing something like this:

Subject: [PATCH] Use `whenCompleteAsync` instead of `thenRunAsync` in PublishingManager

- Replaced `thenRunAsync` with `whenCompleteAsync` to handle exceptions during notification delivery.
- Added logging to capture unexpected exceptions during notification processing.
---
Index: opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/PublishingManager.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/PublishingManager.java b/opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/PublishingManager.java
--- a/opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/PublishingManager.java	(revision 56056644bcd3610964b1c9807c83f9b61059b771)
+++ b/opc-ua-sdk/sdk-client/src/main/java/org/eclipse/milo/opcua/sdk/client/subscriptions/PublishingManager.java	(date 1763487132708)
@@ -307,8 +307,13 @@
       // PublishRequest. Waiting until the client has finished receiving notifications
       // is the backpressure mechanism that prevents the server from flooding the client
       // with data change notifications faster than it can process them.
-      callback.thenRunAsync(
-          () -> {
+      callback.whenCompleteAsync(
+          (unit, ex) -> {
+            if (ex != null) {
+              logger.warn(
+                  "Notification delivery threw an unexpected Exception: {}", ex.getMessage(), ex);
+            }
+
             pendingCount.getAndUpdate(p -> (p > 0) ? p - 1 : 0);
 
             maybeSendPublishRequests();

[kevinherron]

Merged: #1661

[stefanofago73]

Thx for All!