Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebSocket Subscriptions via JRPC #107

Merged
merged 40 commits into from
Feb 27, 2024

Conversation

LiranCohen
Copy link
Member

@LiranCohen LiranCohen commented Feb 12, 2024

This PR enhances WebSocket support via JSON RPC 2.0.

This now supports long running subscriptions via JSON RPC.
The new JSONRPCSocket abstracts this by providing two methods,request and subscribe.

request sends a JSON RPC message over the socket, setting up a message handler to listen to the response, once the response is received it will clean up the listener, and resolve the response to the promise. If a response is not emitted within a given timeout, it will reject the promise.

subscribe does something similar, however after receiving the response to the subscribe message, it will keep the handler open and look for any other messages that match the JSON RPC Id provided, and emit those to a message handler. The subscribe method returns a close method in order to clean up these listeners.

Some things to note:

  • RecordsWrite are currently not supported via sockets, this is due to a poor handling of the data payload in the prior implementation. Would rather add this as a separate effort. TODO is tagged in code with the issue listed below.
  • Subscribe methods are currently only supported via sockets and hot http.
  • A subscription.close JSON RPC Method was added to close a subscription that is active for a connection. I went back and forth between making this a DWN Message vs some other signal. I landed on this for the time being and am open to discussion. More notes below.
  • As a part of getDWNConfig, tenantGate and eventStream were both made optional, as well as the registrationManager for HttpApi. I did this mostly out of convenience, but it also seems plausible that someone might run this without any registration manager. Open to discuss/change.
  • the subscriptionRequest method within tests/utils.ts will be replaced by a full-fledge client, listed in a separate PR below. Less attention was paid to making this usable anywhere outside of the current tests.
  • the sendHttpMessage method within tests/utils.ts will also be replaced by full-fledged client, listed in a separate PR below.
  • Current implementation allows anyone to connect, this will be addressed in a subsequent PR, issue listed below.

Usage of subscription.close JSON RPC method.

Q: Why not use a specific Unsubscribe DWN message such as RecordsUnsubscribe?
A: This would be the first message of it's kind that would need to specifically target the DWN and potentially transport of the DWN which holds the subscription. Instead the DWN RecordsSubscribe message returns a close method which the transport can keep a reference to given a specific JSON RPC Id. This JSON RPC Id represents a specific request to a transport that was created. Later a user can issue a subscription.close JSON RPC Method to tell the server to close that subscription.

Ultimately the owner of the JRPC Socket connection is in charge of closing the subscription, they can close all subscriptions by simply disconnecting. So it makes sense to give them the ability to close a specific JRPC Subscription.

Initial Effort Subsequent Issues/PRs:

Separate effort:

@codecov-commenter
Copy link

codecov-commenter commented Feb 13, 2024

Codecov Report

Attention: Patch coverage is 99.38556% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 94.34%. Comparing base (0e14b8d) to head (95bb520).

Files Patch % Lines
src/json-rpc-socket.ts 98.66% 2 Missing ⚠️
src/index.ts 0.00% 1 Missing ⚠️
src/lib/json-rpc.ts 97.29% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #107      +/-   ##
==========================================
+ Coverage   90.46%   94.34%   +3.88%     
==========================================
  Files          20       25       +5     
  Lines        1752     2212     +460     
  Branches      208      291      +83     
==========================================
+ Hits         1585     2087     +502     
+ Misses        167      125      -42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

src/ws-api.ts Outdated Show resolved Hide resolved
src/ws-api.ts Outdated Show resolved Hide resolved
src/ws-api.ts Outdated Show resolved Hide resolved
src/ws-api.ts Outdated Show resolved Hide resolved
src/ws-api.ts Outdated Show resolved Hide resolved
src/dwn-error.ts Outdated Show resolved Hide resolved
@LiranCohen LiranCohen force-pushed the lirancohen/ws-jrpc-long-running-subscriptions branch from 763410d to f54fac2 Compare February 23, 2024 13:33
@LiranCohen
Copy link
Member Author

@thehenrytsai The biggest change I've made was removing the subscriptionRequest test util in favor of a smarter way to handle the subscriptions within the context of jsonrpc. The changes mostly live within the JsonRpcSocket class and the handleDwnProcessMessage handler method.

I chose to model this change in an attempt to be compliant with the JsonRpc spec guidelines for extensions. Looking to see how they normally extend the spec's capabilities I came across this:
https://groups.google.com/g/json-rpc/c/BYv_s0XIfF4/m/Vj60QgclAgAJ

So we've added an optional subscribe object to JsonRpcRequest which contains a unique JsonRpcId that the client will use to listen to the events emitted for this subscription request. This way the Id for the rpc request to initiate a long-lived subscription and the emitted events themselves are identifiable from one another.

To close this long-running subscription we initiate an rpc.subscribe.close RPC method, which is an extension-specific method as per the JSON RPC spec. This method will also look for the same type of subscribe object as in the subscribe request which contains the subscription Id that the client has been listening for. The server will then clean up anything it needs to related to that subscription upon receipt and stop emitting messages with that Id.

src/json-rpc-api.ts Outdated Show resolved Hide resolved
src/json-rpc-api.ts Outdated Show resolved Hide resolved
src/lib/json-rpc.ts Outdated Show resolved Hide resolved
src/lib/json-rpc.ts Outdated Show resolved Hide resolved
src/lib/json-rpc.ts Outdated Show resolved Hide resolved
thehenrytsai
thehenrytsai previously approved these changes Feb 27, 2024
Copy link
Member

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

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

This is an impactful one!!!

🐐🐐🐐 🎖️ 🎖️ 🎖️

tests/json-rpc-socket.spec.ts Outdated Show resolved Hide resolved
Copy link
Member

@thehenrytsai thehenrytsai left a comment

Choose a reason for hiding this comment

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

🐐 🐐 🐐

@LiranCohen LiranCohen merged commit e1396cb into main Feb 27, 2024
8 checks passed
@LiranCohen LiranCohen deleted the lirancohen/ws-jrpc-long-running-subscriptions branch February 27, 2024 21:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants