[jira] [Commented] (NIFI-4169) PutWebSocket processor with blank WebSocket session id attribute cannot transfer to failure queue

["Koji Kawamura (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/NIFI-4169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16079901#comment-16079901 ] 

Koji Kawamura commented on NIFI-4169:
-------------------------------------

Hi [~ywik] thanks for reporting and sharing your patch. I agree with the idea to improve PutWebSocket broadcasting behavior in failure scenarios. In order to do so, if we emit FlowFiles to 'failure' relationship with the failed session id as an FlowFile attribute, user would be able to construct a better error handling route. Also if we can provide a new processor property to specify whether emit such 'failure' FlowFiles or not would be better, for example 'Broadcast message error handling', options would be 'ignore failed' and 'emit FlowFiles to failure' and defaults to 'ignore failed'. How do you think?

Let me post a few comments on your patch here as well:

{code}
--- a/nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-services-api/src/main/java/org/apache/nifi/websocket/WebSocketMessageRouter.java
+++ b/nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-services-api/src/main/java/org/apache/nifi/websocket/WebSocketMessageRouter.java
@@ -107,14 +107,13 @@ public class WebSocketMessageRouter {
             sendMessage.send(session);
         } else {
             //The sessionID is not specified so broadcast the message to all connected client sessions.
-            sessions.keySet().forEach(itrSessionId -> {
-                try {
-                    final WebSocketSession session = getSessionOrFail(itrSessionId);
-                    sendMessage.send(session);
-                } catch (IOException e) {
-                    logger.warn("Failed to send message to session {} due to {}", itrSessionId, e, e);
-                }
-            });

             // I'm not perfectly sure if this is a good idea for every use-cases to throw an Exception when there's no connected client.
             // For example, If it's just used to periodically share the latest status of something, it wouldn't be important if there's any connected client or not.
+            if (sessions.isEmpty()) {
+                throw new IOException("There are no connected client sessions");
+            }


+            for (Map.Entry<String, WebSocketSession> entry : sessions.entrySet()) {

           // I'd prefer catching the exception and let user to choose how they would like to handle.
+                final WebSocketSession session = getSessionOrFail(entry.getKey());
+                sendMessage.send(session);
+            }
         }
     }
{code}


> PutWebSocket processor with blank WebSocket session id attribute cannot transfer to failure queue
> -------------------------------------------------------------------------------------------------
>
>                 Key: NIFI-4169
>                 URL: https://issues.apache.org/jira/browse/NIFI-4169
>             Project: Apache NiFi
>          Issue Type: Improvement
>          Components: Extensions
>    Affects Versions: 1.3.0, 1.4.0
>            Reporter: Y Wikander
>            Priority: Critical
>              Labels: patch
>             Fix For: 1.3.0, 1.4.0
>
>         Attachments: 0001-websocket-when-sendMessage-fails-under-blank-session.patch
>
>
> If a PutWebSocket processor is setup with a blank WebSocket session id attribute (see NIFI-3318; Send message from PutWebSocket to all connected clients) and it is not connected to a websocket server it will log the failure and mark the flowfile with Success (rather than Failure) -- and the data is effectively lost.
> If there are multiple connected clients, and some succeed and others fail, routing Failure back into the PutWebSocket could result in duplicate data to some clients.
> Other NiFi processors seem to err on the side of "at least once".



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)