Protocol message to reset client state #1213
Conversation
magnetised
force-pushed
the
garry/vax-1817-protocol-message-to-reset-client-state
branch
3 times, most recently
from
46f0255
to
81369a3
Compare
| await this._resetClientState({ | ||
| keepSubscribedShapes: true, | ||
| }) | ||
| this._subscribePreviousShapeRequests() |
There was a problem hiding this comment.
Just an FYI: this working is contingent on the server behaviour. If the server does not remove client subscriptions from the ClientReconnectionInfo and from the WebsocketServer process' state, it will respond with errors to client's attempt to subscribe using a known subscription ID.
There was a problem hiding this comment.
Thanks @alco, helpful reminder
|
I'm missing some context here but from @magnetised's explanation this morning, i'm guessing this message is meant to reset the client state when the client's permissions change, right? This is needed such that both the client and server are "in sync", i.e. they both restart from the "empty state" for this client (which i'm assuming is an enabling simplification). While this will also erase data that a client no longer has permission to see, this is not really it's primary purpose since we can't enforce that on malicious clients, right? |
| this.emitter.on('command', callback) | ||
| } | ||
|
|
||
| unsubscribeToCommands(callback: CommandCallback): void { |
There was a problem hiding this comment.
nit: we unsubscribe from something
There was a problem hiding this comment.
agreed - I was just being consistent with the current unsubscribe function names e.g. unsubscribeToAdditionalData, unsubscribeToTransactions, unsubscribeToRelations
| sql: 'SELECT * FROM main.parent', | ||
| }) | ||
|
|
||
| t.deepEqual(results, []) |
There was a problem hiding this comment.
Should we also check that the subscribed shapes are kept?
There was a problem hiding this comment.
sure. rather than crowbar it in here though, I'll add it in a later pr and ping you.
that's right. pending a "proper" refresh of the subscription information after a permissions change, which is quite complex, this is a simple way to ensure that the client receives data consistent with their new permissions. and no, we can't prevent people from seeing data that's on their device, even if they've lost permissions to see it, so the purpose of this re-sync is to maintain consistency between the client and the server |
magnetised
force-pushed
the
garry/vax-1710-feature-flag-to-turn-on-permissions-in-hard-mode
branch
from
13c9458
to
7a2f4b9
Compare
magnetised
force-pushed
the
garry/vax-1817-protocol-message-to-reset-client-state
branch
from
81369a3
to
5a7e0cc
Compare
Rather than attempt to reset the server connection's state after we've asked the client to reset itself, we just close the connection. The client will then re-connect automatically and, since it's had its state reset, the server will treat it as a first connection (including dropping all client reconnection state). Since this is intended as an enabling simplification to maintain consistency between client and server in the simplest way possible, pending a more efficient, less brutal but much more complicated solution, I've tried to keep the changes to the protocol as small as possible. Hence the use of `throw` -- it's either that or restructure all the message handling on the server side to be more of a `reduce_while` style.
magnetised
merged commit 4e9314a
into
garry/vax-1710-feature-flag-to-turn-on-permissions-in-hard-mode
magnetised
deleted the
garry/vax-1817-protocol-message-to-reset-client-state
branch
No description provided.