[grid] limit the number of websocket connections per session by joerg1985 · Pull Request #14410 · SeleniumHQ/selenium

joerg1985 · 2024-08-19T18:48:41Z

User description

Description

This change will add a limit of 10 websocket connections per session.
The NodeOptions / NodeFlags do allow to overwrite this default value.

Motivation and Context

Usually a client should only open a small number of websocket connections per session.
But there is no limit so a single session can block a lot of resources when connecting again and again.

@diemol this is the PR related to my email and 5bac479 does at least ensure as soon as the client closes the websocket connection, the connection between the hub and the node / the node and the driver is released.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

I have read the contributing document.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

PR Type

enhancement, tests

Description

Introduced a limit on the number of websocket connections per session to improve resource management.
Added a new POST endpoint to check and enforce connection limits in the Node class.
Implemented connection limit logic in various node types including LocalNode, OneShotNode, and RemoteNode.
Updated configuration options to allow setting a connection limit per session.
Enhanced session management by adding connection counters and resetting them appropriately.
Added test stubs to support future testing of connection limit logic.

Changes walkthrough 📝

Relevant files

Enhancement

7 files

Node.java `Add websocket connection limit management in Node` java/src/org/openqa/selenium/grid/node/Node.java Added a new POST endpoint to check websocket connection limits per session. Introduced `tryAcquireConnection` method to manage connection limits.	+11/-0
ProxyNodeWebsockets.java `Implement websocket connection limit check in ProxyNodeWebsockets` java/src/org/openqa/selenium/grid/node/ProxyNodeWebsockets.java Implemented connection limit check before allowing new websocket connections. Added logging for excessive websocket connections.	+7/-0
TryAcquireConnection.java `Create TryAcquireConnection class for connection limit handling` java/src/org/openqa/selenium/grid/node/TryAcquireConnection.java Created `TryAcquireConnection` class to handle connection limit requests. Implemented HTTP handler for connection limit checks.	+45/-0
OneShotNode.java `Integrate connection limit logic in OneShotNode` java/src/org/openqa/selenium/grid/node/k8s/OneShotNode.java Integrated connection limit logic in OneShotNode. Added atomic counter for connection tracking.	+13/-2
LocalNode.java `Implement connection limit logic in LocalNode` java/src/org/openqa/selenium/grid/node/local/LocalNode.java Implemented connection limit logic in LocalNode. Added atomic counter for managing connections.	+31/-2
SessionSlot.java `Add and manage connection counter in SessionSlot` java/src/org/openqa/selenium/grid/node/local/SessionSlot.java Added connection counter to SessionSlot. Reset connection counter when session stops.	+9/-0
RemoteNode.java `Implement remote connection limit check in RemoteNode` java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java Implemented remote connection limit check in RemoteNode.	+12/-0

Configuration changes

3 files

NodeFlags.java `Add connection limit per session parameter in NodeFlags` java/src/org/openqa/selenium/grid/node/config/NodeFlags.java Added a new parameter for connection limit per session. Updated configuration to include connection limit settings.	+9/-0
NodeOptions.java `Define and retrieve default connection limit in NodeOptions` java/src/org/openqa/selenium/grid/node/config/NodeOptions.java Defined default connection limit for sessions. Added method to retrieve connection limit from configuration.	+10/-0
LocalNodeFactory.java `Configure LocalNode with connection limit settings` java/src/org/openqa/selenium/grid/node/local/LocalNodeFactory.java Configured LocalNode to use connection limit settings.	+2/-1

Tests

1 files

AddingNodesTest.java `Add test stub for connection limit logic in AddingNodesTest` java/test/org/openqa/selenium/grid/distributor/AddingNodesTest.java Added test stub for connection limit logic in AddingNodesTest.	+5/-0

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

qodo-code-review · 2024-08-19T18:49:16Z

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Key issues to review Possible Race Condition The `tryAcquireConnection` method uses `getAndIncrement()` on the connection counter, which may lead to a race condition if the limit is reached exactly. Possible Race Condition Similar to the LocalNode issue, the `tryAcquireConnection` method uses `getAndIncrement()` which may lead to a race condition.

