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

Add Headers support for WebSocketMessageHandler #98

Closed
yingfangdu opened this issue May 1, 2018 · 3 comments
Closed

Add Headers support for WebSocketMessageHandler #98

yingfangdu opened this issue May 1, 2018 · 3 comments
Labels

Comments

@yingfangdu
Copy link

I am using vscode-ws-jsonrpc from client side and using vs-streamjsonrpc from server side. The communication is through WebSocket.

While vscode-ws-jsonrpc always expect headers in the message (sending and receiving)

Could you please abstract the Headers feature from HeaderDelimitedMessageHandler and make it to be configurable for these message handlers so that we can enable /disable as needed for different jsonrpc libs?

@AArnott
Copy link
Member

AArnott commented May 2, 2018

That seems reasonable.
But I wonder: why does vscode-ws-jsonrpc use headers? Web sockets provide message boundaries in the transport, so I found that the headers were not necessary.

@yingfangdu
Copy link
Author

I think the solution is to let vs-jsonrpc to remove the headers as the header is not necessary at all.
Adding a header does not follow the jsonrpc protocol.

While for StreamJsonRPC, in order to work with other library, I think the header should be configurable.

@AArnott
Copy link
Member

AArnott commented Jun 26, 2018

You can do this today by exposing your WebSocket as a stream and then using the HeaderDelimitedMessageHandler class (which is used by default for streams). WebSocket doesn't offer an AsStream() extension method, unfortunately. But it's not too hard to write such a wrapper class. My team has an implementation of this that is ~150 lines.

If #80 is fixed, that may allow us to offer HeaderDelimitedMessageHandler for WebSocket as well.

@AArnott AArnott closed this as completed Jun 26, 2018
AArnott pushed a commit that referenced this issue Apr 12, 2022
Bumps [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest) from 16.9.1 to 16.9.4.
- [Release notes](https://github.com/microsoft/vstest/releases)
- [Commits](microsoft/vstest@v16.9.1...v16.9.4)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants