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.

Sign up for GitHub

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

Jump to bottom

Better handling of failures to serialize RPC method return value #549

Closed
tmat opened this issue Sep 16, 2020 · 1 comment · Fixed by #555
Closed

Better handling of failures to serialize RPC method return value #549

tmat opened this issue Sep 16, 2020 · 1 comment · Fixed by #555
Assignees
@AArnott
Milestone
v2.6

Comments

@tmat
Copy link
Member

tmat commented Sep 16, 2020

When the return value of an RPC method fails to serialize the server drops the connection and the client only sees ConnectionLostException without access to the underlying issue (serialization exception).

It would be better if the client could somehow receive the serialization exception.
@AArnott AArnott self-assigned this Sep 16, 2020
@AArnott AArnott added this to the v2.6 milestone Sep 18, 2020
AArnott added a commit to AArnott/vs-streamjsonrpc that referenced this issue Sep 18, 2020
@AArnott
Send error to client when server fails to serialize response
15f7efd 
Previous to this change, a failure to serialize a response would lead the server to terminate the connection (since a response is required). With this change, we instead keep the connection going but send an error response to the client with the serialization error.

We also (still) log the serialization failure on the server.

Fixes microsoft#549
@AArnott
Copy link
Member

AArnott commented Sep 18, 2020

I'll create a PR for this once #554 goes in, since the fix for this builds on that.

AArnott added a commit to AArnott/vs-streamjsonrpc that referenced this issue Sep 18, 2020
@AArnott
Send error to client when server fails to serialize response
18846ae 
Previous to this change, a failure to serialize a response would lead the server to terminate the connection (since a response is required). With this change, we instead keep the connection going but send an error response to the client with the serialization error.

We also (still) log the serialization failure on the server.

Fixes microsoft#549
@AArnott AArnott mentioned this issue Sep 18, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AArnott AArnott

Labels
None yet
Projects
None yet
Milestone
v2.6
Development

Successfully merging a pull request may close this issue.

Send error to client when server fails to serialize response AArnott/vs-streamjsonrpc
2 participants
@AArnott @tmat