qodo-code-review · 2024-08-19T18:50:00Z

PR Code Suggestions ✨

Category	Suggestion	Score
Enhancement	Implement a method to release connections and improve the connection acquisition logic Implement a method to release connections when they are no longer needed, to prevent the connection counter from continuously increasing and potentially reaching the limit even after connections are closed. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [581-597] @Override public boolean tryAcquireConnection(SessionId id) throws NoSuchSessionException { SessionSlot slot = currentSessions.getIfPresent(id); if (slot == null) { return false; } if (connectionLimitPerSession == -1) { // no limit return true; } AtomicLong counter = slot.getConnectionCounter(); - return connectionLimitPerSession > counter.getAndIncrement(); + long currentCount = counter.get(); + while (currentCount < connectionLimitPerSession) { + if (counter.compareAndSet(currentCount, currentCount + 1)) { + return true; + } + currentCount = counter.get(); + } + return false; } +public void releaseConnection(SessionId id) { + SessionSlot slot = currentSessions.getIfPresent(id); + if (slot != null) { + AtomicLong counter = slot.getConnectionCounter(); + counter.updateAndGet(count -> Math.max(0, count - 1)); + } +} + Apply this suggestion Suggestion importance[1-10]: 8 Why: Adding a method to release connections is a significant improvement that prevents the counter from increasing indefinitely, which could lead to reaching the limit even after connections are closed.	8
Performance	Use a more efficient data structure for managing connection counters across multiple sessions Consider using a more efficient data structure for managing connection counters, such as a ConcurrentHashMap<SessionId, AtomicLong>, instead of storing the counter in the SessionSlot. This would allow for better scalability and easier management of connection limits across multiple sessions. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [581-597] +private final ConcurrentHashMap<SessionId, AtomicLong> connectionCounters = new ConcurrentHashMap<>(); + @Override public boolean tryAcquireConnection(SessionId id) throws NoSuchSessionException { SessionSlot slot = currentSessions.getIfPresent(id); if (slot == null) { return false; } if (connectionLimitPerSession == -1) { // no limit return true; } - AtomicLong counter = slot.getConnectionCounter(); + AtomicLong counter = connectionCounters.computeIfAbsent(id, k -> new AtomicLong()); return connectionLimitPerSession > counter.getAndIncrement(); } Apply this suggestion Suggestion importance[1-10]: 7 Why: The suggestion to use a ConcurrentHashMap for managing connection counters is valid as it can improve scalability and management across sessions. However, the current implementation using AtomicLong in SessionSlot is already efficient for the existing use case.	7
Performance	Add a configurable delay when the connection limit is reached to prevent rapid repeated attempts Consider adding a configurable delay or backoff mechanism when the connection limit is reached, to prevent rapid repeated attempts that could overload the system. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [581-597] +private final Duration backoffDelay = Duration.ofMillis(100); + @Override public boolean tryAcquireConnection(SessionId id) throws NoSuchSessionException { SessionSlot slot = currentSessions.getIfPresent(id); if (slot == null) { return false; } if (connectionLimitPerSession == -1) { // no limit return true; } AtomicLong counter = slot.getConnectionCounter(); + long currentCount = counter.get(); - return connectionLimitPerSession > counter.getAndIncrement(); + while (currentCount < connectionLimitPerSession) { + if (counter.compareAndSet(currentCount, currentCount + 1)) { + return true; + } + currentCount = counter.get(); + } + + // Connection limit reached, apply backoff + try { + Thread.sleep(backoffDelay.toMillis()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + + return false; } Apply this suggestion Suggestion importance[1-10]: 7 Why: Introducing a backoff mechanism can prevent system overload from rapid repeated attempts, enhancing performance and stability, but it is not essential for the current implementation.	7
Maintainability	Add logging statements to track connection acquisition and release Add logging statements to track connection acquisition and release, which will help with debugging and monitoring the connection limit feature. java/src/org/openqa/selenium/grid/node/local/LocalNode.java [581-597] +private static final Logger LOG = Logger.getLogger(LocalNode.class.getName()); + @Override public boolean tryAcquireConnection(SessionId id) throws NoSuchSessionException { SessionSlot slot = currentSessions.getIfPresent(id); if (slot == null) { + LOG.fine("No session found for id: " + id); return false; } if (connectionLimitPerSession == -1) { - // no limit + LOG.fine("No connection limit for session: " + id); return true; } AtomicLong counter = slot.getConnectionCounter(); + long newCount = counter.incrementAndGet(); - return connectionLimitPerSession > counter.getAndIncrement(); + if (newCount <= connectionLimitPerSession) { + LOG.fine("Connection acquired for session " + id + ". Current count: " + newCount); + return true; + } else { + counter.decrementAndGet(); + LOG.warning("Connection limit reached for session " + id + ". Current count: " + (newCount - 1)); + return false; + } } Apply this suggestion Suggestion importance[1-10]: 6 Why: Adding logging statements can aid in debugging and monitoring, which is beneficial for maintainability, but it is not a critical change.	6

qodo-code-review · 2024-08-19T19:01:39Z

CI Failure Feedback 🧐

(Checks updated until commit `0b5e1c8`)

Action: Ruby / Remote Tests (edge, windows) / Remote Tests (edge, windows)
Failed stage: Run Bazel [❌]
Failed test name: Selenium::WebDriver::Remote::Driver errors when not set
Failure summary: The action failed because the test `Selenium::WebDriver::Remote::Driver errors when not set` did not pass. The failure was due to: The test expected a `Selenium::WebDriver::Error::WebDriverError` with the message "You must enable downloads in order to work with downloadable files." Instead, it received a `Selenium::WebDriver::Error::UnknownError` indicating "Cannot find downloads file system for session id."
Relevant error logs: 1: ##[group]Operating System 2: Microsoft Windows Server 2022 ... 681: �[32m[2,921 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 29s local, disk-cache ... (4 actions, 2 running) 682: �[32m[2,932 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 30s local, disk-cache ... (4 actions, 3 running) 683: �[32m[2,933 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 33s local, disk-cache ... (4 actions, 2 running) 684: �[32m[2,940 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 35s local, disk-cache ... (4 actions, 2 running) 685: �[32m[2,942 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 36s local, disk-cache ... (4 actions, 2 running) 686: �[32m[2,957 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 37s local, disk-cache ... (4 actions, 2 running) 687: �[32m[2,965 / 3,098]�[0m Extracting npm package @mui/icons-material@5.15.18_796719057; 38s local, disk-cache ... (4 actions, 3 running) 688: �[32mINFO: �[0mFrom Building java/src/org/openqa/selenium/remote/libapi-class.jar (71 source files): 689: java\src\org\openqa\selenium\remote\ErrorHandler.java:46: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 690: private final ErrorCodes errorCodes; 691: ^ 692: java\src\org\openqa\selenium\remote\ErrorHandler.java:60: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 693: this.errorCodes = new ErrorCodes(); 694: ^ 695: java\src\org\openqa\selenium\remote\ErrorHandler.java:68: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 696: public ErrorHandler(ErrorCodes codes, boolean includeServerErrors) { 697: ^ 698: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 699: ErrorCodes errorCodes = new ErrorCodes(); 700: ^ 701: java\src\org\openqa\selenium\remote\Response.java:97: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 702: ErrorCodes errorCodes = new ErrorCodes(); 703: ^ 704: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:181: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 705: response.setStatus(ErrorCodes.SUCCESS); 706: ^ 707: java\src\org\openqa\selenium\remote\ProtocolHandshake.java:182: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 708: response.setState(ErrorCodes.SUCCESS_STRING); 709: ^ 710: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:53: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 711: new ErrorCodes().toStatus((String) rawError, Optional.of(tuple.getStatusCode()))); 712: ^ 713: java\src\org\openqa\selenium\remote\W3CHandshakeResponse.java:56: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 714: new ErrorCodes().getExceptionType((String) rawError); 715: ^ 716: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:44: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 717: private final ErrorCodes errorCodes = new ErrorCodes(); 718: ^ 719: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:44: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 720: private final ErrorCodes errorCodes = new ErrorCodes(); 721: ^ 722: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:55: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 723: int status = response.getStatus() == ErrorCodes.SUCCESS ? HTTP_OK : HTTP_INTERNAL_ERROR; 724: ^ 725: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:101: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 726: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 727: ^ 728: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:103: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 729: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 730: ^ 731: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:117: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 732: response.setStatus(ErrorCodes.SUCCESS); 733: ^ 734: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:118: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 735: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 736: ^ 737: java\src\org\openqa\selenium\remote\codec\AbstractHttpResponseCodec.java:124: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 738: response.setState(errorCodes.toState(ErrorCodes.SUCCESS)); 739: ^ 740: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:70: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 741: private final ErrorCodes errorCodes = new ErrorCodes(); 742: ^ 743: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:70: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 744: private final ErrorCodes errorCodes = new ErrorCodes(); 745: ^ 746: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:93: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 747: response.setStatus(ErrorCodes.UNKNOWN_COMMAND); 748: ^ 749: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:98: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 750: response.setStatus(ErrorCodes.UNHANDLED_ERROR); 751: ^ 752: java\src\org\openqa\selenium\remote\codec\w3c\W3CHttpResponseCodec.java:145: warning: [removal] ErrorCodes in org.openqa.selenium.remote has been deprecated and marked for removal 753: response.setStatus(ErrorCodes.SUCCESS); ... 915: �[32m[3,098 / 3,122]�[0m Testing //rb/spec/integration/selenium/webdriver:manager-edge-remote; 8s disk-cache ... (4 actions, 0 running) 916: �[32m[3,098 / 3,122]�[0m Testing //rb/spec/integration/selenium/webdriver:manager-edge-remote; 36s disk-cache ... (4 actions, 0 running) 917: �[32m[3,098 / 3,122]�[0m Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 1s local, disk-cache ... (4 actions, 1 running) 918: �[32m[3,098 / 3,122]�[0m Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 2s local, disk-cache ... (4 actions, 1 running) 919: �[32m[3,098 / 3,122]�[0m Testing //rb/spec/integration/selenium/webdriver:element-edge-remote; 62s local, disk-cache ... (4 actions, 1 running) 920: �[32m[3,099 / 3,122]�[0m 1 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:shadow_root-edge-remote; 63s ... (4 actions, 1 running) 921: �[32m[3,099 / 3,122]�[0m 1 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:shadow_root-edge-remote; 73s ... (4 actions, 1 running) 922: �[32m[3,099 / 3,122]�[0m 1 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:shadow_root-edge-remote; 76s ... (4 actions, 1 running) 923: �[32m[3,099 / 3,122]�[0m 1 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 79s ... (4 actions, 2 running) 924: �[32m[3,100 / 3,122]�[0m 2 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 80s ... (4 actions, 1 running) 925: �[32m[3,100 / 3,122]�[0m 2 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 90s ... (4 actions, 1 running) 926: �[32m[3,100 / 3,122]�[0m 2 / 29 tests;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:error-edge-remote; 94s ... (4 actions, 1 running) ... 993: �[32m[3,115 / 3,122]�[0m 17 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 167s local, disk-cache ... (4 actions, 2 running) 994: �[32m[3,115 / 3,122]�[0m 17 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 170s local, disk-cache ... (4 actions, 2 running) 995: �[32m[3,115 / 3,122]�[0m 17 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 172s local, disk-cache ... (4 actions, 2 running) 996: �[32m[3,116 / 3,122]�[0m 18 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 174s local, disk-cache ... (4 actions, 1 running) 997: �[32m[3,116 / 3,122]�[0m 18 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 184s local, disk-cache ... (4 actions, 1 running) 998: �[32m[3,116 / 3,122]�[0m 18 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 187s local, disk-cache ... (4 actions, 1 running) 999: �[32m[3,116 / 3,122]�[0m 18 / 29 tests;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote; 200s local, disk-cache ... (4 actions, 2 running) 1000: �[31m�[1mFAIL: �[0m//rb/spec/integration/selenium/webdriver/remote:driver-edge-remote (see D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/remote/driver-edge-remote/test.log) 1001: �[31m�[1mFAILED: �[0m//rb/spec/integration/selenium/webdriver/remote:driver-edge-remote (Summary) ... 1016: Selenium::WebDriver::Remote::Driver 1017: exposes session_id 1018: exposes remote status 1019: Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead 1020: uses a default file detector 1021: lists downloads 1022: downloads a file 1023: deletes downloadable files 1024: errors when not set (FAILED - 1) 1025: Failures: 1026: 1) Selenium::WebDriver::Remote::Driver errors when not set 1027: Failure/Error: 1028: expect { 1029: driver.downloadable_files 1030: }.to raise_exception(Error::WebDriverError, 1031: 'You must enable downloads in order to work with downloadable files.') 1032: expected Selenium::WebDriver::Error::WebDriverError with "You must enable downloads in order to work with downloadable files.", got #<Selenium::WebDriver::Error::UnknownError: Cannot find downloads file system for session id: fb43ba6... os.arch: 'amd64', os.version: '10.0', java.version: '17.0.11' 1033: Driver info: driver.version: unknown> with backtrace: 1034: # ./rb/lib/selenium/webdriver/remote/response.rb:63:in `add_cause' 1035: # ./rb/lib/selenium/webdriver/remote/response.rb:41:in `error' ... 1041: # ./rb/lib/selenium/webdriver/remote/http/common.rb:67:in `call' 1042: # ./rb/lib/selenium/webdriver/remote/bridge.rb:685:in `execute' 1043: # ./rb/lib/selenium/webdriver/remote/features.rb:62:in `downloadable_files' 1044: # ./rb/lib/selenium/webdriver/common/driver_extensions/has_file_downloads.rb:27:in `downloadable_files' 1045: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:87:in `block (3 levels) in <module:Remote>' 1046: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:86:in `block (2 levels) in <module:Remote>' 1047: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:86:in `block (2 levels) in <module:Remote>' 1048: Finished in 22.11 seconds (files took 0.70913 seconds to load) 1049: 7 examples, 1 failure 1050: Failed examples: 1051: rspec ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:84 # Selenium::WebDriver::Remote::Driver errors when not set ... 1063: Selenium::WebDriver::Remote::Driver 1064: exposes session_id 1065: exposes remote status 1066: Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead 1067: uses a default file detector 1068: lists downloads 1069: downloads a file 1070: deletes downloadable files 1071: errors when not set (FAILED - 1) 1072: Failures: 1073: 1) Selenium::WebDriver::Remote::Driver errors when not set 1074: Failure/Error: 1075: expect { 1076: driver.downloadable_files 1077: }.to raise_exception(Error::WebDriverError, 1078: 'You must enable downloads in order to work with downloadable files.') 1079: expected Selenium::WebDriver::Error::WebDriverError with "You must enable downloads in order to work with downloadable files.", got #<Selenium::WebDriver::Error::UnknownError: Cannot find downloads file system for session id: 7190140... os.arch: 'amd64', os.version: '10.0', java.version: '17.0.11' 1080: Driver info: driver.version: unknown> with backtrace: 1081: # ./rb/lib/selenium/webdriver/remote/response.rb:63:in `add_cause' 1082: # ./rb/lib/selenium/webdriver/remote/response.rb:41:in `error' ... 1088: # ./rb/lib/selenium/webdriver/remote/http/common.rb:67:in `call' 1089: # ./rb/lib/selenium/webdriver/remote/bridge.rb:685:in `execute' 1090: # ./rb/lib/selenium/webdriver/remote/features.rb:62:in `downloadable_files' 1091: # ./rb/lib/selenium/webdriver/common/driver_extensions/has_file_downloads.rb:27:in `downloadable_files' 1092: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:87:in `block (3 levels) in <module:Remote>' 1093: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:86:in `block (2 levels) in <module:Remote>' 1094: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:86:in `block (2 levels) in <module:Remote>' 1095: Finished in 20.59 seconds (files took 0.89474 seconds to load) 1096: 7 examples, 1 failure 1097: Failed examples: 1098: rspec ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:84 # Selenium::WebDriver::Remote::Driver errors when not set ... 1110: Selenium::WebDriver::Remote::Driver 1111: exposes session_id 1112: exposes remote status 1113: Request#[] is deprecated and will be removed in a future version of Rack. Please use request.params[] instead 1114: uses a default file detector 1115: lists downloads 1116: downloads a file 1117: deletes downloadable files 1118: errors when not set (FAILED - 1) 1119: Failures: 1120: 1) Selenium::WebDriver::Remote::Driver errors when not set 1121: Failure/Error: 1122: expect { 1123: driver.downloadable_files 1124: }.to raise_exception(Error::WebDriverError, 1125: 'You must enable downloads in order to work with downloadable files.') 1126: expected Selenium::WebDriver::Error::WebDriverError with "You must enable downloads in order to work with downloadable files.", got #<Selenium::WebDriver::Error::UnknownError: Cannot find downloads file system for session id: a509a4a... os.arch: 'amd64', os.version: '10.0', java.version: '17.0.11' 1127: Driver info: driver.version: unknown> with backtrace: 1128: # ./rb/lib/selenium/webdriver/remote/response.rb:63:in `add_cause' 1129: # ./rb/lib/selenium/webdriver/remote/response.rb:41:in `error' ... 1135: # ./rb/lib/selenium/webdriver/remote/http/common.rb:67:in `call' 1136: # ./rb/lib/selenium/webdriver/remote/bridge.rb:685:in `execute' 1137: # ./rb/lib/selenium/webdriver/remote/features.rb:62:in `downloadable_files' 1138: # ./rb/lib/selenium/webdriver/common/driver_extensions/has_file_downloads.rb:27:in `downloadable_files' 1139: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:87:in `block (3 levels) in <module:Remote>' 1140: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:86:in `block (2 levels) in <module:Remote>' 1141: # ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:86:in `block (2 levels) in <module:Remote>' 1142: Finished in 21.2 seconds (files took 0.66158 seconds to load) 1143: 7 examples, 1 failure 1144: Failed examples: 1145: rspec ./rb/spec/integration/selenium/webdriver/remote/driver_spec.rb:84 # Selenium::WebDriver::Remote::Driver errors when not set 1146: ================================================================================ 1147: �[32m[3,117 / 3,122]�[0m 19 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:zipper-edge-remote; 31s ... (4 actions, 1 running) 1148: �[32m[3,117 / 3,122]�[0m 19 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:zipper-edge-remote; 42s ... (4 actions, 1 running) 1149: �[32m[3,117 / 3,122]�[0m 19 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:zipper-edge-remote; 44s ... (4 actions, 1 running) 1150: �[32m[3,117 / 3,122]�[0m 19 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:storage-edge-remote; 47s ... (4 actions, 2 running) 1151: �[32m[3,118 / 3,122]�[0m 20 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:storage-edge-remote; 48s ... (4 actions, 1 running) 1152: �[32m[3,118 / 3,122]�[0m 20 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:storage-edge-remote; 58s ... (4 actions, 1 running) 1153: �[32m[3,118 / 3,122]�[0m 20 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:storage-edge-remote; 61s ... (4 actions, 1 running) 1154: �[32m[3,118 / 3,122]�[0m 20 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/remote:element-edge-remote; 34s ... (4 actions, 2 running) 1155: �[32m[3,119 / 3,122]�[0m 21 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/remote:element-edge-remote; 36s ... (3 actions, 1 running) 1156: �[32m[3,119 / 3,122]�[0m 21 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/remote:element-edge-remote; 46s ... (3 actions, 1 running) 1157: �[32m[3,119 / 3,122]�[0m 21 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:timeout-edge-remote; 18s ... (3 actions, 2 running) 1158: �[32m[3,120 / 3,122]�[0m 22 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:timeout-edge-remote; 20s ... (2 actions, 1 running) 1159: �[32m[3,120 / 3,122]�[0m 22 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:timeout-edge-remote; 30s ... (2 actions, 1 running) 1160: �[32m[3,120 / 3,122]�[0m 22 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/remote:element-edge-remote; 16s local, disk-cache ... (2 actions running) 1161: �[32m[3,121 / 3,122]�[0m 23 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:timeout-edge-remote; 1s local, disk-cache 1162: �[32m[3,121 / 3,122]�[0m 23 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:timeout-edge-remote; 11s local, disk-cache 1163: �[32m[3,121 / 3,122]�[0m 23 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:timeout-edge-remote; 23s local, disk-cache 1164: �[32m[3,122 / 3,123]�[0m 24 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote; 0s disk-cache 1165: �[32m[3,122 / 3,123]�[0m 24 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote 1166: �[32m[3,122 / 3,123]�[0m 24 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote; 1s local, disk-cache 1167: �[32m[3,122 / 3,123]�[0m 24 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote; 14s local, disk-cache 1168: �[32m[3,123 / 3,124]�[0m 25 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote; 0s disk-cache 1169: �[32m[3,123 / 3,124]�[0m 25 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote 1170: �[32m[3,123 / 3,124]�[0m 25 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote; 1s local, disk-cache 1171: �[32m[3,123 / 3,124]�[0m 25 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:devtools-edge-remote; 88s local, disk-cache 1172: �[32m[3,124 / 3,125]�[0m 26 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote; 0s disk-cache 1173: �[32m[3,124 / 3,125]�[0m 26 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote 1174: �[32m[3,124 / 3,125]�[0m 26 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote; 1s local, disk-cache 1175: �[32m[3,124 / 3,125]�[0m 26 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote; 14s local, disk-cache 1176: �[32m[3,125 / 3,126]�[0m 27 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote; 0s disk-cache 1177: �[32m[3,125 / 3,126]�[0m 27 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote 1178: �[32m[3,125 / 3,126]�[0m 27 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote; 1s local, disk-cache 1179: �[32m[3,125 / 3,126]�[0m 27 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote; 14s local, disk-cache 1180: �[32m[3,126 / 3,127]�[0m 28 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote; 0s disk-cache 1181: �[32m[3,126 / 3,127]�[0m 28 / 29 tests, �[31m�[1m1 failed�[0m;�[0m [Sched] Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote 1182: �[32m[3,126 / 3,127]�[0m 28 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote; 1s local, disk-cache 1183: �[32m[3,126 / 3,127]�[0m 28 / 29 tests, �[31m�[1m1 failed�[0m;�[0m Testing //rb/spec/integration/selenium/webdriver:bidi-edge-remote; 14s local, disk-cache 1184: �[32mINFO: �[0mFound 29 test targets... 1185: �[32mINFO: �[0mElapsed time: 1531.342s, Critical Path: 778.22s 1186: �[32mINFO: �[0m2866 processes: 1438 disk cache hit, 1168 internal, 200 local, 60 worker. 1187: �[32mINFO: �[0mBuild completed, 1 test FAILED, 2866 total actions 1188: //rb/spec/integration/selenium/webdriver:action_builder-edge-remote �[0m�[32mPASSED�[0m in 28.4s 1189: //rb/spec/integration/selenium/webdriver:bidi-edge-remote �[0m�[32mPASSED�[0m in 14.5s 1190: //rb/spec/integration/selenium/webdriver:devtools-edge-remote �[0m�[32mPASSED�[0m in 88.2s 1191: //rb/spec/integration/selenium/webdriver:driver-edge-remote �[0m�[32mPASSED�[0m in 34.2s 1192: //rb/spec/integration/selenium/webdriver:element-edge-remote �[0m�[32mPASSED�[0m in 62.1s 1193: //rb/spec/integration/selenium/webdriver:error-edge-remote �[0m�[32mPASSED�[0m in 14.8s ... 1208: //rb/spec/integration/selenium/webdriver/bidi:browsing_context-edge-remote �[0m�[32mPASSED�[0m in 14.4s 1209: //rb/spec/integration/selenium/webdriver/bidi:log_inspector-edge-remote �[0m�[32mPASSED�[0m in 14.6s 1210: //rb/spec/integration/selenium/webdriver/bidi:script-edge-remote �[0m�[32mPASSED�[0m in 14.5s 1211: //rb/spec/integration/selenium/webdriver/edge:driver-edge-remote �[0m�[32mPASSED�[0m in 34.6s 1212: //rb/spec/integration/selenium/webdriver/edge:options-edge-remote �[0m�[32mPASSED�[0m in 21.4s 1213: //rb/spec/integration/selenium/webdriver/edge:profile-edge-remote �[0m�[32mPASSED�[0m in 15.0s 1214: //rb/spec/integration/selenium/webdriver/edge:service-edge-remote �[0m�[32mPASSED�[0m in 21.8s 1215: //rb/spec/integration/selenium/webdriver/remote:element-edge-remote �[0m�[32mPASSED�[0m in 16.1s 1216: //rb/spec/integration/selenium/webdriver/remote:driver-edge-remote �[0m�[31m�[1mFAILED�[0m in 3 out of 3 in 29.1s 1217: Stats over 3 runs: max = 29.1s, min = 27.9s, avg = 28.4s, dev = 0.5s 1218: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/remote/driver-edge-remote/test.log 1219: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/remote/driver-edge-remote/test_attempts/attempt_1.log 1220: D:/_bazel/execroot/_main/bazel-out/x64_windows-fastbuild/testlogs/rb/spec/integration/selenium/webdriver/remote/driver-edge-remote/test_attempts/attempt_2.log 1221: Executed 29 out of 29 tests: 28 tests pass and �[0m�[31m�[1m1 fails locally�[0m. 1222: There were tests whose specified size is too big. Use the --test_verbose_timeout_warnings command line option to see which ones these are. 1223: �[0m 1224: ##[error]Process completed with exit code 1.

✨ CI feedback usage guide:

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

Failed stage
Failed test name
Failure summary
Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.

VietND96 · 2024-10-29T12:55:16Z

Thank you, @joerg1985 !

[grid] limit the number of websocket connections per session

be01115

qodo-code-review bot added P-enhancement PR with a new feature tests Review effort [1-5]: 3 labels Aug 19, 2024

VietND96 requested a review from diemol October 25, 2024 13:02

VietND96 added the B-grid Everything grid and server related label Oct 25, 2024

VietND96 added 2 commits October 25, 2024 13:02

Merge branch 'trunk' into limit-connections

3745c62

Merge branch 'trunk' into limit-connections

0b5e1c8

VietND96 merged commit e9e684d into trunk Oct 29, 2024

VietND96 deleted the limit-connections branch October 29, 2024 12:55

This was referenced Dec 4, 2024

[🐛 Bug]: Unexpected HTTP response status code 400 error when running tests inside chrome-standalone docker container #14842

Closed

[grid] decrement the connection per session counter on close #14842 #14854

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[grid] limit the number of websocket connections per session#14410

[grid] limit the number of websocket connections per session#14410
VietND96 merged 3 commits intotrunkfrom
limit-connections

joerg1985 commented Aug 19, 2024 •

edited by qodo-code-review bot

Loading

Uh oh!

qodo-code-review bot commented Aug 19, 2024

Uh oh!

qodo-code-review bot commented Aug 19, 2024 •

edited

Loading

Uh oh!

qodo-code-review bot commented Aug 19, 2024 •

edited

Loading

Configuration options

Uh oh!

VietND96 commented Oct 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

joerg1985 commented Aug 19, 2024 • edited by qodo-code-review bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

Description

Motivation and Context

Types of changes

Checklist

PR Type

Description

Changes walkthrough 📝

Uh oh!

qodo-code-review bot commented Aug 19, 2024

PR Reviewer Guide 🔍

Uh oh!

qodo-code-review bot commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Uh oh!

qodo-code-review bot commented Aug 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Failure Feedback 🧐

(Checks updated until commit 0b5e1c8)

Configuration options

Uh oh!

VietND96 commented Oct 29, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

joerg1985 commented Aug 19, 2024 •

edited by qodo-code-review bot

Loading

qodo-code-review bot commented Aug 19, 2024 •

edited

Loading

qodo-code-review bot commented Aug 19, 2024 •

edited

Loading

(Checks updated until commit `0b5e1c8`)