opts string #17
opts string #17
Conversation
Improved encode and parse functions to handle TLS options and extended URL parsing capabilities. Refactored property encoding and decoding logic for better maintainability. Introduced test cases to ensure correct handling of TLS and URL parsing features.
| ) { | ||
| u = new URL(opts.servers[0]); | ||
| } else { | ||
| u = new URL(`nats://${opts.servers[0]}`); |
There was a problem hiding this comment.
Should this do something to start with the opts.servers starting with something other than wss:// or ws://? It looks like if I provide nats:// then this will turn it into nats://nats://whatever, and tls://x into nats://tls://x
There was a problem hiding this comment.
for the javascript client you don't provide the scheme - it is host port, the only one that needs to have a scheme is ws/s or I cannot tell what they are. - But possibly the thing to do is whip through all the servers and rip out the scheme if it isn't ws/s.
There was a problem hiding this comment.
I have added a check for this - the javascript clients strops nats/tls type schemes if they are included as they are meaningless. For secure options, tls needs to be set, this prevents inconsistencies where a client connects via TLS on one server, but not in another because of some prefix.
There was a problem hiding this comment.
So you've now introduced in parse() handling for natss: to do the TLS-handshake-on-connect behavior, which is great. But doesn't this mean that encode() should also check for natss: and avoid prepending nats: to that? Or are we relying upon the parse inserting only the hostname into the server list, so encode will never see that?
There was a problem hiding this comment.
Encode is using the connection options, there's no such prefix that is understood - the serialized version does do that, but when decoded recreates the connect options.
2 participants
No description provided.