Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ws server]: use send text instead of send binary #374

Merged
merged 1 commit into from
Jun 10, 2021
Merged

Conversation

niklasad1
Copy link
Member

Basically because some of common libraries seems to assume that text is the way to go.

@jsdw
Copy link
Collaborator

jsdw commented Jun 10, 2021

Perhaps unrelated to this PR but; I ran the tests and, while everything passed, I also saw some errors logged to the console that may be related to the change in how messages are being sent?:

test access_control::cors::tests::should_not_allow_partially_matching_origin ... ok
[2021-06-10T10:17:03Z ERROR jsonrpsee_http_server::server] [service_fn], Cannot parse request body="{\"jsonrpc\":\"2.0\",\"method\":\"say_hello\",\"id\"}"
test tests::invalid_json_id_missing_value ... ok
[2021-06-10T10:17:03Z ERROR jsonrpsee_http_server::server] [service_fn], Cannot parse request body="{\"jsonrpc\":\"2.0\",\"method\":\"bar\",\"id\":1,\"is_not_request_object\":1}"
[2021-06-10T10:17:03Z ERROR jsonrpsee_http_server::server] [service_fn], Cannot parse request body="[123]"
test tests::invalid_request_object ... ok
[2021-06-10T10:17:03Z ERROR jsonrpsee_http_server::server] [service_fn], Cannot parse request body="{\"jsonrpc\":\"2.0\",\"method\":1, \"params\": \"bar\"}"
[2021-06-10T10:17:03Z ERROR jsonrpsee_http_server::server] [service_fn], Cannot parse request body="[1, 2, 3]"
test tests::invalid_single_method_call ... ok
test tests::notif_works ... ok
[2021-06-10T10:17:03Z ERROR jsonrpsee_http_server::server] [service_fn], Cannot parse request body="[\n\t\t{\"jsonrpc\": \"2.0\", \"method\": \"sum\", \"params\": [1,2,4], \"id\": \"1\"},\n\t\t{\"jsonrpc\": \"2.0\", \"method\"\n\t  ]"

@niklasad1
Copy link
Member Author

niklasad1 commented Jun 10, 2021

Perhaps unrelated to this PR but; I ran the tests and, while everything passed, I also saw some errors logged to the console that may be related:

It's just because we test with malformed requests to see that they are handled "correctly" and cargo test is multi-threaded by default

@jsdw
Copy link
Collaborator

jsdw commented Jun 10, 2021

Ah, looking at the test names that makes sense!

@dvdplm dvdplm merged commit 9a02c10 into master Jun 10, 2021
@dvdplm dvdplm deleted the na-send-text branch June 10, 2021 10:23
dvdplm added a commit that referenced this pull request Jul 6, 2021
* master: (21 commits)
  New proc macro (#387)
  Streaming RpcParams parsing (#401)
  Set allowed Host header values (#399)
  Synchronization-less async connections in ws-server (#388)
  [ws server]: terminate already established connection(s) when the server is stopped (#396)
  feat: customizable JSON-RPC error codes via new enum variant on `CallErrror` (#394)
  [ci]: test each individual crate's manifest (#392)
  Add a way to stop servers (#386)
  [jsonrpsee types]: unify a couple of types + more tests (#389)
  Update roadmap link in readme (#390)
  Cross-origin protection (#375)
  Method aliases + RpcModule: Clone (#383)
  Use criterion's async bencher (#385)
  Async/subscription benches (#372)
  send text (#374)
  Fix link to ws server in README.md (#373)
  Concat -> simple push (#370)
  Add missing `rt` feature (#369)
  Release prep for v0.2 (#368)
  chore(scripts): publish script (#354)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants