feat(node): support connection string syntax for nodejs client creation - #3917
feat(node): support connection string syntax for nodejs client creation#3917T1B0 wants to merge 2 commits into
Conversation
2afce42 to
e9ffac3
Compare
Codecov Report❌ Patch coverage is
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
🚀 New features to boost your workflow:
|
6a3b7a1 to
44edc7c
Compare
44edc7c to
be0d997
Compare
3745728 to
3de15b0
Compare
hubcio
left a comment
There was a problem hiding this comment.
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.
1760fed to
83122df
Compare
83122df to
8c10423
Compare
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:urlmodule 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.