Closed michielbdejong closed 3 years ago
@acoburn @Vinnl can you review?
Looks mostly good to me. I wonder if it's possible/a good idea to also have the server notify the client if it's subscribing to too many updates before it closes the connection?
Yeah, that's probably what I would implement - send an informational error message 'too many subsriptions', then close the connections. thanks!
We should discuss this in https://www.w3.org/community/solid/wiki/Meetings#20190530_1000CEST
At present there are two keywords involved in this interaction: sub
and pub
. At what point should we start to view this interaction as a "protocol" with formal semantics. (I would be very much in favor of this). Defining a specific protocol for this would allow a client to have a clearer idea of exactly how the interaction works, and it would establish a stronger contract between client and server. As it is, the JavaScript API for establishing a WebSocket is:
WebSocket(url [, protocols])
If this specification defines a solid
protocol, this would be a way to formalize these interactions. Or would that be too much complexity/overhead for what we're trying to achieve?
^ sorry, didn't want to downvote your proposal on its merits as such, you're right that there are more beautiful ways to design the protocol we have. But it's what we have, and I like that it's supersimple.
The reason I downvoted is just that I think that changing the websockets-pubsub protocol to something different but functionally equivalent (i.e., a 'mostly sideways change') would have a tiny benefit to us as Solid app developers, and a huge cost.
I'm concerned that we are making medium changes to the spec here without sufficiently stepping back and thinking about the design.
This has all grown organically; no one has ever considered whether even the pub
/sub
mechanism is a good idea or whether there are standard alternatives. We are currently suggesting a fix on top of a hack.
Why don't we take a step back, do requirements engineering for the sockets, and then come back with a full solution instead of putting duct tape on duct tape?
It's now implemented in IPS, but no plans (yet) to implement it in NSS. Will leave this as a proposed spec change.
Do not merge unless NSS also implements this.