search

dfordivam / reflex-websocket-interface

A simple to use interface for websocket using reflex-frp
GNU General Public License v3.0
19 stars 4 forks source link

Request tracking #8

Closed mulderr closed 3 years ago

mulderr commented 7 years ago

Hi,

first of, thank you for publishing this! It's been a huge help.

Second, I have a question, a bit of a nitpick really, regarding how the library handles state associated with open requests client side.

If I read things correctly, basically we're keeping a Map Int (RequesterDataKey a) that allows us to track open requests and map Int identifiers we can serialize and send over the network to opaque tags produced by requester. So that way, when a response comes back we know how to find the original RequesterDataKey that we need to give back to Requester.

What happens to requests that produce no response from the server for whatever reason?

Currently I guess the request map is not "GCed" in any way. Nor is there a way to force cleanup if one detects such a condition by other means. It wouldn't be a problem under normal conditions but do you think it would be good to have a configurable timeout and periodically clean the request map or another mechanism maybe?

Or is there a reason this is non-issue that I'm not seeing right now? It seems in a particularly nasty case these request could stack over a long period of time and affect performance here at the very least. Of course then you have other problems.

Surely there must be a way to this without potentially leaking memory.

dfordivam commented 7 years ago

One way is to add a timeout ..

Can be implemented using the delay api...

mulderr commented 7 years ago

Just realized RequesterT itself has the same issue internally. How about we not only give an option to time out requests but also give a mock timeout response back to requester when a timeout indeed happens. This way not only RequesterT gets the response it's expecting but also the request origin knows whats what happened.

However that changes the semantics a bit? Now we need to know how to construct a timeout response whereas we didn't need that knowledge before.

dfordivam commented 7 years ago

Yes this can be done, by proving a wrapper over the response types.

Since this will require the user to handle this special response type, may be a separate set of APIs with these added functionality can be provided.

Otherwise the current interface can implement a naive timeout without any change in the interface...

mulderr commented 3 years ago

No longer need this and it's been some time. I'll go ahead and close.