From 546d815251fa6ae3bbd51fe2a6f2901a38eb60a9 Mon Sep 17 00:00:00 2001 From: Julien Giovaresco Date: Tue, 26 Mar 2024 09:35:45 +0100 Subject: [PATCH 1/4] feat: improve handshake error handling When HelloReply's status is ERROR, throw an exception containing error detail that can be handled by the caller. --- .../websocket/channel/WebSocketConnectorChannel.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/channel/WebSocketConnectorChannel.java b/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/channel/WebSocketConnectorChannel.java index cd9e110..8d5d024 100644 --- a/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/channel/WebSocketConnectorChannel.java +++ b/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/channel/WebSocketConnectorChannel.java @@ -16,7 +16,6 @@ package io.gravitee.exchange.connector.websocket.channel; import io.gravitee.exchange.api.channel.exception.ChannelException; -import io.gravitee.exchange.api.channel.exception.ChannelInitializationException; import io.gravitee.exchange.api.command.Command; import io.gravitee.exchange.api.command.CommandAdapter; import io.gravitee.exchange.api.command.CommandHandler; @@ -75,7 +74,10 @@ public Completable initialize() { this.targetId = reply.getPayload().getTargetId(); this.active = true; } else { - throw new ChannelInitializationException("Unable to parse hello reply payload"); + throw new WebSocketConnectorException( + String.format("Hello handshake failed: %s", reply.getErrorDetails()), + false + ); } }); }) From 9c108bbd0db7ade9aaddc0d7767937319c286432 Mon Sep 17 00:00:00 2001 From: Julien Giovaresco Date: Tue, 26 Mar 2024 10:40:01 +0100 Subject: [PATCH 2/4] fix: check targetId before unregistering the connector When the handshake has failed, the targetId is not defined, and the removal was throwing a NullPointerException. --- .../connector/core/DefaultExchangeConnectorManager.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gravitee-exchange-connector/gravitee-exchange-connector-core/src/main/java/io/gravitee/exchange/connector/core/DefaultExchangeConnectorManager.java b/gravitee-exchange-connector/gravitee-exchange-connector-core/src/main/java/io/gravitee/exchange/connector/core/DefaultExchangeConnectorManager.java index 25b1aad..77ba3ff 100644 --- a/gravitee-exchange-connector/gravitee-exchange-connector-core/src/main/java/io/gravitee/exchange/connector/core/DefaultExchangeConnectorManager.java +++ b/gravitee-exchange-connector/gravitee-exchange-connector-core/src/main/java/io/gravitee/exchange/connector/core/DefaultExchangeConnectorManager.java @@ -68,7 +68,9 @@ public Completable register(final ExchangeConnector exchangeConnector) { public Completable unregister(final ExchangeConnector exchangeConnector) { return Completable .defer(() -> { - exchangeConnectors.remove(exchangeConnector.targetId(), exchangeConnector); + if (exchangeConnector.targetId() != null) { + exchangeConnectors.remove(exchangeConnector.targetId(), exchangeConnector); + } return exchangeConnector.close(); }) .doOnComplete(() -> log.debug("Connector successfully unregister for target [{}]", exchangeConnector.targetId())) From d95bde7ca8209b7d3317961d8cb6b6af561888fd Mon Sep 17 00:00:00 2001 From: Julien Giovaresco Date: Tue, 26 Mar 2024 10:44:09 +0100 Subject: [PATCH 3/4] fix: do not log the error in the log informing retries have stop This would prevent polluting the log. The caller would be able to create its own log with the cause if wanted. --- .../connector/websocket/WebSocketExchangeConnector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/WebSocketExchangeConnector.java b/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/WebSocketExchangeConnector.java index 657cad3..18392f0 100644 --- a/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/WebSocketExchangeConnector.java +++ b/gravitee-exchange-connector/gravitee-exchange-connector-websocket/src/main/java/io/gravitee/exchange/connector/websocket/WebSocketExchangeConnector.java @@ -92,7 +92,7 @@ public Completable initialize() { if (err instanceof WebSocketConnectorException connectorException && connectorException.isRetryable()) { return Flowable.timer(5000, TimeUnit.MILLISECONDS); } - log.error("Unable to connect to Exchange Connect Endpoint, stop retrying.", err); + log.error("Unable to connect to Exchange Connect Endpoint, stop retrying."); return Flowable.error(err); }) ); From 1532880bcfc3896d8ddef09911950242f7a0663a Mon Sep 17 00:00:00 2001 From: Julien Giovaresco Date: Tue, 26 Mar 2024 11:45:10 +0100 Subject: [PATCH 4/4] fix: use default implementation for Command/Reply adapters declaration Controllers may not need adapters for Command/Reply. Using a default implementation in the interface simplifies the DX by avoiding to implement those methods. --- .../ControllerCommandHandlersFactory.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/gravitee-exchange-api/src/main/java/io/gravitee/exchange/api/controller/ControllerCommandHandlersFactory.java b/gravitee-exchange-api/src/main/java/io/gravitee/exchange/api/controller/ControllerCommandHandlersFactory.java index d2a26bd..45d60e3 100644 --- a/gravitee-exchange-api/src/main/java/io/gravitee/exchange/api/controller/ControllerCommandHandlersFactory.java +++ b/gravitee-exchange-api/src/main/java/io/gravitee/exchange/api/controller/ControllerCommandHandlersFactory.java @@ -40,19 +40,23 @@ public interface ControllerCommandHandlersFactory { * @param controllerCommandContext the command context * @return a list of command decorators */ - List, ? extends Command, ? extends Reply>> buildCommandAdapters( + default List, ? extends Command, ? extends Reply>> buildCommandAdapters( final ControllerCommandContext controllerCommandContext, final ProtocolVersion protocolVersion - ); + ) { + return List.of(); + } /** - * Build a list of command decorators dedicated to the specified context. + * Build a list of reply decorators dedicated to the specified context. * * @param controllerCommandContext the command context - * @return a list of command decorators + * @return a list of reply decorators */ - List, ? extends Reply>> buildReplyAdapters( + default List, ? extends Reply>> buildReplyAdapters( final ControllerCommandContext controllerCommandContext, final ProtocolVersion protocolVersion - ); + ) { + return List.of(); + } }