From 441de42908c8704275919548ee927579b033e2ef Mon Sep 17 00:00:00 2001 From: ayash Date: Sun, 16 Mar 2025 21:55:07 +0530 Subject: [PATCH 01/27] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 6 +++--- ballerina/Dependencies.toml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index a438078b0..e212c0ade 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -22,19 +22,19 @@ path = "../native/build/libs/websocket-native-2.14.0-SNAPSHOT.jar" groupId = "io.ballerina.stdlib" artifactId = "http-native" version = "2.14.0" -path = "./lib/http-native-2.14.0-20250305-195600-c559baa.jar" +path = "./lib/http-native-2.14.0-20250311-152000-c0b9408.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "mime-native" version = "2.12.0" -path = "./lib/mime-native-2.12.0-20250305-174800-8528404.jar" +path = "./lib/mime-native-2.12.0-20250311-141300-98bc7d6.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "constraint-native" version = "1.7.0" -path = "./lib/constraint-native-1.7.0-20250305-165400-52275bd.jar" +path = "./lib/constraint-native-1.7.0-20250311-133400-acfb095.jar" [[platform.java21.dependency]] groupId = "io.netty" diff --git a/ballerina/Dependencies.toml b/ballerina/Dependencies.toml index 4ac31dcd6..5eea934ff 100644 --- a/ballerina/Dependencies.toml +++ b/ballerina/Dependencies.toml @@ -5,7 +5,7 @@ [ballerina] dependencies-toml-version = "2" -distribution-version = "2201.12.0-20250228-201300-8d411a0f" +distribution-version = "2201.12.0-20250311-124800-b9003fed" [[package]] org = "ballerina" From 9c23d5554c65d3cadad06c1e0a9efdb16440fe68 Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 17 Mar 2025 09:27:37 +0530 Subject: [PATCH 02/27] Add connection closure timeout configuration to WebSocket service --- ballerina/annotation.bal | 12 +++-- .../tests/connection_closure_timeout.bal | 50 +++++++++++++++++++ ballerina/websocket_caller.bal | 4 +- ballerina/websocket_sync_client.bal | 2 +- .../stdlib/websocket/WebSocketConstants.java | 2 + .../actions/websocketconnector/Close.java | 16 +++++- .../server/WebSocketServerService.java | 7 +++ 7 files changed, 85 insertions(+), 8 deletions(-) create mode 100644 ballerina/tests/connection_closure_timeout.bal diff --git a/ballerina/annotation.bal b/ballerina/annotation.bal index 3286fcc18..fbe886eac 100644 --- a/ballerina/annotation.bal +++ b/ballerina/annotation.bal @@ -22,14 +22,19 @@ # # + subProtocols - Negotiable sub protocol by the service # + idleTimeout - Idle timeout for the client connection. Upon timeout, `onIdleTimeout` resource (if defined) -# in the server service will be triggered. Note that this overrides the `timeout` config -# in the `websocket:Listener`, which is applicable only for the initial HTTP upgrade request +# in the server service will be triggered. Note that this overrides the `timeout` config +# in the `websocket:Listener`, which is applicable only for the initial HTTP upgrade request # + maxFrameSize - The maximum payload size of a WebSocket frame in bytes. -# If this is not set or is negative or zero, the default frame size, which is 65536 will be used +# If this is not set or is negative or zero, the default frame size, which is 65536 will be used # + auth - Listener authentication configurations # + validation - Enable/disable constraint validation # + dispatcherKey - The key which is going to be used for dispatching to custom remote functions. # + dispatcherStreamId - The identifier used to distinguish between requests and their corresponding responses in a multiplexing scenario. +# + connectionClosureTimeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint before closing the +# connection. If the timeout exceeds, then the connection is terminated even though a close frame +# is not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits +# until a close frame is received. If the WebSocket frame is received from the remote endpoint +# within the waiting period, the connection is terminated immediately. public type WSServiceConfig record {| string[] subProtocols = []; decimal idleTimeout = 0; @@ -38,6 +43,7 @@ public type WSServiceConfig record {| boolean validation = true; string dispatcherKey?; string dispatcherStreamId?; + decimal connectionClosureTimeout = 60; |}; # The annotation which is used to configure a WebSocket service. diff --git a/ballerina/tests/connection_closure_timeout.bal b/ballerina/tests/connection_closure_timeout.bal new file mode 100644 index 000000000..d955dd718 --- /dev/null +++ b/ballerina/tests/connection_closure_timeout.bal @@ -0,0 +1,50 @@ +// Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/lang.runtime; +import ballerina/test; + +listener Listener connectionClosureTimeoutListener = new (22100); + +@ServiceConfig { + connectionClosureTimeout: 10 +} +service / on connectionClosureTimeoutListener { + resource function get .() returns Service|UpgradeError { + return new ConnectionClosureTimeoutService(); + } +} + +service class ConnectionClosureTimeoutService { + *Service; + + remote function onMessage(Caller caller, string data) returns error? { + _ = start caller->close(); + runtime:sleep(1); + test:assertTrue(caller.isOpen()); + runtime:sleep(10); + test:assertTrue(!caller.isOpen()); + } +} + +@test:Config { + groups: ["connectionClosureTimeout"] +} +public function testConnectionClosureTimeoutFromServer() returns Error? { + Client wsClient = check new ("ws://localhost:22100/"); + check wsClient->writeMessage("Hi"); + runtime:sleep(20); +} diff --git a/ballerina/websocket_caller.bal b/ballerina/websocket_caller.bal index f8cfa5dfb..ebae7c408 100644 --- a/ballerina/websocket_caller.bal +++ b/ballerina/websocket_caller.bal @@ -88,7 +88,7 @@ public isolated client class Caller { # within the waiting period, the connection is terminated immediately # + return - A `websocket:Error` if an error occurs when sending remote isolated function close(int? statusCode = 1000, string? reason = (), - decimal timeout = 60) returns Error? { + decimal? timeout = ()) returns Error? { int code = 1000; if (statusCode is int) { if (statusCode <= 999 || statusCode >= 1004 && statusCode <= 1006 || statusCode >= 1012 && @@ -101,7 +101,7 @@ public isolated client class Caller { return self.externClose(code, reason is () ? "" : reason, timeout); } - isolated function externClose(int statusCode, string reason, decimal timeoutInSecs) returns Error? = @java:Method { + isolated function externClose(int statusCode, string reason, decimal? timeoutInSecs = ()) returns Error? = @java:Method { 'class: "io.ballerina.stdlib.websocket.actions.websocketconnector.Close" } external; diff --git a/ballerina/websocket_sync_client.bal b/ballerina/websocket_sync_client.bal index bfec57e71..c9afa1aee 100644 --- a/ballerina/websocket_sync_client.bal +++ b/ballerina/websocket_sync_client.bal @@ -226,7 +226,7 @@ public isolated client class Client { } } - isolated function externClose(int statusCode, string reason, decimal timeoutInSecs) + isolated function externClose(int statusCode, string reason, decimal? timeoutInSecs) returns Error? = @java:Method { 'class: "io.ballerina.stdlib.websocket.actions.websocketconnector.Close" } external; diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java index e3ad7aa16..4acf9bdee 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java @@ -45,6 +45,8 @@ public class WebSocketConstants { public static final String WEBSOCKET_ANNOTATION_CONFIGURATION = "ServiceConfig"; public static final BString ANNOTATION_ATTR_SUB_PROTOCOLS = StringUtils.fromString("subProtocols"); public static final BString ANNOTATION_ATTR_IDLE_TIMEOUT = StringUtils.fromString("idleTimeout"); + public static final BString ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT = + StringUtils.fromString("connectionClosureTimeout"); public static final BString ANNOTATION_ATTR_READ_IDLE_TIMEOUT = StringUtils.fromString("readTimeout"); public static final BString ANNOTATION_ATTR_TIMEOUT = StringUtils.fromString("timeout"); public static final BString ANNOTATION_ATTR_MAX_FRAME_SIZE = StringUtils.fromString("maxFrameSize"); diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java index d8b47d373..fa5700d46 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java @@ -28,6 +28,7 @@ import io.ballerina.stdlib.websocket.observability.WebSocketObservabilityConstants; import io.ballerina.stdlib.websocket.observability.WebSocketObservabilityUtil; import io.ballerina.stdlib.websocket.server.WebSocketConnectionInfo; +import io.ballerina.stdlib.websocket.server.WebSocketServerService; import io.netty.channel.ChannelFuture; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,20 +46,21 @@ public class Close { private static final Logger log = LoggerFactory.getLogger(Close.class); public static Object externClose(Environment env, BObject wsConnection, long statusCode, BString reason, - BDecimal timeoutInSecs) { + Object bTimeoutInSecs) { return env.yieldAndRun(() -> { CompletableFuture balFuture = new CompletableFuture<>(); WebSocketConnectionInfo connectionInfo = (WebSocketConnectionInfo) wsConnection .getNativeData(WebSocketConstants.NATIVE_DATA_WEBSOCKET_CONNECTION_INFO); WebSocketObservabilityUtil.observeResourceInvocation(env, connectionInfo, WebSocketConstants.RESOURCE_NAME_CLOSE); + int timeoutInSecs = getConnectionClosureTimeout(bTimeoutInSecs, connectionInfo); try { CountDownLatch countDownLatch = new CountDownLatch(1); List errors = new ArrayList<>(1); ChannelFuture closeFuture = initiateConnectionClosure(errors, (int) statusCode, reason.getValue(), connectionInfo, countDownLatch); connectionInfo.getWebSocketConnection().readNextFrame(); - waitForTimeout(errors, (int) timeoutInSecs.floatValue(), countDownLatch, connectionInfo); + waitForTimeout(errors, timeoutInSecs, countDownLatch, connectionInfo); closeFuture.channel().close().addListener(future -> { WebSocketUtil.setListenerOpenField(connectionInfo); if (errors.isEmpty()) { @@ -81,6 +83,16 @@ public static Object externClose(Environment env, BObject wsConnection, long sta }); } + private static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { + int connectionClosureTimeout = 60; + if (bTimeoutInSecs instanceof BDecimal) { + return (int) ((BDecimal) bTimeoutInSecs).floatValue(); + } else if (connectionInfo.getService() instanceof WebSocketServerService webSocketServerService) { + return webSocketServerService.getConnectionClosureTimeout(); + } + return connectionClosureTimeout; + } + private static ChannelFuture initiateConnectionClosure(List errors, int statusCode, String reason, WebSocketConnectionInfo connectionInfo, CountDownLatch latch) throws IllegalAccessException { WebSocketConnection webSocketConnection = connectionInfo.getWebSocketConnection(); diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java index 41a12b787..e230f7332 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java @@ -44,6 +44,7 @@ public class WebSocketServerService extends WebSocketService { private int idleTimeoutInSeconds = 0; private boolean enableValidation = true; private String dispatchingKey = null; + private int connectionClosureTimeout; public WebSocketServerService(BObject service, Runtime runtime, String basePath) { super(service, runtime); @@ -56,6 +57,8 @@ private void populateConfigs(String basePath) { negotiableSubProtocols = WebSocketUtil.findNegotiableSubProtocols(configAnnotation); idleTimeoutInSeconds = WebSocketUtil.findTimeoutInSeconds(configAnnotation, WebSocketConstants.ANNOTATION_ATTR_IDLE_TIMEOUT, 0); + connectionClosureTimeout = WebSocketUtil.findTimeoutInSeconds(configAnnotation, + WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT, 60); maxFrameSize = WebSocketUtil.findMaxFrameSize(configAnnotation); enableValidation = configAnnotation.getBooleanValue(ANNOTATION_ATTR_VALIDATION_ENABLED); if (configAnnotation.getStringValue(ANNOTATION_ATTR_DISPATCHER_KEY) != null) { @@ -101,4 +104,8 @@ public String getDispatchingKey() { public String getBasePath() { return basePath; } + + public int getConnectionClosureTimeout() { + return connectionClosureTimeout; + } } From 3e71ae4f081682934f0cce328c2d5eea04969368 Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 17 Mar 2025 22:44:15 +0530 Subject: [PATCH 03/27] Add default connection closure timeout constant --- .../io/ballerina/stdlib/websocket/WebSocketConstants.java | 1 + .../stdlib/websocket/actions/websocketconnector/Close.java | 5 +++-- .../stdlib/websocket/server/WebSocketServerService.java | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java index 4acf9bdee..dee5950f5 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java @@ -115,6 +115,7 @@ public class WebSocketConstants { public static final int STATUS_CODE_FOR_NO_STATUS_CODE_PRESENT = 1005; public static final int DEFAULT_MAX_FRAME_SIZE = 65536; + public static final int DEFAULT_CONNECTION_CLOSURE_TIMEOUT = 60; // Warning suppression public static final String UNCHECKED = "unchecked"; diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java index fa5700d46..98edef9a2 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java @@ -39,6 +39,8 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import static io.ballerina.stdlib.websocket.WebSocketConstants.DEFAULT_CONNECTION_CLOSURE_TIMEOUT; + /** * {@code Get} is the GET action implementation of the HTTP Connector. */ @@ -84,13 +86,12 @@ public static Object externClose(Environment env, BObject wsConnection, long sta } private static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { - int connectionClosureTimeout = 60; if (bTimeoutInSecs instanceof BDecimal) { return (int) ((BDecimal) bTimeoutInSecs).floatValue(); } else if (connectionInfo.getService() instanceof WebSocketServerService webSocketServerService) { return webSocketServerService.getConnectionClosureTimeout(); } - return connectionClosureTimeout; + return DEFAULT_CONNECTION_CLOSURE_TIMEOUT; } private static ChannelFuture initiateConnectionClosure(List errors, int statusCode, diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java index e230f7332..0ce8703a5 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java @@ -32,6 +32,7 @@ import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_DISPATCHER_KEY; import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_VALIDATION_ENABLED; +import static io.ballerina.stdlib.websocket.WebSocketConstants.DEFAULT_CONNECTION_CLOSURE_TIMEOUT; /** * WebSocket service for service dispatching. @@ -58,7 +59,7 @@ private void populateConfigs(String basePath) { idleTimeoutInSeconds = WebSocketUtil.findTimeoutInSeconds(configAnnotation, WebSocketConstants.ANNOTATION_ATTR_IDLE_TIMEOUT, 0); connectionClosureTimeout = WebSocketUtil.findTimeoutInSeconds(configAnnotation, - WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT, 60); + WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT, DEFAULT_CONNECTION_CLOSURE_TIMEOUT); maxFrameSize = WebSocketUtil.findMaxFrameSize(configAnnotation); enableValidation = configAnnotation.getBooleanValue(ANNOTATION_ATTR_VALIDATION_ENABLED); if (configAnnotation.getStringValue(ANNOTATION_ATTR_DISPATCHER_KEY) != null) { From d88e8f36b2e556d1e3483d07d851adb531296f25 Mon Sep 17 00:00:00 2001 From: ayash Date: Fri, 28 Mar 2025 09:30:17 +0530 Subject: [PATCH 04/27] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 7 +++---- ballerina/Dependencies.toml | 3 +-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index 5bb8d94a3..3a9267535 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -22,20 +22,19 @@ path = "../native/build/libs/websocket-native-2.14.1-SNAPSHOT.jar" groupId = "io.ballerina.stdlib" artifactId = "http-native" version = "2.14.0" -path = "./lib/http-native-2.14.0-20250311-152000-c0b9408.jar" - +path = "./lib/http-native-2.14.0.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "mime-native" version = "2.12.0" -path = "./lib/mime-native-2.12.0-20250311-141300-98bc7d6.jar" +path = "./lib/mime-native-2.12.0.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "constraint-native" version = "1.7.0" -path = "./lib/constraint-native-1.7.0-20250311-133400-acfb095.jar" +path = "./lib/constraint-native-1.7.0.jar" [[platform.java21.dependency]] groupId = "io.netty" diff --git a/ballerina/Dependencies.toml b/ballerina/Dependencies.toml index 672f2b879..bf4b4e276 100644 --- a/ballerina/Dependencies.toml +++ b/ballerina/Dependencies.toml @@ -5,8 +5,7 @@ [ballerina] dependencies-toml-version = "2" -distribution-version = "2201.12.0-20250311-124800-b9003fed" - +distribution-version = "2201.12.0" [[package]] org = "ballerina" From 9d29c757f484a579b45f0b0447d67befae8f31ae Mon Sep 17 00:00:00 2001 From: ayash Date: Fri, 28 Mar 2025 09:32:46 +0530 Subject: [PATCH 05/27] Update changelog --- changelog.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changelog.md b/changelog.md index 673f5472e..4d3e9c266 100644 --- a/changelog.md +++ b/changelog.md @@ -5,6 +5,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## [Unreleased] +### Added + +- [Introduce Service Config Annotation for connectionClosureTimeout in Websocket module](https://github.com/ballerina-platform/ballerina-library/issues/7697) + ### Fixed - [Fix JSON response parsing issue with escaped double quotes](https://github.com/ballerina-platform/ballerina-library/issues/7720) From 5b6c9420134252db3e977c55e5e085e2856037a4 Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 31 Mar 2025 09:40:21 +0530 Subject: [PATCH 06/27] Refactor connection closure timeout handling in WebSocket connector --- .../stdlib/websocket/actions/websocketconnector/Close.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java index 98edef9a2..596ecb744 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java @@ -86,12 +86,13 @@ public static Object externClose(Environment env, BObject wsConnection, long sta } private static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { + int timeoutInSecs = DEFAULT_CONNECTION_CLOSURE_TIMEOUT; if (bTimeoutInSecs instanceof BDecimal) { - return (int) ((BDecimal) bTimeoutInSecs).floatValue(); + timeoutInSecs = (int) ((BDecimal) bTimeoutInSecs).floatValue(); } else if (connectionInfo.getService() instanceof WebSocketServerService webSocketServerService) { - return webSocketServerService.getConnectionClosureTimeout(); + timeoutInSecs = webSocketServerService.getConnectionClosureTimeout(); } - return DEFAULT_CONNECTION_CLOSURE_TIMEOUT; + return timeoutInSecs; } private static ChannelFuture initiateConnectionClosure(List errors, int statusCode, From fb232b6b2835d14f83c271a082453d7b3b588a51 Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 31 Mar 2025 14:43:41 +0530 Subject: [PATCH 07/27] Update the spec --- ballerina/annotation.bal | 10 +++++----- docs/spec/spec.md | 8 +++++++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/ballerina/annotation.bal b/ballerina/annotation.bal index fbe886eac..1198501bd 100644 --- a/ballerina/annotation.bal +++ b/ballerina/annotation.bal @@ -30,11 +30,11 @@ # + validation - Enable/disable constraint validation # + dispatcherKey - The key which is going to be used for dispatching to custom remote functions. # + dispatcherStreamId - The identifier used to distinguish between requests and their corresponding responses in a multiplexing scenario. -# + connectionClosureTimeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint before closing the -# connection. If the timeout exceeds, then the connection is terminated even though a close frame -# is not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits -# until a close frame is received. If the WebSocket frame is received from the remote endpoint -# within the waiting period, the connection is terminated immediately. +# + connectionClosureTimeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint +# before closing the connection. If the timeout exceeds, then the connection is terminated even though a close frame is +# not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits until a close frame is +# received. If the WebSocket frame is received from the remote endpoint within the waiting period, the connection is +# terminated immediately. public type WSServiceConfig record {| string[] subProtocols = []; decimal idleTimeout = 0; diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 5dea9b243..282a2c9be 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -216,6 +216,11 @@ When writing the service, following configurations can be provided, # + auth - Listener authenticaton configurations # + dispatcherKey - The key which is going to be used for dispatching to custom remote functions. # + dispatcherStreamId - The identifier used to distinguish between requests and their corresponding responses in a multiplexing scenario. +# + connectionClosureTimeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint +# before closing the connection. If the timeout exceeds, then the connection is terminated even though a close frame is +# not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits until a close frame is +# received. If the WebSocket frame is received from the remote endpoint within the waiting period, the connection is +# terminated immediately. public type WSServiceConfig record {| string[] subProtocols = []; decimal idleTimeout = 0; @@ -224,6 +229,7 @@ public type WSServiceConfig record {| boolean validation = true; string dispatcherKey?; string dispatcherStreamId?; + decimal connectionClosureTimeout = 60; |}; ``` @@ -760,4 +766,4 @@ public function main() returns error? { string stringResp = check 'string:fromBytes(byteResp); io:println(stringResp); } -``` \ No newline at end of file +``` From c86d1002da9ea9c861c201703b74dd1dbc1ecf96 Mon Sep 17 00:00:00 2001 From: ayash Date: Wed, 2 Apr 2025 07:22:53 +0530 Subject: [PATCH 08/27] Update sendCloseFrame method to use connectionClosureTimeout --- .../io/ballerina/stdlib/websocket/WebSocketConstants.java | 1 - .../stdlib/websocket/WebSocketResourceCallback.java | 5 +++-- .../stdlib/websocket/actions/websocketconnector/Close.java | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java index 9f84a600c..600d0f5bd 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java @@ -148,7 +148,6 @@ public class WebSocketConstants { public static final BString CLOSE_FRAME_REASON = StringUtils.fromString("reason"); public static final String PREDEFINED_CLOSE_FRAME_TYPE = "PredefinedCloseFrameType"; public static final String CUSTOM_CLOSE_FRAME_TYPE = "CustomCloseFrameType"; - public static final int CLOSE_FRAME_DEFAULT_TIMEOUT = 60; private WebSocketConstants() { } diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketResourceCallback.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketResourceCallback.java index 14aa45e37..0333708d9 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketResourceCallback.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketResourceCallback.java @@ -48,11 +48,11 @@ import java.util.concurrent.CountDownLatch; import static io.ballerina.runtime.api.utils.StringUtils.fromString; -import static io.ballerina.stdlib.websocket.WebSocketConstants.CLOSE_FRAME_DEFAULT_TIMEOUT; import static io.ballerina.stdlib.websocket.WebSocketConstants.CLOSE_FRAME_TYPE; import static io.ballerina.stdlib.websocket.WebSocketConstants.PACKAGE_WEBSOCKET; import static io.ballerina.stdlib.websocket.WebSocketConstants.STREAMING_NEXT_FUNCTION; import static io.ballerina.stdlib.websocket.WebSocketResourceDispatcher.dispatchOnError; +import static io.ballerina.stdlib.websocket.actions.websocketconnector.Close.getConnectionClosureTimeout; import static io.ballerina.stdlib.websocket.actions.websocketconnector.Close.initiateConnectionClosure; import static io.ballerina.stdlib.websocket.actions.websocketconnector.Close.waitForTimeout; import static io.ballerina.stdlib.websocket.actions.websocketconnector.WebSocketConnector.fromByteArray; @@ -152,7 +152,8 @@ public static void sendCloseFrame(Object result, WebSocketConnectionInfo connect ChannelFuture closeFuture = initiateConnectionClosure(errors, statusCode, reason, connectionInfo, countDownLatch); connectionInfo.getWebSocketConnection().readNextFrame(); - waitForTimeout(errors, CLOSE_FRAME_DEFAULT_TIMEOUT, countDownLatch, connectionInfo); + int timeoutInSecs = getConnectionClosureTimeout(null, connectionInfo); + waitForTimeout(errors, timeoutInSecs, countDownLatch, connectionInfo); closeFuture.channel().close().addListener(future -> { WebSocketUtil.setListenerOpenField(connectionInfo); }); diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java index bbe833957..f2c7e70cf 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java @@ -85,7 +85,7 @@ public static Object externClose(Environment env, BObject wsConnection, long sta }); } - private static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { + public static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { int timeoutInSecs = DEFAULT_CONNECTION_CLOSURE_TIMEOUT; if (bTimeoutInSecs instanceof BDecimal) { timeoutInSecs = (int) ((BDecimal) bTimeoutInSecs).floatValue(); @@ -95,8 +95,8 @@ private static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketC return timeoutInSecs; } - private static ChannelFuture initiateConnectionClosure(List errors, int statusCode, - String reason, WebSocketConnectionInfo connectionInfo, CountDownLatch latch) throws IllegalAccessException { + public static ChannelFuture initiateConnectionClosure(List errors, int statusCode, String reason, + WebSocketConnectionInfo connectionInfo, CountDownLatch latch) throws IllegalAccessException { WebSocketConnection webSocketConnection = connectionInfo.getWebSocketConnection(); ChannelFuture closeFuture; closeFuture = webSocketConnection.initiateConnectionClosure(statusCode, reason); From 27442b902178fc056332ba652d179de866554abd Mon Sep 17 00:00:00 2001 From: ayash Date: Wed, 2 Apr 2025 09:43:59 +0530 Subject: [PATCH 09/27] Add test for connection closure timeout when returning close frames --- .../tests/connection_closure_timeout.bal | 56 +++++++++++++++---- 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/ballerina/tests/connection_closure_timeout.bal b/ballerina/tests/connection_closure_timeout.bal index d955dd718..af4bc7797 100644 --- a/ballerina/tests/connection_closure_timeout.bal +++ b/ballerina/tests/connection_closure_timeout.bal @@ -1,4 +1,4 @@ -// Copyright (c) 2025, WSO2 LLC. (http://www.wso2.org). +// Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). // // WSO2 LLC. licenses this file to you under the Apache License, // Version 2.0 (the "License"); you may not use this file except @@ -19,8 +19,11 @@ import ballerina/test; listener Listener connectionClosureTimeoutListener = new (22100); +map callers = {}; + @ServiceConfig { - connectionClosureTimeout: 10 + dispatcherKey: "event", + connectionClosureTimeout: 5 } service / on connectionClosureTimeoutListener { resource function get .() returns Service|UpgradeError { @@ -31,20 +34,51 @@ service / on connectionClosureTimeoutListener { service class ConnectionClosureTimeoutService { *Service; - remote function onMessage(Caller caller, string data) returns error? { + remote function onSubscribe(Caller caller) returns error? { + callers["onSubscribe"] = caller; _ = start caller->close(); - runtime:sleep(1); - test:assertTrue(caller.isOpen()); - runtime:sleep(10); - test:assertTrue(!caller.isOpen()); } + + remote function onChat(Caller caller) returns NormalClosure? { + callers["onChat"] = caller; + return NORMAL_CLOSURE; + } + + remote function onIsClosed(record {string event; string name;} data) returns boolean|error { + Caller? caller = callers[data.name]; + if caller is Caller { + return !caller.isOpen(); + } + return error("Caller not found"); + } +} + +@test:Config { + groups: ["connectionClosureTimeout"] +} +public function testConnectionClosureTimeoutCaller() returns error? { + Client wsClient1 = check new ("ws://localhost:22100/"); + check wsClient1->writeMessage({event: "subscribe"}); + runtime:sleep(8); + + // Check if the connection is closed using another client + Client wsClient2 = check new ("ws://localhost:22100/"); + check wsClient2->writeMessage({event: "is_closed", name: "onSubscribe"}); + boolean isClosed = check wsClient2->readMessage(); + test:assertTrue(isClosed); } @test:Config { groups: ["connectionClosureTimeout"] } -public function testConnectionClosureTimeoutFromServer() returns Error? { - Client wsClient = check new ("ws://localhost:22100/"); - check wsClient->writeMessage("Hi"); - runtime:sleep(20); +public function testConnectionClosureTimeoutCloseFrames() returns error? { + Client wsClient1 = check new ("ws://localhost:22100/"); + check wsClient1->writeMessage({event: "chat"}); + runtime:sleep(8); + + // Check if the connection is closed using another client + Client wsClient2 = check new ("ws://localhost:22100/"); + check wsClient2->writeMessage({event: "is_closed", name: "onChat"}); + boolean isClosed = check wsClient2->readMessage(); + test:assertTrue(isClosed); } From e8cfcf8ce15718cfef997e6dc56c1cff4952cfd6 Mon Sep 17 00:00:00 2001 From: ayash Date: Wed, 2 Apr 2025 09:55:30 +0530 Subject: [PATCH 10/27] [Automated] Update the native jar versions --- ballerina/Ballerina.toml | 8 ++++---- ballerina/CompilerPlugin.toml | 2 +- ballerina/Dependencies.toml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ballerina/Ballerina.toml b/ballerina/Ballerina.toml index 3a9267535..826b7eee2 100644 --- a/ballerina/Ballerina.toml +++ b/ballerina/Ballerina.toml @@ -1,7 +1,7 @@ [package] org = "ballerina" name = "websocket" -version = "2.14.1" +version = "2.15.0" authors = ["Ballerina"] keywords = ["ws", "network", "bi-directional", "streaming", "service", "client"] repository = "https://github.com/ballerina-platform/module-ballerina-websocket" @@ -15,8 +15,8 @@ graalvmCompatible = true [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" artifactId = "websocket-native" -version = "2.14.1" -path = "../native/build/libs/websocket-native-2.14.1-SNAPSHOT.jar" +version = "2.15.0" +path = "../native/build/libs/websocket-native-2.15.0-SNAPSHOT.jar" [[platform.java21.dependency]] groupId = "io.ballerina.stdlib" @@ -85,5 +85,5 @@ version = "4.1.118.Final" path = "./lib/netty-handler-proxy-4.1.118.Final.jar" [[platform.java21.dependency]] -path = "../test-utils/build/libs/websocket-test-utils-2.14.1-SNAPSHOT.jar" +path = "../test-utils/build/libs/websocket-test-utils-2.15.0-SNAPSHOT.jar" scope = "testOnly" diff --git a/ballerina/CompilerPlugin.toml b/ballerina/CompilerPlugin.toml index f0c2e2e19..35c0f6193 100644 --- a/ballerina/CompilerPlugin.toml +++ b/ballerina/CompilerPlugin.toml @@ -3,4 +3,4 @@ id = "websocket-compiler-plugin" class = "io.ballerina.stdlib.websocket.plugin.WebSocketCompilerPlugin" [[dependency]] -path = "../compiler-plugin/build/libs/websocket-compiler-plugin-2.14.1-SNAPSHOT.jar" +path = "../compiler-plugin/build/libs/websocket-compiler-plugin-2.15.0-SNAPSHOT.jar" diff --git a/ballerina/Dependencies.toml b/ballerina/Dependencies.toml index bf4b4e276..729ac223f 100644 --- a/ballerina/Dependencies.toml +++ b/ballerina/Dependencies.toml @@ -342,7 +342,7 @@ dependencies = [ [[package]] org = "ballerina" name = "websocket" -version = "2.14.1" +version = "2.15.0" dependencies = [ {org = "ballerina", name = "auth"}, {org = "ballerina", name = "constraint"}, From 6db92c9f1069d2816bfa551cdcfa1cb3c7019a75 Mon Sep 17 00:00:00 2001 From: ayash Date: Wed, 2 Apr 2025 09:56:53 +0530 Subject: [PATCH 11/27] Bump version to 2.15.0 --- gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index b6314f160..28023d5be 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,6 +1,6 @@ org.gradle.caching=true group=io.ballerina.stdlib -version=2.14.1-SNAPSHOT +version=2.15.0-SNAPSHOT ballerinaLangVersion=2201.12.0 ballerinaTomlParserVersion=1.2.2 nettyVersion=4.1.118.Final From 8912ad0e407840023d29769560b153225fa6f2ee Mon Sep 17 00:00:00 2001 From: ayash Date: Wed, 2 Apr 2025 21:43:23 +0530 Subject: [PATCH 12/27] Update the behavior of handling negative values in connectionClosureTimeout and add tests --- ballerina/annotation.bal | 6 ++-- .../tests/connection_closure_timeout.bal | 33 ++++++++++++++++--- ballerina/websocket_caller.bal | 10 ++++-- ballerina/websocket_sync_client.bal | 11 +++++-- docs/spec/spec.md | 4 +-- .../stdlib/websocket/WebSocketConstants.java | 1 - .../stdlib/websocket/WebSocketUtil.java | 10 ++++++ .../actions/websocketconnector/Close.java | 4 +-- .../server/WebSocketServerService.java | 3 +- 9 files changed, 61 insertions(+), 21 deletions(-) diff --git a/ballerina/annotation.bal b/ballerina/annotation.bal index 1198501bd..1d11832de 100644 --- a/ballerina/annotation.bal +++ b/ballerina/annotation.bal @@ -32,9 +32,9 @@ # + dispatcherStreamId - The identifier used to distinguish between requests and their corresponding responses in a multiplexing scenario. # + connectionClosureTimeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint # before closing the connection. If the timeout exceeds, then the connection is terminated even though a close frame is -# not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits until a close frame is -# received. If the WebSocket frame is received from the remote endpoint within the waiting period, the connection is -# terminated immediately. +# not received from the remote endpoint. If the value is -1, then the connection waits until a close frame is +# received, and any other negative value results in an error. If the WebSocket frame is received from the remote endpoint +# within the waiting period, the connection is terminated immediately. public type WSServiceConfig record {| string[] subProtocols = []; decimal idleTimeout = 0; diff --git a/ballerina/tests/connection_closure_timeout.bal b/ballerina/tests/connection_closure_timeout.bal index af4bc7797..1148b583f 100644 --- a/ballerina/tests/connection_closure_timeout.bal +++ b/ballerina/tests/connection_closure_timeout.bal @@ -17,15 +17,14 @@ import ballerina/lang.runtime; import ballerina/test; -listener Listener connectionClosureTimeoutListener = new (22100); - map callers = {}; +error negativeTimeoutErrorMessage = error(""); @ServiceConfig { dispatcherKey: "event", connectionClosureTimeout: 5 } -service / on connectionClosureTimeoutListener { +service / on new Listener(22100) { resource function get .() returns Service|UpgradeError { return new ConnectionClosureTimeoutService(); } @@ -36,7 +35,7 @@ service class ConnectionClosureTimeoutService { remote function onSubscribe(Caller caller) returns error? { callers["onSubscribe"] = caller; - _ = start caller->close(); + check caller->close(); } remote function onChat(Caller caller) returns NormalClosure? { @@ -44,6 +43,13 @@ service class ConnectionClosureTimeoutService { return NORMAL_CLOSURE; } + remote function onNegativeTimeout(Caller caller) returns error? { + Error? close = caller->close(timeout = -10); + if close is Error { + negativeTimeoutErrorMessage = close; + } + } + remote function onIsClosed(record {string event; string name;} data) returns boolean|error { Caller? caller = callers[data.name]; if caller is Caller { @@ -51,6 +57,10 @@ service class ConnectionClosureTimeoutService { } return error("Caller not found"); } + + remote function onNegativeTimeoutErrorMessage(string data) returns string { + return negativeTimeoutErrorMessage.message(); + } } @test:Config { @@ -82,3 +92,18 @@ public function testConnectionClosureTimeoutCloseFrames() returns error? { boolean isClosed = check wsClient2->readMessage(); test:assertTrue(isClosed); } + +@test:Config { + groups: ["connectionClosureTimeout"] +} +public function testConnectionClosureTimeoutCallerNegativeTimeout() returns error? { + Client wsClient1 = check new ("ws://localhost:22100/"); + check wsClient1->writeMessage({event: "negative_timeout"}); + runtime:sleep(1); + + // Check error in the service using another client + Client wsClient2 = check new ("ws://localhost:22100/"); + check wsClient2->writeMessage({event: "negative_timeout_error_message"}); + string errorMessage = check wsClient2->readMessage(); + test:assertEquals(errorMessage, "Invalid timeout value: -10"); +} diff --git a/ballerina/websocket_caller.bal b/ballerina/websocket_caller.bal index ebae7c408..165060b8c 100644 --- a/ballerina/websocket_caller.bal +++ b/ballerina/websocket_caller.bal @@ -83,9 +83,9 @@ public isolated client class Caller { # + reason - Reason for closing the connection # + timeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint before closing the # connection. If the timeout exceeds, then the connection is terminated even though a close frame - # is not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits - # until a close frame is received. If the WebSocket frame is received from the remote endpoint - # within the waiting period, the connection is terminated immediately + # is not received from the remote endpoint. If the value is -1, then the connection waits + # until a close frame is received, and any other negative value results in an error. If the WebSocket frame is received + # from the remote endpoint within the waiting period, the connection is terminated immediately # + return - A `websocket:Error` if an error occurs when sending remote isolated function close(int? statusCode = 1000, string? reason = (), decimal? timeout = ()) returns Error? { @@ -98,6 +98,10 @@ public isolated client class Caller { } code = statusCode; } + if (timeout is decimal && timeout < 0d && timeout != -1d) { + string errorMessage = "Invalid timeout value: " + timeout.toString(); + return error Error(errorMessage); + } return self.externClose(code, reason is () ? "" : reason, timeout); } diff --git a/ballerina/websocket_sync_client.bal b/ballerina/websocket_sync_client.bal index c9afa1aee..f97cdbca9 100644 --- a/ballerina/websocket_sync_client.bal +++ b/ballerina/websocket_sync_client.bal @@ -105,9 +105,10 @@ public isolated client class Client { # + reason - Reason for closing the connection # + timeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint before closing the # connection. If the timeout exceeds, then the connection is terminated even though a close frame - # is not received from the remote endpoint. If the value is < 0 (e.g., -1), then the connection - # waits until a close frame is received. If the WebSocket frame is received from the remote - # endpoint within the waiting period, the connection is terminated immediately + # is not received from the remote endpoint. If the value is -1, then the connection + # waits until a close frame is received, and any other negative value results in an error. + # If the WebSocket frame is received from the remote endpoint within the waiting period, + # the connection is terminated immediately # + return - A `websocket:Error` if an error occurs while closing the WebSocket connection remote isolated function close(int? statusCode = 1000, string? reason = (), decimal timeout = 60) returns Error? { int code = 1000; @@ -119,6 +120,10 @@ public isolated client class Client { } code = statusCode; } + if (timeout < 0d && timeout != -1d) { + string errorMessage = "Invalid timeout value: " + timeout.toString(); + return error Error(errorMessage); + } return self.externClose(code, reason is () ? "" : reason, timeout); } diff --git a/docs/spec/spec.md b/docs/spec/spec.md index a018d0dd4..5b9a9deb7 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -219,8 +219,8 @@ When writing the service, following configurations can be provided, # + dispatcherStreamId - The identifier used to distinguish between requests and their corresponding responses in a multiplexing scenario. # + connectionClosureTimeout - Time to wait (in seconds) for the close frame to be received from the remote endpoint # before closing the connection. If the timeout exceeds, then the connection is terminated even though a close frame is -# not received from the remote endpoint. If the value < 0 (e.g., -1), then the connection waits until a close frame is -# received. If the WebSocket frame is received from the remote endpoint within the waiting period, the connection is +# not received from the remote endpoint. If the value is -1, then the connection waits until a close frame is +# received, and any other negative value results in an error. If the WebSocket frame is received from the remote endpoint within the waiting period, the connection is # terminated immediately. public type WSServiceConfig record {| string[] subProtocols = []; diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java index 600d0f5bd..7df39774c 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java @@ -115,7 +115,6 @@ public class WebSocketConstants { public static final int STATUS_CODE_FOR_NO_STATUS_CODE_PRESENT = 1005; public static final int DEFAULT_MAX_FRAME_SIZE = 65536; - public static final int DEFAULT_CONNECTION_CLOSURE_TIMEOUT = 60; // Warning suppression public static final String UNCHECKED = "unchecked"; diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java index e6d54aa20..7b12dcd99 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java @@ -200,6 +200,16 @@ public static int findMaxFrameSize(BMap configs) { } + public static int findTimeoutInSeconds(BMap config, BString key) { + try { + return (int) ((BDecimal) config.get(key)).floatValue(); + } catch (Exception e) { + logger.warn("The value set for " + key + " is not a valid integer. The " + key + " value is set to " + + Integer.MAX_VALUE); + return Integer.MAX_VALUE; + } + } + public static int findTimeoutInSeconds(BMap config, BString key, int defaultValue) { try { int timeout = (int) ((BDecimal) config.get(key)).floatValue(); diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java index f2c7e70cf..7c9fe3252 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java @@ -39,8 +39,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import static io.ballerina.stdlib.websocket.WebSocketConstants.DEFAULT_CONNECTION_CLOSURE_TIMEOUT; - /** * {@code Get} is the GET action implementation of the HTTP Connector. */ @@ -86,7 +84,7 @@ public static Object externClose(Environment env, BObject wsConnection, long sta } public static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { - int timeoutInSecs = DEFAULT_CONNECTION_CLOSURE_TIMEOUT; + int timeoutInSecs = 0; if (bTimeoutInSecs instanceof BDecimal) { timeoutInSecs = (int) ((BDecimal) bTimeoutInSecs).floatValue(); } else if (connectionInfo.getService() instanceof WebSocketServerService webSocketServerService) { diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java index 0ce8703a5..bed348776 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java @@ -32,7 +32,6 @@ import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_DISPATCHER_KEY; import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_VALIDATION_ENABLED; -import static io.ballerina.stdlib.websocket.WebSocketConstants.DEFAULT_CONNECTION_CLOSURE_TIMEOUT; /** * WebSocket service for service dispatching. @@ -59,7 +58,7 @@ private void populateConfigs(String basePath) { idleTimeoutInSeconds = WebSocketUtil.findTimeoutInSeconds(configAnnotation, WebSocketConstants.ANNOTATION_ATTR_IDLE_TIMEOUT, 0); connectionClosureTimeout = WebSocketUtil.findTimeoutInSeconds(configAnnotation, - WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT, DEFAULT_CONNECTION_CLOSURE_TIMEOUT); + WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT); maxFrameSize = WebSocketUtil.findMaxFrameSize(configAnnotation); enableValidation = configAnnotation.getBooleanValue(ANNOTATION_ATTR_VALIDATION_ENABLED); if (configAnnotation.getStringValue(ANNOTATION_ATTR_DISPATCHER_KEY) != null) { From d0a8285a8b7abd37cc68cf3aabb7bb5f19cd008e Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 00:59:08 +0530 Subject: [PATCH 13/27] Add compiler plugin validation for connection closure timeout --- .../WebSocketServiceValidationTest.java | 10 +++++ .../sample_package_64/Ballerina.toml | 4 ++ .../sample_package_64/service.bal | 30 ++++++++++++++ .../websocket/plugin/PluginConstants.java | 4 +- .../WebSocketUpgradeServiceValidatorTask.java | 39 +++++++++++++++++++ .../stdlib/websocket/WebSocketConstants.java | 3 +- .../server/WebSocketServerService.java | 7 ++-- 7 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/Ballerina.toml create mode 100644 compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal diff --git a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/websocket/compiler/WebSocketServiceValidationTest.java b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/websocket/compiler/WebSocketServiceValidationTest.java index e77ff657a..1c50f48e7 100644 --- a/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/websocket/compiler/WebSocketServiceValidationTest.java +++ b/compiler-plugin-tests/src/test/java/io/ballerina/stdlib/websocket/compiler/WebSocketServiceValidationTest.java @@ -572,6 +572,16 @@ public void testRemoteFunctionWithStreamAndCloseFrameReturnTypes() { Assert.assertEquals(diagnosticResult.errorCount(), 0); } + @Test + public void testConnectionClosureTimeoutInTheServiceConfig() { + Package currentPackage = loadPackage("sample_package_64"); + PackageCompilation compilation = currentPackage.getCompilation(); + DiagnosticResult diagnosticResult = compilation.diagnosticResult(); + Assert.assertEquals(diagnosticResult.errorCount(), 1); + Diagnostic diagnostic = (Diagnostic) diagnosticResult.errors().toArray()[0]; + assertDiagnostic(diagnostic, PluginConstants.CompilationErrors.INVALID_CONNECTION_CLOSURE_TIMEOUT); + } + private void assertDiagnostic(Diagnostic diagnostic, PluginConstants.CompilationErrors error) { Assert.assertEquals(diagnostic.diagnosticInfo().code(), error.getErrorCode()); Assert.assertEquals(diagnostic.diagnosticInfo().messageFormat(), diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/Ballerina.toml b/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/Ballerina.toml new file mode 100644 index 000000000..eb158bba7 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/Ballerina.toml @@ -0,0 +1,4 @@ +[package] +org = "websocket_test" +name = "sample_64" +version = "0.1.0" diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal new file mode 100644 index 000000000..468a19a18 --- /dev/null +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal @@ -0,0 +1,30 @@ +// Copyright (c) 2025, WSO2 LLC. (http://www.wso2.com). +// +// WSO2 LLC. licenses this file to you under the Apache License, +// Version 2.0 (the "License"); you may not use this file except +// in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +import ballerina/websocket; + +@websocket:ServiceConfig { + connectionClosureTimeout: -5 +} +service / on new websocket:Listener(9090) { + resource isolated function get .() returns websocket:Service|websocket:UpgradeError { + return new WsService(); + } +} + +service isolated class WsService { + *websocket:Service; +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/PluginConstants.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/PluginConstants.java index 933bef014..35306477c 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/PluginConstants.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/PluginConstants.java @@ -105,7 +105,9 @@ public enum CompilationErrors { CONTRADICTING_RETURN_TYPES("Contradicting return types provided for `{0}` remote function, cannot contain" + " stream type with other types", "WEBSOCKET_119"), DISPATCHER_STREAM_ID_WITHOUT_KEY("The `dispatcherStreamId` annotation is used without `dispatcherKey` " + - "annotation", "WEBSOCKET_120"); + "annotation", "WEBSOCKET_120"), + INVALID_CONNECTION_CLOSURE_TIMEOUT("Invalid connection closure timeout provided for the service", + "WEBSOCKET_121"); private final String error; private final String errorCode; diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index eb1bfc49c..1cd914d10 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -37,6 +37,7 @@ import io.ballerina.compiler.syntax.tree.PositionalArgumentNode; import io.ballerina.compiler.syntax.tree.SeparatedNodeList; import io.ballerina.compiler.syntax.tree.ServiceDeclarationNode; +import io.ballerina.compiler.syntax.tree.SpecificFieldNode; import io.ballerina.compiler.syntax.tree.SyntaxKind; import io.ballerina.projects.plugins.AnalysisTask; import io.ballerina.projects.plugins.SyntaxNodeAnalysisContext; @@ -47,6 +48,7 @@ import java.util.List; import java.util.Optional; +import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT; import static io.ballerina.stdlib.websocket.plugin.PluginConstants.DISPATCHER_ANNOTATION; import static io.ballerina.stdlib.websocket.plugin.PluginConstants.DISPATCHER_STREAM_ID_ANNOTATION; import static io.ballerina.stdlib.websocket.plugin.PluginConstants.ORG_NAME; @@ -74,6 +76,11 @@ public void perform(SyntaxNodeAnalysisContext ctx) { reportDiagnostics(ctx, PluginConstants.CompilationErrors.DISPATCHER_STREAM_ID_WITHOUT_KEY, serviceDeclarationNode.location()); } + Optional timeoutValue = getConnectionClosureTimeoutValue(serviceDeclarationNode, ctx.semanticModel()); + if (timeoutValue.isPresent() && timeoutValue.get() < 0 && timeoutValue.get().intValue() != -1) { + reportDiagnostics(ctx, PluginConstants.CompilationErrors.INVALID_CONNECTION_CLOSURE_TIMEOUT, + serviceDeclarationNode.metadata().get().location()); + } String modulePrefix = Utils.getPrefix(ctx); Optional serviceDeclarationSymbol = ctx.semanticModel().symbol(serviceDeclarationNode); @@ -119,6 +126,38 @@ private boolean getDispatcherConfigAnnotation(ServiceDeclarationNode serviceNode .anyMatch(ann -> isAnnotationPresent(ann, semanticModel, annotationName)); } + private Optional getConnectionClosureTimeoutValue(ServiceDeclarationNode serviceNode, + SemanticModel semanticModel) { + Optional metadata = serviceNode.metadata(); + if (metadata.isEmpty()) { + return Optional.empty(); + } + MetadataNode metaData = metadata.get(); + NodeList annotations = metaData.annotations(); + return annotations.stream() + .filter(ann -> isAnnotationPresent(ann, semanticModel, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) + .map(ann -> getAnnotationValue(ann, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) + .filter(Optional::isPresent) + .map(Optional::get) + .findFirst() + .map(Double::parseDouble); + } + + private Optional getAnnotationValue(AnnotationNode annotation, String annotationName) { + if (annotation.annotValue().isEmpty()) { + return Optional.empty(); + } + return annotation.annotValue().get() + .fields().stream() + .map(field -> (SpecificFieldNode) field) + .filter(field -> field.fieldName().toString().contains(annotationName)) + .map(field -> field.valueExpr().map(Object::toString)) + .filter(Optional::isPresent) + .map(Optional::get) + .map(s -> s.strip().replaceAll("\"", "")) + .findFirst(); + } + private boolean isListenerBelongsToWebSocketModule(TypeSymbol listenerType) { if (listenerType.typeKind() == TypeDescKind.UNION) { return ((UnionTypeSymbol) listenerType).memberTypeDescriptors().stream() diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java index 7df39774c..b06555fa7 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketConstants.java @@ -45,8 +45,7 @@ public class WebSocketConstants { public static final String WEBSOCKET_ANNOTATION_CONFIGURATION = "ServiceConfig"; public static final BString ANNOTATION_ATTR_SUB_PROTOCOLS = StringUtils.fromString("subProtocols"); public static final BString ANNOTATION_ATTR_IDLE_TIMEOUT = StringUtils.fromString("idleTimeout"); - public static final BString ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT = - StringUtils.fromString("connectionClosureTimeout"); + public static final String ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT = "connectionClosureTimeout"; public static final BString ANNOTATION_ATTR_READ_IDLE_TIMEOUT = StringUtils.fromString("readTimeout"); public static final BString ANNOTATION_ATTR_TIMEOUT = StringUtils.fromString("timeout"); public static final BString ANNOTATION_ATTR_MAX_FRAME_SIZE = StringUtils.fromString("maxFrameSize"); diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java index bed348776..d6f2d531f 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/server/WebSocketServerService.java @@ -20,7 +20,6 @@ import io.ballerina.runtime.api.Runtime; import io.ballerina.runtime.api.types.ObjectType; -import io.ballerina.runtime.api.utils.StringUtils; import io.ballerina.runtime.api.utils.TypeUtils; import io.ballerina.runtime.api.values.BMap; import io.ballerina.runtime.api.values.BObject; @@ -30,6 +29,8 @@ import io.ballerina.stdlib.websocket.WebSocketService; import io.ballerina.stdlib.websocket.WebSocketUtil; +import static io.ballerina.runtime.api.utils.StringUtils.fromString; +import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT; import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_DISPATCHER_KEY; import static io.ballerina.stdlib.websocket.WebSocketConstants.ANNOTATION_ATTR_VALIDATION_ENABLED; @@ -58,7 +59,7 @@ private void populateConfigs(String basePath) { idleTimeoutInSeconds = WebSocketUtil.findTimeoutInSeconds(configAnnotation, WebSocketConstants.ANNOTATION_ATTR_IDLE_TIMEOUT, 0); connectionClosureTimeout = WebSocketUtil.findTimeoutInSeconds(configAnnotation, - WebSocketConstants.ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT); + fromString(ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)); maxFrameSize = WebSocketUtil.findMaxFrameSize(configAnnotation); enableValidation = configAnnotation.getBooleanValue(ANNOTATION_ATTR_VALIDATION_ENABLED); if (configAnnotation.getStringValue(ANNOTATION_ATTR_DISPATCHER_KEY) != null) { @@ -73,7 +74,7 @@ private void populateConfigs(String basePath) { @SuppressWarnings(WebSocketConstants.UNCHECKED) private BMap getServiceConfigAnnotation() { ObjectType serviceType = (ObjectType) TypeUtils.getReferredType(TypeUtils.getType(service)); - return (BMap) serviceType.getAnnotation(StringUtils.fromString( + return (BMap) serviceType.getAnnotation(fromString( ModuleUtils.getPackageIdentifier() + ":" + WebSocketConstants.WEBSOCKET_ANNOTATION_CONFIGURATION)); } From ad49678e50744e30d8b85231254ac1ee280df3d6 Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 10:50:57 +0530 Subject: [PATCH 14/27] Remove unreachable exception in findTimeoutInSeconds --- .../java/io/ballerina/stdlib/websocket/WebSocketUtil.java | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java index 7b12dcd99..17c00e036 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java @@ -201,13 +201,7 @@ public static int findMaxFrameSize(BMap configs) { } public static int findTimeoutInSeconds(BMap config, BString key) { - try { - return (int) ((BDecimal) config.get(key)).floatValue(); - } catch (Exception e) { - logger.warn("The value set for " + key + " is not a valid integer. The " + key + " value is set to " + - Integer.MAX_VALUE); - return Integer.MAX_VALUE; - } + return (int) ((BDecimal) config.get(key)).floatValue(); } public static int findTimeoutInSeconds(BMap config, BString key, int defaultValue) { From 1af2fffdc0d21c6cdedf55818a5f9e58fa3a345e Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 11:04:30 +0530 Subject: [PATCH 15/27] Add test for handling negative timeout values in close method of the client --- ballerina/tests/connection_closure_timeout.bal | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ballerina/tests/connection_closure_timeout.bal b/ballerina/tests/connection_closure_timeout.bal index 1148b583f..e9fce7280 100644 --- a/ballerina/tests/connection_closure_timeout.bal +++ b/ballerina/tests/connection_closure_timeout.bal @@ -107,3 +107,15 @@ public function testConnectionClosureTimeoutCallerNegativeTimeout() returns erro string errorMessage = check wsClient2->readMessage(); test:assertEquals(errorMessage, "Invalid timeout value: -10"); } + +@test:Config { + groups: ["connectionClosureTimeout"] +} +public function testConnectionClosureTimeoutNegativeValueInClient() returns error? { + Client wsClient1 = check new ("ws://localhost:22100/"); + Error? close = wsClient1->close(timeout = -10); + test:assertTrue(close is error); + if close is error { + test:assertEquals(close.message(), "Invalid timeout value: -10"); + } +} From bf91804f917de98e0119c79dee2dc378234d7c1a Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 14:48:49 +0530 Subject: [PATCH 16/27] Remove unnecessary variable assignment --- .../plugin/WebSocketUpgradeServiceValidatorTask.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index 1cd914d10..c42a93ce4 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -116,11 +116,10 @@ private boolean isAnnotationPresent(AnnotationNode annotation, SemanticModel sem private boolean getDispatcherConfigAnnotation(ServiceDeclarationNode serviceNode, SemanticModel semanticModel, String annotationName) { - Optional metadata = serviceNode.metadata(); - if (metadata.isEmpty()) { + if (serviceNode.metadata().isEmpty()) { return false; } - MetadataNode metaData = metadata.get(); + MetadataNode metaData = serviceNode.metadata().get(); NodeList annotations = metaData.annotations(); return annotations.stream() .anyMatch(ann -> isAnnotationPresent(ann, semanticModel, annotationName)); @@ -128,12 +127,10 @@ private boolean getDispatcherConfigAnnotation(ServiceDeclarationNode serviceNode private Optional getConnectionClosureTimeoutValue(ServiceDeclarationNode serviceNode, SemanticModel semanticModel) { - Optional metadata = serviceNode.metadata(); - if (metadata.isEmpty()) { + if (serviceNode.metadata().isEmpty()) { return Optional.empty(); } - MetadataNode metaData = metadata.get(); - NodeList annotations = metaData.annotations(); + NodeList annotations = serviceNode.metadata().get().annotations(); return annotations.stream() .filter(ann -> isAnnotationPresent(ann, semanticModel, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) .map(ann -> getAnnotationValue(ann, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) From 81062ca848e80a95a841fc96b91c2c81116e7417 Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 18:47:42 +0530 Subject: [PATCH 17/27] Remove unnecessary parenthesis --- ballerina/websocket_caller.bal | 2 +- ballerina/websocket_sync_client.bal | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ballerina/websocket_caller.bal b/ballerina/websocket_caller.bal index 165060b8c..32bb59ed3 100644 --- a/ballerina/websocket_caller.bal +++ b/ballerina/websocket_caller.bal @@ -98,7 +98,7 @@ public isolated client class Caller { } code = statusCode; } - if (timeout is decimal && timeout < 0d && timeout != -1d) { + if timeout is decimal && timeout < 0d && timeout != -1d { string errorMessage = "Invalid timeout value: " + timeout.toString(); return error Error(errorMessage); } diff --git a/ballerina/websocket_sync_client.bal b/ballerina/websocket_sync_client.bal index f97cdbca9..6e9c40672 100644 --- a/ballerina/websocket_sync_client.bal +++ b/ballerina/websocket_sync_client.bal @@ -120,7 +120,7 @@ public isolated client class Client { } code = statusCode; } - if (timeout < 0d && timeout != -1d) { + if timeout < 0d && timeout != -1d { string errorMessage = "Invalid timeout value: " + timeout.toString(); return error Error(errorMessage); } From ccbaef30e97b90be18d520740a58a7f9071ce990 Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 23:24:56 +0530 Subject: [PATCH 18/27] Refactor isAnnotationPresent method --- .../WebSocketUpgradeServiceValidatorTask.java | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index c42a93ce4..c5d61f048 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -19,7 +19,6 @@ package io.ballerina.stdlib.websocket.plugin; import io.ballerina.compiler.api.SemanticModel; -import io.ballerina.compiler.api.symbols.AnnotationSymbol; import io.ballerina.compiler.api.symbols.ModuleSymbol; import io.ballerina.compiler.api.symbols.ServiceDeclarationSymbol; import io.ballerina.compiler.api.symbols.Symbol; @@ -29,6 +28,7 @@ import io.ballerina.compiler.api.symbols.UnionTypeSymbol; import io.ballerina.compiler.syntax.tree.AnnotationNode; import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; +import io.ballerina.compiler.syntax.tree.IdentifierToken; import io.ballerina.compiler.syntax.tree.MetadataNode; import io.ballerina.compiler.syntax.tree.NamedArgumentNode; import io.ballerina.compiler.syntax.tree.NodeList; @@ -99,19 +99,18 @@ public void perform(SyntaxNodeAnalysisContext ctx) { } } - private boolean isAnnotationPresent(AnnotationNode annotation, SemanticModel semanticModel, - String annotationName) { - Optional symbolOpt = semanticModel.symbol(annotation); - if (symbolOpt.isEmpty()) { - return false; - } - - Symbol symbol = symbolOpt.get(); - if (!(symbol instanceof AnnotationSymbol)) { + private boolean isAnnotationFieldPresent(AnnotationNode annotation, SemanticModel semanticModel, + String annotationName) { + if (annotation.annotValue().isEmpty()) { return false; } - - return annotation.annotValue().toString().contains(annotationName); + return annotation.annotValue().get() + .fields().stream() + .map(s -> ((SpecificFieldNode) s).fieldName()) + .map(s -> (IdentifierToken) s) + .map(semanticModel::symbol).filter(Optional::isPresent).map(Optional::get) + .map(Symbol::getName).filter(Optional::isPresent).map(Optional::get) + .anyMatch(s -> s.equals(annotationName)); } private boolean getDispatcherConfigAnnotation(ServiceDeclarationNode serviceNode, @@ -122,7 +121,7 @@ private boolean getDispatcherConfigAnnotation(ServiceDeclarationNode serviceNode MetadataNode metaData = serviceNode.metadata().get(); NodeList annotations = metaData.annotations(); return annotations.stream() - .anyMatch(ann -> isAnnotationPresent(ann, semanticModel, annotationName)); + .anyMatch(ann -> isAnnotationFieldPresent(ann, semanticModel, annotationName)); } private Optional getConnectionClosureTimeoutValue(ServiceDeclarationNode serviceNode, @@ -132,7 +131,7 @@ private Optional getConnectionClosureTimeoutValue(ServiceDeclarationNode } NodeList annotations = serviceNode.metadata().get().annotations(); return annotations.stream() - .filter(ann -> isAnnotationPresent(ann, semanticModel, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) + .filter(ann -> isAnnotationFieldPresent(ann, semanticModel, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) .map(ann -> getAnnotationValue(ann, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) .filter(Optional::isPresent) .map(Optional::get) From 7be39dacb7dffd4cd82e73af46dd834cf50fec07 Mon Sep 17 00:00:00 2001 From: ayash Date: Thu, 3 Apr 2025 23:35:00 +0530 Subject: [PATCH 19/27] Handle ArithmeticException in findTimeoutInSeconds method --- .../java/io/ballerina/stdlib/websocket/WebSocketUtil.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java index 17c00e036..e642cd227 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java @@ -201,7 +201,13 @@ public static int findMaxFrameSize(BMap configs) { } public static int findTimeoutInSeconds(BMap config, BString key) { - return (int) ((BDecimal) config.get(key)).floatValue(); + try { + return (int) ((BDecimal) config.get(key)).floatValue(); + } catch (ArithmeticException e) { + logger.warn("The value set for {} needs to be less than {} .The {} value is set to {} ", key, + Integer.MAX_VALUE, key, Integer.MAX_VALUE); + return Integer.MAX_VALUE; + } } public static int findTimeoutInSeconds(BMap config, BString key, int defaultValue) { From eead202c91d41ddf7a560aceaba5dd2139e29f1d Mon Sep 17 00:00:00 2001 From: ayash Date: Fri, 4 Apr 2025 16:41:32 +0530 Subject: [PATCH 20/27] Enhance WebSocketUpgradeServiceValidatorTask --- .../sample_package_64/service.bal | 21 ++++++++++++++++ .../WebSocketUpgradeServiceValidatorTask.java | 25 ++++++++++++------- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal b/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal index 468a19a18..51d98e5b7 100644 --- a/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal +++ b/compiler-plugin-tests/src/test/resources/ballerina_sources/sample_package_64/service.bal @@ -28,3 +28,24 @@ service / on new websocket:Listener(9090) { service isolated class WsService { *websocket:Service; } + +// We ignore the compiler validation for below services since value is provided via a variable +decimal connectionClosureTimeout = 5.0; + +@websocket:ServiceConfig { + connectionClosureTimeout: connectionClosureTimeout +} +service / on new websocket:Listener(9090) { + resource isolated function get .() returns websocket:Service|websocket:UpgradeError { + return new WsService(); + } +} + +@websocket:ServiceConfig { + connectionClosureTimeout +} +service / on new websocket:Listener(9090) { + resource isolated function get .() returns websocket:Service|websocket:UpgradeError { + return new WsService(); + } +} diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index c5d61f048..78bf8a005 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -29,6 +29,7 @@ import io.ballerina.compiler.syntax.tree.AnnotationNode; import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; import io.ballerina.compiler.syntax.tree.IdentifierToken; +import io.ballerina.compiler.syntax.tree.MappingFieldNode; import io.ballerina.compiler.syntax.tree.MetadataNode; import io.ballerina.compiler.syntax.tree.NamedArgumentNode; import io.ballerina.compiler.syntax.tree.NodeList; @@ -143,15 +144,21 @@ private Optional getAnnotationValue(AnnotationNode annotation, String an if (annotation.annotValue().isEmpty()) { return Optional.empty(); } - return annotation.annotValue().get() - .fields().stream() - .map(field -> (SpecificFieldNode) field) - .filter(field -> field.fieldName().toString().contains(annotationName)) - .map(field -> field.valueExpr().map(Object::toString)) - .filter(Optional::isPresent) - .map(Optional::get) - .map(s -> s.strip().replaceAll("\"", "")) - .findFirst(); + for (MappingFieldNode field : annotation.annotValue().get().fields()) { + if (field instanceof SpecificFieldNode specificFieldNode) { + if (!specificFieldNode.fieldName().toString().strip().equals(annotationName)){ + continue; + } + if (specificFieldNode.valueExpr().isEmpty()) { + return Optional.empty(); + } + if (specificFieldNode.valueExpr().get().kind() != SyntaxKind.UNARY_EXPRESSION) { + return Optional.empty(); + } + return Optional.of(specificFieldNode.valueExpr().get().toString().strip()); + } + } + return Optional.empty(); } private boolean isListenerBelongsToWebSocketModule(TypeSymbol listenerType) { From 2b1ff8e522d755cea079216f8bdb4aa14f3f0aa8 Mon Sep 17 00:00:00 2001 From: ayash Date: Fri, 4 Apr 2025 17:00:03 +0530 Subject: [PATCH 21/27] Refactor getAnnotationValue method --- .../plugin/WebSocketUpgradeServiceValidatorTask.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index 78bf8a005..c6102522c 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -133,20 +133,25 @@ private Optional getConnectionClosureTimeoutValue(ServiceDeclarationNode NodeList annotations = serviceNode.metadata().get().annotations(); return annotations.stream() .filter(ann -> isAnnotationFieldPresent(ann, semanticModel, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) - .map(ann -> getAnnotationValue(ann, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) + .map(ann -> getAnnotationValue(ann, semanticModel, ANNOTATION_ATTR_CONNECTION_CLOSURE_TIMEOUT)) .filter(Optional::isPresent) .map(Optional::get) .findFirst() .map(Double::parseDouble); } - private Optional getAnnotationValue(AnnotationNode annotation, String annotationName) { + private Optional getAnnotationValue(AnnotationNode annotation, SemanticModel semanticModel, + String annotationName) { if (annotation.annotValue().isEmpty()) { return Optional.empty(); } for (MappingFieldNode field : annotation.annotValue().get().fields()) { if (field instanceof SpecificFieldNode specificFieldNode) { - if (!specificFieldNode.fieldName().toString().strip().equals(annotationName)){ + Optional symbol = semanticModel.symbol(specificFieldNode); + if (symbol.isEmpty()) { + continue; + } + if (symbol.get().getName().isEmpty() || !annotationName.equals(symbol.get().getName().get())) { continue; } if (specificFieldNode.valueExpr().isEmpty()) { From 2e2dfb1a6484287b51656a492f3568d368692523 Mon Sep 17 00:00:00 2001 From: ayash Date: Fri, 4 Apr 2025 21:11:27 +0530 Subject: [PATCH 22/27] Improve timeout handling in WebSocket close operations and add tests for invalid timeout values --- ballerina/tests/connection_closure_timeout.bal | 16 ++++++++++++++-- .../actions/websocketconnector/Close.java | 18 +++++++++++------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/ballerina/tests/connection_closure_timeout.bal b/ballerina/tests/connection_closure_timeout.bal index e9fce7280..ec3fe4dde 100644 --- a/ballerina/tests/connection_closure_timeout.bal +++ b/ballerina/tests/connection_closure_timeout.bal @@ -113,9 +113,21 @@ public function testConnectionClosureTimeoutCallerNegativeTimeout() returns erro } public function testConnectionClosureTimeoutNegativeValueInClient() returns error? { Client wsClient1 = check new ("ws://localhost:22100/"); - Error? close = wsClient1->close(timeout = -10); + Error? close = wsClient1->close(timeout = -20); test:assertTrue(close is error); if close is error { - test:assertEquals(close.message(), "Invalid timeout value: -10"); + test:assertEquals(close.message(), "Invalid timeout value: -20"); + } +} + +@test:Config { + groups: ["connectionClosureTimeout","test"] +} +public function testInvalidConnectionClosureTimeoutValue() returns error? { + Client wsClient1 = check new ("ws://localhost:22100/"); + Error? close = wsClient1->close(timeout = 200000000000000000); + test:assertTrue(close is error); + if close is error { + test:assertEquals(close.message(), "Error: Invalid timeout value: 200000000000000000"); } } diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java index 7c9fe3252..d4624b0bd 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/actions/websocketconnector/Close.java @@ -53,8 +53,8 @@ public static Object externClose(Environment env, BObject wsConnection, long sta .getNativeData(WebSocketConstants.NATIVE_DATA_WEBSOCKET_CONNECTION_INFO); WebSocketObservabilityUtil.observeResourceInvocation(env, connectionInfo, WebSocketConstants.RESOURCE_NAME_CLOSE); - int timeoutInSecs = getConnectionClosureTimeout(bTimeoutInSecs, connectionInfo); try { + int timeoutInSecs = getConnectionClosureTimeout(bTimeoutInSecs, connectionInfo); CountDownLatch countDownLatch = new CountDownLatch(1); List errors = new ArrayList<>(1); ChannelFuture closeFuture = initiateConnectionClosure(errors, (int) statusCode, reason.getValue(), @@ -84,13 +84,17 @@ public static Object externClose(Environment env, BObject wsConnection, long sta } public static int getConnectionClosureTimeout(Object bTimeoutInSecs, WebSocketConnectionInfo connectionInfo) { - int timeoutInSecs = 0; - if (bTimeoutInSecs instanceof BDecimal) { - timeoutInSecs = (int) ((BDecimal) bTimeoutInSecs).floatValue(); - } else if (connectionInfo.getService() instanceof WebSocketServerService webSocketServerService) { - timeoutInSecs = webSocketServerService.getConnectionClosureTimeout(); + try { + int timeoutInSecs = 0; + if (bTimeoutInSecs instanceof BDecimal) { + timeoutInSecs = Integer.parseInt(bTimeoutInSecs.toString()); + } else if (connectionInfo.getService() instanceof WebSocketServerService webSocketServerService) { + timeoutInSecs = webSocketServerService.getConnectionClosureTimeout(); + } + return timeoutInSecs; + } catch (Exception e) { + throw new RuntimeException("Invalid timeout value: " + bTimeoutInSecs, e); } - return timeoutInSecs; } public static ChannelFuture initiateConnectionClosure(List errors, int statusCode, String reason, From 648bb8f513eb93bb5b5ea51da3598a7a5572489c Mon Sep 17 00:00:00 2001 From: ayash Date: Sat, 5 Apr 2025 11:09:31 +0530 Subject: [PATCH 23/27] Refactor findTimeoutInSeconds method to handle exceptions --- .../ballerina/stdlib/websocket/WebSocketUtil.java | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java index e642cd227..393b99777 100644 --- a/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java +++ b/native/src/main/java/io/ballerina/stdlib/websocket/WebSocketUtil.java @@ -201,13 +201,17 @@ public static int findMaxFrameSize(BMap configs) { } public static int findTimeoutInSeconds(BMap config, BString key) { + String value = config.get(key).toString(); + int timeout; try { - return (int) ((BDecimal) config.get(key)).floatValue(); - } catch (ArithmeticException e) { - logger.warn("The value set for {} needs to be less than {} .The {} value is set to {} ", key, - Integer.MAX_VALUE, key, Integer.MAX_VALUE); - return Integer.MAX_VALUE; + timeout = Integer.parseInt(value); + } catch (NumberFormatException e) { + throw WebSocketUtil.createErrorByType(new Exception("Invalid timeout value: " + value)); + } + if (timeout < 0 && timeout != -1) { + throw WebSocketUtil.createErrorByType(new Exception("Invalid timeout value: " + value)); } + return timeout; } public static int findTimeoutInSeconds(BMap config, BString key, int defaultValue) { From b7f3a736930201ca28e04a7686219c02887ef740 Mon Sep 17 00:00:00 2001 From: ayash Date: Sat, 5 Apr 2025 11:11:28 +0530 Subject: [PATCH 24/27] Refactor WebSocketUpgradeServiceValidatorTask to use SyntaxKind for field type checking --- .../websocket/plugin/WebSocketUpgradeServiceValidatorTask.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index c6102522c..fd8e91f52 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -146,7 +146,8 @@ private Optional getAnnotationValue(AnnotationNode annotation, SemanticM return Optional.empty(); } for (MappingFieldNode field : annotation.annotValue().get().fields()) { - if (field instanceof SpecificFieldNode specificFieldNode) { + if (field.kind().equals(SyntaxKind.SPECIFIC_FIELD)) { + SpecificFieldNode specificFieldNode = (SpecificFieldNode) field; Optional symbol = semanticModel.symbol(specificFieldNode); if (symbol.isEmpty()) { continue; From 47f0ef3d7385d692d35933cb56786f0376d150f0 Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 7 Apr 2025 17:23:14 +0530 Subject: [PATCH 25/27] Refactor getAnnotationValue function to use equal symbol --- .../websocket/plugin/WebSocketUpgradeServiceValidatorTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index fd8e91f52..f1f86becf 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -146,7 +146,7 @@ private Optional getAnnotationValue(AnnotationNode annotation, SemanticM return Optional.empty(); } for (MappingFieldNode field : annotation.annotValue().get().fields()) { - if (field.kind().equals(SyntaxKind.SPECIFIC_FIELD)) { + if (field.kind() == SyntaxKind.SPECIFIC_FIELD) { SpecificFieldNode specificFieldNode = (SpecificFieldNode) field; Optional symbol = semanticModel.symbol(specificFieldNode); if (symbol.isEmpty()) { From a908f5c48f030d35f23925363138876ff05680b5 Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 7 Apr 2025 18:13:28 +0530 Subject: [PATCH 26/27] Add note on connectionClosureTimeout validation in WSServiceConfig documentation --- docs/spec/spec.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/spec/spec.md b/docs/spec/spec.md index 5b9a9deb7..f2a76f10e 100644 --- a/docs/spec/spec.md +++ b/docs/spec/spec.md @@ -234,6 +234,8 @@ public type WSServiceConfig record {| |}; ``` +> **Note:** The `connectionClosureTimeout` is validated at compile-time for literal values and at runtime for non-literal values such as variables. + ### 3.2. [WebSocket Service](#32-websocket-service) Once the WebSocket upgrade is accepted by the UpgradeService, it returns a `websocket:Service`. This service has a fixed set of remote methods that do not have any configs. Receiving messages will get dispatched to the relevant remote method. Each remote method is explained below. From 89e8d2b31fd91c6ea5d221130ffeb501dda57e5b Mon Sep 17 00:00:00 2001 From: ayash Date: Mon, 7 Apr 2025 18:19:14 +0530 Subject: [PATCH 27/27] Refactor isAnnotationFieldPresent to use syntax kind checking --- .../WebSocketUpgradeServiceValidatorTask.java | 27 +++++++++++++------ 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java index f1f86becf..285d436b1 100644 --- a/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java +++ b/compiler-plugin/src/main/java/io/ballerina/stdlib/websocket/plugin/WebSocketUpgradeServiceValidatorTask.java @@ -28,10 +28,10 @@ import io.ballerina.compiler.api.symbols.UnionTypeSymbol; import io.ballerina.compiler.syntax.tree.AnnotationNode; import io.ballerina.compiler.syntax.tree.FunctionArgumentNode; -import io.ballerina.compiler.syntax.tree.IdentifierToken; import io.ballerina.compiler.syntax.tree.MappingFieldNode; import io.ballerina.compiler.syntax.tree.MetadataNode; import io.ballerina.compiler.syntax.tree.NamedArgumentNode; +import io.ballerina.compiler.syntax.tree.Node; import io.ballerina.compiler.syntax.tree.NodeList; import io.ballerina.compiler.syntax.tree.NodeLocation; import io.ballerina.compiler.syntax.tree.ParenthesizedArgList; @@ -105,13 +105,24 @@ private boolean isAnnotationFieldPresent(AnnotationNode annotation, SemanticMode if (annotation.annotValue().isEmpty()) { return false; } - return annotation.annotValue().get() - .fields().stream() - .map(s -> ((SpecificFieldNode) s).fieldName()) - .map(s -> (IdentifierToken) s) - .map(semanticModel::symbol).filter(Optional::isPresent).map(Optional::get) - .map(Symbol::getName).filter(Optional::isPresent).map(Optional::get) - .anyMatch(s -> s.equals(annotationName)); + for (MappingFieldNode field : annotation.annotValue().get().fields()) { + if (field.kind() != SyntaxKind.SPECIFIC_FIELD) { + continue; + } + Node fieldNameNode = ((SpecificFieldNode) field).fieldName(); + if (fieldNameNode.kind() != SyntaxKind.IDENTIFIER_TOKEN) { + continue; + } + Optional symbol = semanticModel.symbol(fieldNameNode); + if (symbol.isEmpty()) { + continue; + } + if (symbol.get().getName().isEmpty() || !annotationName.equals(symbol.get().getName().get())) { + continue; + } + return true; + } + return false; } private boolean getDispatcherConfigAnnotation(ServiceDeclarationNode serviceNode,