wintercg / proposal-sockets-api

Proposal for an API for establishing TCP connections in Non-Browser JavaScript runtime environments
https://sockets-api.proposal.wintercg.org/
Other
46 stars 2 forks source link

Peer addresses #17

Open mmastrac opened 9 months ago

mmastrac commented 9 months ago

The socket should provide the local and peer addresses for connections. For example, you may wish to know on which port the outgoing connection was established for logging or accounting purposes. In addition, it may be useful to know which IP address a wildcard connection resolved to (though obviously this will be 127.0.0.1 in most cases).

jasnell commented 9 months ago

I've been considering adding a socket.info property for precisely this kind of metadata and other use cases.

The local address would need to be optional as managed environments like Workers do not actually have a useful local address given the fact that the connection is proxied.

For the remote address, this could also be a bit tricky due to proxying if the runtimes view of the remote address is a local/private address.

I think for both of these I would make these: implementations MAY provide information on the local/remote addresses for connections.

The other use case I have for socket.info are stats like socket.info.bytesSent and socket.info.bytesReceived, primarily for debugging purposes.

mmastrac commented 9 months ago

I agree -- MAY is the right term for this.

jasnell commented 9 months ago

Added in https://github.com/wintercg/proposal-sockets-api/pull/19

const socket = connect('https://example.com');
await info = socket.opened;
console.log(info.remoteAddress);
console.log(info.localAddress);
teabroker commented 9 months ago

The other use case I have for socket.info are stats like socket.info.bytesSent and socket.info.bytesReceived, primarily for debugging purposes.

This kind of information is better to put into method called stat. It should be synchronous, because statistics could have zero values and doesn't need a connection to be established to be available.

let stat = socket.stat()

console.log(stat.bytesSent)
console.log(stat.bytesReceived)

It also could contain information about delays, number of packets, connection attempts, network errors, ping timeout and other common network statistics. It could be useful for debugging, testing, and for devTools purposes.

jasnell commented 8 months ago

The SocketInfo dictionary has been added providing these. It is currently only provided when the opened promise is resolved but I would also like to expose a socket.info property that also provides this. That would serve the same purpose as socket.stat. It does not need to be a method, it can simply be an attribute.

dom96 commented 8 months ago

I'm not sure that kind of info should be provided by this API, instead it should be provided by dev tools (which presumably have other APIs they use to get this kind of info).

mmastrac commented 8 months ago

@dom96 This is critically necessary for server-side hosted applications in many cases.

dom96 commented 8 months ago

I see. Then it makes sense, but I would say "MAY" (as you've written above) is definitely the way to go for these. My worry is we discourage implementors of this API if it becomes too big, so making this stuff optional sounds like a good way to go.

mmastrac commented 8 months ago

I think that's reasonable, as browsers may not wish to expose this information ever while servers may almost always.

teabroker commented 8 months ago

@jasnell

The SocketInfo dictionary has been added providing these. It is currently only provided when the opened promise is resolved but I would also like to expose a socket.info property that also provides this. That would serve the same purpose as socket.stat. It does not need to be a method, it can simply be an attribute.

I see several drawbacks is seen right now. The connection opened promise resolves only when connection is fully established. From § 3.3.3:

The opened attribute is a promise that is resolved when the socket connection has been successfully established, or is rejected if the connection fails. For sockets which use secure-transport, the resolution of the opened promise indicates the completion of the secure handshake.

It means that connection became a total blackbox when it stales. And when connection fails due to timeout, developers wouldn't have information for analysis.

So I think it's critical to have this property available since the moment the Socket instance was created. Am I right that you and other authors shares this point of view?

Second issue of having such property is it's dynamic behavior. Should the SocketInfo object be equal to itself conn.info === conn.info? And if yes, then these assertions should be valid:

const {info} = conn.info
const {bytesSent} = info;
bytesSent; // 0
await write(conn, "Hello"); // Send 5 bytes.
assert(conn.info === info);
assert(info.bytesSent !== bytesSent);

Though if conn.info === info, then it should not be a plain object, it should be an instance of SocketInfo. If it's a plain object it could be confusing for developers to find that some properties of detached info got changed over time. Now it's not obvious whether SocketInfo is a class or just a plain object. According to other W3C provided standards it should have its own class.

If conn.info !== info then it's better to be a method. While it's easier to use destruction to receive SocketInfo, in my opinion it would be less error prone to return it from a method.

I still think it's not a good idea to mix up connection details with a statistics in one object, for me it's very different kind of information and it's better to join all the statistic under a distinct object. Maybe as a property of SocketInfo. Could you explain the logic you follow?

teabroker commented 8 months ago

@dom96

My worry is we discourage implementors of this API if it becomes too big, so making this stuff optional sounds like a good way to go.

But underdeveloped standard could lead to a situation when developers would have to struggle fighting with the platform. When you building new API it's hard to predict what could be necessary, but network APIs are very common and we just can look around and collect best practices. Or if we don't know what the best practices are, then it's better to call developers who work in this field. Now it seems that socket could be very hard to control and debug. Do you have referential specifications you use to build this standard?

I could imagine some minimalistic runtimes like IoT, which could have only minimal set of APIs implemented. But I'm not sure whether these are the target platforms for WinterWG. So if this API is designed for some more complex runtimes then I'd suggest to focus on stability and predictability for developers, because when the platforms limits you it what makes it very hard to build reliable software. So I'm not sure that such kind of concerns should be on the first place, while I'm agree about the overall standard simplicity.

jasnell commented 8 months ago

@teabroker ... What I have in mind is this:

Though if conn.info === info, then it should not be a plain object, it should be an instance of SocketInfo. If it's a plain object it could be confusing for developers to find that some properties of detached info got changed over time. Now it's not obvious whether SocketInfo is a class or just a plain object. According to other W3C provided standards it should h

SocketInfo is defined as a webidl dictionary, which means it is realized as a plain javascript object but with defined properties with documented semantics, even tho it is not an actual class. Can you expand on why you believe this wouldn't be sufficient?

... we just can look around and collect best practices

The key challenge there is that it's difficult to analyze what exactly is really necessary. For instance, the Node.js tls api has a broad range (~49) of configuration options that we just simply have no idea whether folks are actually using or not. It's our intention to keep the options and properties available via this API as minimal as possible for the time being, expanding them only when there is a clear need and specific requests. We do not want to add anything just because someone might need it.