Skip to content

feat(node): support connection string syntax for nodejs client creation - #3917

Open
T1B0 wants to merge 2 commits into
apache:masterfrom
T1B0:add-connection-string-support-node-sdk
Open

feat(node): support connection string syntax for nodejs client creation#3917
T1B0 wants to merge 2 commits into
apache:masterfrom
T1B0:add-connection-string-support-node-sdk

Conversation

@T1B0

@T1B0 T1B0 commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Which issue does this PR address?

this PR add node sdk support for connection string syntax on client creation
accept common url like syntax new Client("iggy://iggy:iggy@127.0.0.1:8090");

note node:url module was not used since it would have introduced small differencez with rust and other sdk parsing, so connection string is manually parsed to match others sdk behaviors.

@T1B0
T1B0 requested review from hubcio and spetz August 18, 2026 16:15
@github-actions github-actions Bot added the S-waiting-on-review PR is waiting on a reviewer label Aug 18, 2026
@T1B0
T1B0 force-pushed the add-connection-string-support-node-sdk branch from 2afce42 to e9ffac3 Compare August 18, 2026 16:16
@codecov

codecov Bot commented Aug 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.14040% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.06%. Comparing base (37d7eee) to head (a09ab2e).

Files with missing lines Patch % Lines
...oreign/node/src/client/client.connection-string.ts 99.15% 2 Missing ⚠️
foreign/node/src/stream/consumer-stream.ts 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #3917      +/-   ##
============================================
+ Coverage     84.03%   84.06%   +0.03%     
  Complexity     1358     1358              
============================================
  Files          1215     1217       +2     
  Lines        170820   170725      -95     
  Branches     138576   138239     -337     
============================================
- Hits         143547   143528      -19     
+ Misses        23411    23347      -64     
+ Partials       3862     3850      -12     
Components Coverage Δ
Rust Core 84.93% <ø> (+<0.01%) ⬆️
Java SDK 66.67% <ø> (ø)
C# SDK 74.96% <ø> (ø)
Python SDK 90.10% <ø> (ø)
PHP SDK 85.65% <ø> (ø)
Node SDK 95.97% <99.14%> (+0.16%) ⬆️
Go SDK 68.29% <ø> (ø)
Files with missing lines Coverage Δ
foreign/node/src/client/client.config.ts 100.00% <100.00%> (ø)
foreign/node/src/client/client.connection.ts 96.19% <100.00%> (-0.27%) ⬇️
foreign/node/src/client/client.socket.ts 95.69% <100.00%> (+0.01%) ⬆️
foreign/node/src/client/client.ts 97.17% <100.00%> (-0.02%) ⬇️
foreign/node/src/constant.ts 100.00% <100.00%> (ø)
foreign/node/src/stream/consumer-stream.ts 76.22% <83.33%> (+0.16%) ⬆️
...oreign/node/src/client/client.connection-string.ts 99.15% <99.15%> (ø)

... and 46 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@T1B0
T1B0 force-pushed the add-connection-string-support-node-sdk branch 5 times, most recently from 6a3b7a1 to 44edc7c Compare August 22, 2026 17:58
Comment thread foreign/node/src/client/client.connection-string.ts Outdated
@github-actions github-actions Bot added S-waiting-on-author PR is waiting on author response and removed S-waiting-on-review PR is waiting on a reviewer labels Aug 23, 2026
@T1B0
T1B0 force-pushed the add-connection-string-support-node-sdk branch from 44edc7c to be0d997 Compare August 24, 2026 10:36
slbotbm
slbotbm previously approved these changes Aug 24, 2026
@T1B0
T1B0 force-pushed the add-connection-string-support-node-sdk branch 4 times, most recently from 3745728 to 3de15b0 Compare August 25, 2026 07:30

@hubcio hubcio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few notes from comparing against the rust parser. one i'd want fixed before merge - the credential echo in errors.

these landed outside the diff:

foreign/node/src/stream/consumer-stream.ts:81 and :113 - singleConsumerStream and groupConsumerStream still take ClientConfig, so passing a connection string fails to compile (TS2345). they call the widened getClient, and since getClient isn't exported they're the only place users can reach the widening.

foreign/node/src/client/client.connection.ts:86-90 - reconnect timer is never unref'd and the handle isn't kept, so unlimited means the process can't exit and destroy() can't cancel a wait already in flight. the heartbeat timer next door unrefs itself with a comment saying why. pre-existing, but this PR makes unlimited a one-word option and ships it in the new e2e.

foreign/node/src/client/client.config.ts:61 - error says heartbeatInterval in raw ms at someone who typed heartbeat_interval=1000h.

nothing tests the tls branch - the pre-merge action runs tls.system.e2e.ts, which builds from an object config, so the parser never gets hit.

Comment thread foreign/node/src/client/client.connection-string.ts Outdated
Comment thread foreign/node/src/client/client.connection-string.ts Outdated
Comment thread foreign/node/src/client/client.connection-string.ts Outdated
Comment thread foreign/node/src/client/client.connection-string.ts
Comment thread foreign/node/src/client/client.connection-string.ts Outdated
Comment thread foreign/node/src/client/client.connection-string.ts Outdated
Comment thread foreign/node/src/client/client.connection-string.ts Outdated
Comment thread foreign/node/src/client/index.ts Outdated
Comment thread foreign/node/README.md
Comment thread foreign/node/src/e2e/tcp.connection-string.e2e.ts
@T1B0
T1B0 force-pushed the add-connection-string-support-node-sdk branch 5 times, most recently from 1760fed to 83122df Compare August 25, 2026 17:06
@T1B0
T1B0 force-pushed the add-connection-string-support-node-sdk branch from 83122df to 8c10423 Compare August 25, 2026 17:10
@T1B0
T1B0 requested a review from hubcio August 25, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author PR is waiting on author response

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants