merge v2 to master #180
merge v2 to master #180
Conversation
This is temporary fix to work with the `server` which assumes that `WebSocket` resonses are `text`
When the server receives an request with invalid JSON `-32700, Parse error shall be returned`
`.await` was missing in RegisteredSubscription::send() and no responses were actually sent which this commit fixes.
Hostname is required when using `wss` and `127.0.0.1` is not valid hostname.
Fixes newly introduced bug that causes `register_subscription` to have side-effects even if the subscription fails.
* [ws server]: don't close conn. when `deser` fails * Update src/ws/transport.rs * grumbles: prefer matching of if else.
|
LGTM. |
examples/subscription.rs
Outdated
| async fn run_server(server_started_tx: Sender<()>, url: &str) { | ||
| let server = WsServer::new(url).await.unwrap(); | ||
| let mut subscription = | ||
| server.register_subscription("subscribe_hello".to_string(), "unsubscribe_hello".to_string()).unwrap(); |
There was a problem hiding this comment.
Is it better to rename this file to ws_subscription? Though http1.1 don't suppport subscription.
| /// server automatically returns an "internal error" to the client. | ||
| /// | ||
| /// Returns an error if the method name was already registered. | ||
| pub fn register_method(&self, method_name: String) -> Result<RegisteredMethod, Error> { |
There was a problem hiding this comment.
Whether register_method should provide generic parameters, such as T: Into>. The same goes for some other similar methods
|
|
||
| server_started_tx.send(()).unwrap(); | ||
| loop { | ||
| let r = say_hello.next().await; |
There was a problem hiding this comment.
Provide a channel for each rpc method, I want to know if this programming model is reasonable
| } | ||
|
|
||
| log::trace!("[frontend]: register_method={}", method_name); | ||
| let (tx, rx) = mpsc::channel(32); |
There was a problem hiding this comment.
Whether similar parameters should be configurable, I think this kind of parameters may affect performance and availability
|
Thanks for reviewing, your comments are legit but I will not address them in this PR, the server implementations are under refactoring/on going work thus will likely change. |
|
//cc @dvdplm needs another review ^^ |
No description provided.