adding subscription handling to dwn server. #68
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
@finn-tbd if you have any time, can you take a look and give some feedback? I feel like there's some better ways to handle a few of these pieces, but wanted to get your thoughts if possible. |
| export class SubscriptionManager { | ||
| private wss: WebSocketServer; | ||
| private dwn: Dwn; | ||
| private connections: Map<string, Subscription>; |
There was a problem hiding this comment.
keep track of all the subscriptions here. IMPORTANT is to close the subscription when it's finished.
| if ( | ||
| messageType === DwnInterfaceName.Records + DwnMethodName.Write && | ||
| !dataStream | ||
| ) { | ||
| reply = await dwn.synchronizePrunedInitialRecordsWrite(target, message); | ||
| } else if ( | ||
| messageType === | ||
| DwnInterfaceName.Subscriptions + DwnMethodName.Request |
There was a problem hiding this comment.
we need to handle subscription requests differently than a normal request, because we also need to setup and manage the socket connection, unlike most messages which terminate after the data is sent.
| @@ -22,4 +22,8 @@ export const config = { | |||
|
|
|||
| // log level - trace/debug/info/warn/error | |||
| logLevel: process.env.DWN_SERVER_LOG_LEVEL || 'INFO', | |||
|
|
|||
| subscriptionsEnabled: | |||
| { on: true, off: false }[process.env.SUBSCRIPTIONS] ?? true, | |||
There was a problem hiding this comment.
please change the environment variable to DWN_SUBSCRIPTIONS. I realize not all environment variables currently follow that convention, but I would like to move in that direction.
|
sorry i think this got lost in the shuffle. I made one comment but i'm hoping to do more in depth looking later today. |
|
i looked through this more, it seems fine for now. |
|
Closing in favor of: #107 |
Server updates for #508. In draft as #508 is still in progress and still working out some kinks. Subscriptions only work on web socket connections. It's a little different than the rest of the processMessage records, as the server has to manage the live socket connection pipes given by the dwn.
In DRAFT until everything is together and also some of the tests aren't working yet (fixing today).
Do NOT merge yet, but sharing for transparency about direction.