Add proxy protocol support and test infrastructure by stefanrammo · Pull Request #22 · CDPTechnologies/JavascriptCDPClient

stefanrammo · 2025-12-02T19:23:37Z

Add ServicesRequest/ServicesNotification handling for compatVersion >= 4
Update studioapi.proto.js with service message types
Remove StatusFlags enum from proto
Add Jest tests and GitHub Actions CI

Karmo7 · 2025-12-03T10:19:35Z

index.js

+          if (metadata.compatVersion >= 4) {
+            var servicesReq = obj.Container.create();
+            servicesReq.messageType = obj.ContainerType.eServicesRequest;
+            servicesReq.servicesRequest = obj.ServicesRequest.create({ subscribe: true });


C++ client also sets inactivity_resend_interval to 120 seconds to improve resiliency to packet loss for long running automatic services. So should enable this presuming the code here can handle duplicate ServicesNotifications.

Karmo7 · 2025-12-03T10:21:56Z

index.js

+                var servicesReq = obj.Container.create();
+                servicesReq.messageType = obj.ContainerType.eServicesRequest;
+                servicesReq.servicesRequest = obj.ServicesRequest.create({ subscribe: true });
+                socket.send(obj.Container.encode(servicesReq).finish());


Should extract the block as a function to avoid duplicating code from line 276. Then the ServicesRequest arguments will be set in one place only.

Karmo7

How are you testing this code? At least I did not get it working. I deployed two apps to different remote machines, started the apps, then ran sudo iptables -A OUTPUT -d 192.168.1.210 -j REJECT to block connection to one of them and tried to connect to the other one with the JS client. The result is:

Socket close: The connection was closed abnormally, e.g., without sending or receiving a Close control frame
Trying to reconnect ws://192.168.1.210:7692
Socket error: undefined
Socket close: The connection was closed abnormally, e.g., without sending or receiving a Close control frame
Trying to reconnect ws://192.168.1.210:7692
Socket error: undefined
Socket close: The connection was closed abnormally, e.g., without sending or receiving a Close control frame
Trying to reconnect ws://192.168.1.210:7692
Socket error: undefined

Karmo7 · 2025-12-03T10:48:24Z

README.rst

+
+        client.root().then(root => {
+          // All apps accessible (proxy + discovered backends)
+          root.forEachChild(app => {


That only prints the first app? Because it does not add itself as listener but only prints once at startup.

Seems it was trying to use direct connection here. In the commit amend it should default to proxy mode so should work now.

Tried running the new version and connect to a system with two apps, for me it prints:

Connected to: App2 /home/kar/dev/projects/JavascriptCDPClient/index.js:781 this.applicationNodes().forEach(function (app) { ^ TypeError: this.applicationNodes is not a function at async.subscribeToStructure (/home/kar/dev/projects/JavascriptCDPClient/index.js:781:12) at INode.subscribeToStructure (/home/kar/dev/projects/JavascriptCDPClient/index.js:1641:18) at /home/kar/dev/projects/JavascriptCDPClient/test2.js:13:16 at process.processTicksAndRejections (node:internal/process/task_queues:105:5) Node.js v24.10.0

Does this example here work for you?

This is a bug. The this. in some functions referred to async not SystemNode, I used app.node for testing so I did not notice it, fixed in commit amend.

How are you testing this example? I've already reported 3 times it gives errors and now with latest I still got an error

node:internal/process/promises:392 new UnhandledPromiseRejection(reason); ^ UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "Child named 'RolePermissions' not found". at throwUnhandledRejectionsMode (node:internal/process/promises:392:7) at processPromiseRejections (node:internal/process/promises:475:17) at process.processTicksAndRejections (node:internal/process/task_queues:106:32) { code: 'ERR_UNHANDLED_REJECTION' }

Example was missing a .catch() to handle non-app structure events. I didn't test the exact readme example before.

nuubik · 2025-12-05T11:45:07Z

README.rst

+Proxy Support
+-------------
+
+The client supports the Generic Proxy Protocol (requires CDP application with compatVersion >= 4) which allows


Maybe the Actual CDP version this is supported from would be more helpful for the end user than the protocol version.

Karmo7 · 2025-12-08T10:44:15Z

README.rst

+
+        client.root().then(async root => {
+          // Wait for backend apps to be discovered
+          await new Promise(r => setTimeout(r, 2500));


Waits should not be if one subscribes to structure?

Karmo7 · 2025-12-22T11:21:43Z

README.rst

+              subscribeToApp(appName);
+            }
+          });
+        });


It is better now but I still get some issues when testing. The sequence is:

Create a CDP 5.1 system with 3 applications (it works with CDP 5.0 where proxy is not used).

Run the system (I tested remote deploy to a single raspberry pi but probably not important).

Connect the example code here to the first app

Use CDP Studio to stop and restart the other apps

The client is expected to reconnect to sibling apps after they are restarted. But what happens instead is sometimes connection just fails and sometimes I get ProtocolError printouts in addition:

Have you tested the sequence I described in the previous comment? It still does not work for me. When I close App2, the client loses connection to all sibling apps (both App2 and App3). Also, it does not reconnect after I restart App2.

Should always test code before committing.

Karmo7 · 2026-03-09T11:41:52Z

README.rst

+
+- Sends a ServicesRequest to discover available proxy services
+- Receives ServicesNotification with available backend applications
+- Establishes virtual connections to backend applications with ``type: 'websocketproxy'`` and ``metadata.proxy_type: 'studioapi'``


This doc is meant for the user of the JS client public interface. I don't think it is relevant to mention how which protobuf messages are send in the background. So I'd remove these 4 lines.

Should think of the doc as a quick start and API reference guide not implementation details.

Karmo7 · 2026-03-09T11:44:54Z

README.rst

+- Simplified firewall configuration - only one port needs to be opened
+- SSH port forwarding - forward a single port to access entire CDP system
+
+Example


It is good to have this example code in the doc but I understand that the code would look exactly the same whether connecting to a CDP 5.1 system with proxy support or an older one without proxy support?

If that is the case, should move the example to top-level (currently it is under proxy support paragraph). And maybe put the example before the proxy support paragraph as that might be more relevant to a user as a quick start guide.

Karmo7 · 2026-03-09T11:55:33Z

README.rst

+Proxy Support (CDP 5.1+)
+------------------------
+
+The client supports the Generic Proxy Protocol which allows a single WebSocket connection to access


"Generic Proxy Protocol" - have we used that name anywhere in CDP doc? If not, it is weird to invent a name in JS client doc, so maybe don't use capital letters and reword, e.g. "CDP 5.1+ proxy feature"

Karmo7 · 2026-03-09T11:58:19Z

README.rst

+~~~~~~~~~
+
+- Simplified firewall configuration - only one port needs to be opened
+- SSH port forwarding - forward a single port to access entire CDP system


Maybe add an extra sentence that for older CDP versions without the proxy support, the client connected directly to each CDP application in a system meaning that a dedicated port had to be opened for each application.

Karmo7 · 2026-03-09T11:59:32Z

index.js

+              if (typeof window !== 'undefined' && typeof window.confirm === 'function') {
+                window.confirm(metadata.systemUseNotification) ? resolve() : reject();
+              } else {
+                // Node.js: auto-accept system use notification


Confirm with @martlaak that auto-accept is ok as a default.

Yes. We can auto-accept in our default applicationAcceptanceRequested() handler. Users can overwrite and provide their custom applicationAcceptanceRequested() handler when they need to show/prompt the message somewhere or verify the message against some known notification.

Karmo7 · 2026-03-09T12:28:43Z

index.js

+      f.invalidateApp = function(appName) {
+        Object.keys(memoize).forEach(function(key) {
+          // Key is array converted to string, e.g. "App2" or "App2,CPULoad"
+          if (key === appName || key.startsWith(appName + ',')) {


Comma? CDP routings normally use dot. Make sure to test this actually works

Cache keys come from Array.toString() which joins with commas.

Karmo7 · 2026-03-09T12:30:53Z

index.js

+     * @param {number} sampleRate - Maximum samples per second (default 0 = all)
+     * @returns {Promise} Resolves when subscription is registered (not necessarily established)
+     */
+    this.subscribeWithResume = function(nodePath, valueConsumer, fs, sampleRate) {


Are these new "WithResume" methods meant to be public API for users or private API here?

Public. Documented them

Karmo7 · 2026-03-09T12:37:07Z

README.rst

+    .. code:: javascript
+
+        // Connect to a proxy-enabled CDP application (with authentication)
+        const client = new studio.api.Client("127.0.0.1:7690", {


I think should also add const studio = require('./index'); line, so the code would work out of box. Or at least comment how to run in nodejs and browser context. I do see there is something in the API paragraph but this is currently after the example, so a user reading this doc might get confused. Should imporve or reorganize here.

Enable single WebSocket connection to access multiple CDP applications via ServiceMessage tunneling (compatVersion >= 4). Key features: - ServicesRequest/ServicesNotification for proxy service discovery - ServiceMessage tunneling (eConnect, eConnected, eData, eDisconnect, eError) - Send buffering to prevent race conditions before eConnected - Dynamic sibling discovery via subscribeToStructure - Client-level structure subscriptions for app ADD/REMOVE notifications - 30-second connect timeout with automatic cleanup - Graceful cleanup on primary connection close Also: - Update studioapi.proto.js with service message types - Add README proxy example - Add Jest tests and GitHub Actions CI

stefanrammo requested review from Karmo7, nuubik and nuustik December 2, 2025 19:23

Karmo7 reviewed Dec 3, 2025

View reviewed changes

Karmo7 requested changes Dec 3, 2025

View reviewed changes

nuustik approved these changes Dec 3, 2025

View reviewed changes

stefanrammo force-pushed the add-proxy-support branch 4 times, most recently from 18fd464 to 61afdfa Compare December 4, 2025 12:57

nuubik approved these changes Dec 5, 2025

View reviewed changes

stefanrammo force-pushed the add-proxy-support branch from 61afdfa to 5e0301b Compare December 8, 2025 07:39

Karmo7 reviewed Dec 8, 2025

View reviewed changes

stefanrammo force-pushed the add-proxy-support branch 2 times, most recently from d7a1043 to f543ce9 Compare December 11, 2025 08:53

stefanrammo requested a review from Karmo7 December 19, 2025 08:32

Karmo7 requested changes Dec 22, 2025

View reviewed changes

stefanrammo force-pushed the add-proxy-support branch 2 times, most recently from b9f3ac1 to fe7138f Compare January 6, 2026 19:27

stefanrammo force-pushed the add-proxy-support branch from fe7138f to 7f3de28 Compare January 22, 2026 11:14

stefanrammo force-pushed the add-proxy-support branch 2 times, most recently from 491509e to fc6f6b5 Compare February 6, 2026 06:53

Karmo7 approved these changes Mar 9, 2026

View reviewed changes

stefanrammo force-pushed the add-proxy-support branch from fc6f6b5 to 7e88adb Compare March 10, 2026 10:51

stefanrammo force-pushed the add-proxy-support branch from 7e88adb to 72563f7 Compare March 10, 2026 15:14

Conversation

stefanrammo commented Dec 2, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karmo7 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Karmo7 Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Karmo7 Dec 4, 2025 •

edited

Loading