Skip to content

Add connection error/retry API and UI support#2902

Merged
texodus merged 4 commits intomasterfrom
client-reconnect
Mar 6, 2025
Merged

Add connection error/retry API and UI support#2902
texodus merged 4 commits intomasterfrom
client-reconnect

Conversation

@texodus
Copy link
Member

@texodus texodus commented Jan 23, 2025

This PR adds a reconnect API to Perspective, as well as a UX to invoke a reconnect to <perspective-viewer>.

@texodus texodus added the enhancement Feature requests or improvements label Jan 23, 2025
@texodus texodus linked an issue Jan 23, 2025 that may be closed by this pull request
@texodus texodus force-pushed the client-reconnect branch 2 times, most recently from d49720c to e4fe5c2 Compare February 28, 2025 07:23
Signed-off-by: Andrew Stein <steinlink@gmail.com>

# Conflicts:
#	rust/perspective-client/src/rust/lib.rs
@texodus texodus force-pushed the client-reconnect branch 3 times, most recently from d7bcc91 to 53416cc Compare March 6, 2025 06:03
texodus and others added 2 commits March 6, 2025 11:29
Signed-off-by: Andrew Stein <steinlink@gmail.com>
Signed-off-by: Tom Jakubowski <tom@prospective.dev>
@texodus texodus force-pushed the client-reconnect branch from a178ccc to 695f4dd Compare March 6, 2025 17:19
Signed-off-by: Andrew Stein <steinlink@gmail.com>
@texodus texodus force-pushed the client-reconnect branch from 695f4dd to b966a8b Compare March 6, 2025 18:44
@texodus texodus marked this pull request as ready for review March 6, 2025 20:12
@texodus texodus merged commit 04953db into master Mar 6, 2025
14 checks passed
@texodus texodus deleted the client-reconnect branch March 6, 2025 20:13
@greg-atomic
Copy link

Websockets can only handle auth during handshake. And auth tokens typically have an expiration time for security reasons. If a websocket is open for long enough then the token expires. The currently implemented reconnect function doesn't allow for updating the auth details in the new handshake, which happens at reconnect: it sends the old auth data that was used at the initial handshake. So the new reconnect functionality can't be used for this case: instead, one has to explicitly terminate and clean up the websocket connection and establish a brand new one to handle broken connections properly.

I'd suggest to change the reconnect function to enable for an updated URL to reconnect to as a parameter.

@texodus
Copy link
Member Author

texodus commented Jun 17, 2025

Websockets can only handle auth during handshake.

Perspective doesn't handle auth tokens at all. You will need to implement your own Client for a custom WebSocket protocol, there are no docs for this yet but you can follow the websocket.ts implementation in the repo

@greg-atomic
Copy link

Thanks for the quick response, really appreciate it! And I totally understand that you don't want to expand the scope of perspective with authentication. What I'm seeking is simply not making it impossible.

My problem is that this doesn't seem to be a typescript level, client wrapping question, but deeper in how rust handles websocket errors. The client's on_error function adds a non-removable error handler, which seems to keep firing even if one calls .terminate() on the client in typescript and deletes the client object.

As I can not use the reconnect function if there is authentication and expired tokens, I have no better idea than instantiating a new client. And what I see that the old on_error keeps firing because of the outdated url. And I also haven't found a way for cleaning up non-used clients, so if I go this route, after a while I see this error in the browser's console:

Uncaught (in promise) RangeError: WebAssembly.instantiate(): Out of memory: Cannot allocate Wasm memory for new instance

And I don't see how I could workaround that by creating a new websocket client in typescript.

I believe it would be the cleanest to support the suggested new url for reconnect: there's no need for a new client for any reasons, so we could simple avoid that, and on_error would also keep working as is now. A much less ideal, but still working solution could be able to remove the on_error handler from typescript, and have a way to delete the client and free up the wasm memory. This way one has to recreate the client in case of an error, refetch all data again, but at least it would work.

@texodus
Copy link
Member Author

texodus commented Jun 19, 2025

We would need to see your implementation, you have quite a lot more going on here well beyond the scope of this (merged, 3mo old) PR. Client doesn't tick on its own, it is continuing to throw errors because you are still calling methods on it in your application somewhere after calling Client::terminate (perhaps on a derived object) - and regardless, creating a new Client instance on disconnect (and enough of them to literally OOM the runtime as they are only a few bytes ...) is not the right way to do whatever it is you are trying to do (e.g. I do not understand what you expect to be the result of using the Client after explicitly terminating it).

Either way, this PR is not the appropriate venue - please reach out to us at Prospective, or you can open an Issue or Discussion if you have a self-contained, isolated reproduction of a specific issue or behavior you think is broken.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Feature requests or improvements

Development

Successfully merging this pull request may close these issues.

Expose DOM WebSocket events close, error, open on WebSocketClient

3 participants