exceptionfactory commented on a change in pull request #5130:
URL: https://github.com/apache/nifi/pull/5130#discussion_r646963775
##########
File path:
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/pom.xml
##########
@@ -52,5 +52,17 @@
<version>1.14.0-SNAPSHOT</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-websocket-services-jetty</artifactId>
+ <version>1.14.0-SNAPSHOT</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.nifi</groupId>
+ <artifactId>nifi-websocket-services-jetty</artifactId>
+ <version>1.14.0-SNAPSHOT</version>
+ <scope>test</scope>
+ </dependency>
Review comment:
This dependency declaration appears to be duplicated.
##########
File path:
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-processors/src/main/java/org/apache/nifi/processors/websocket/AbstractWebSocketGatewayProcessor.java
##########
@@ -165,27 +177,36 @@ private void deregister() {
}
@Override
- public final void onTrigger(final ProcessContext context, final
ProcessSessionFactory sessionFactory) throws ProcessException {
+ public final void onTrigger(final ProcessContext context, final
ProcessSessionFactory sessionFactory) {
if (processSessionFactory == null) {
processSessionFactory = sessionFactory;
}
if (!isProcessorRegisteredToService()) {
try {
- registerProcessorToService(context, webSocketService ->
onWebSocketServiceReady(webSocketService));
- } catch (IOException|WebSocketConfigurationException e) {
+ registerProcessorToService(context, webSocketService ->
onWebSocketServiceReady(webSocketService, context));
+ } catch (IOException | WebSocketConfigurationException e) {
// Deregister processor if it failed so that it can retry next
onTrigger.
deregister();
context.yield();
throw new ProcessException("Failed to register processor to
WebSocket service due to: " + e, e);
}
+
+ } else {
+ try {
+ onWebSocketServiceReady(webSocketService, context);
+ } catch (IOException e) {
+ deregister();
+ context.yield();
+ throw new ProcessException("Failed to renew session and
connect to WebSocket service due to: " + e, e);
Review comment:
Should the message include `e.getMessage()` instead of implicitly
calling `e.toString()`?
```suggestion
throw new ProcessException("Failed to renew session and
connect to WebSocket service due to: " + e.getMessage(), e);
```
##########
File path:
nifi-nar-bundles/nifi-websocket-bundle/nifi-websocket-services-jetty/src/main/java/org/apache/nifi/websocket/jetty/JettyWebSocketClient.java
##########
@@ -293,35 +303,37 @@ private void connect(final String clientId, String
sessionId) throws IOException
try {
router = routers.getRouterOrFail(clientId);
} catch (WebSocketConfigurationException e) {
- throw new IllegalStateException("Failed to get router due to:
" + e, e);
+ throw new IllegalStateException("Failed to get router due to:
" + e, e);
Review comment:
Should this message call `e.getMessage()`?
```suggestion
throw new IllegalStateException("Failed to get router due
to: " + e.getMessage(), e);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
