Previously, we encountered a service could get overloaded due to many LSPSMessage::Invalid being sent if speaking to an incompatible client. Here, we make several changes to fix this behavior and generally improve performance and fix some oversights in message serialization. Most notably:
We ensure we disconnect if we fail to parse a message. Could be debatable if we want to do this on the first incident, but for now we should be fine with being strict here.
We simplify the LSPSMessageVisitor logic a bit
We avoid inserting anything to our request_id_to_method_map if we receive a request, as remembering this mapping is only required client side.
Rather than just retrieving (get) said mapping client-side, we remove it once we receive the response, stopping the map from growing indefinitely.
We fix LSPS1 serialization which previously used wrong field keys in the JSON-RPC object
We add handling for GetInfoResponses in LSPS1, which was previously overlooked
We introduce a LSPSMethod enum and track everything by this enum and RequestId rather than as (String, String), which is more efficient and allows the compiler to typecheck more things.
We fix handling of LSPSMessage::Invalid, i.e., now properly parsing the errors rather than erroring and hence sending one in return.
Finally, we move all serialization types out of lsps0/msgs.rs to lsps0/ser.rs, which generally separates concerns better and allows LSPS0 to mirror the structure of LSPS1/LSPS2, i.e., have only its actual message types in msgs.rs.
Note to reviewers: as this PR is rather invasive as a whole (in particular the last commit), I'd recommend reviewing commit-by-commit, which should be much more digestible.
Previously, we encountered a service could get overloaded due to many
LSPSMessage::Invalid
being sent if speaking to an incompatible client. Here, we make several changes to fix this behavior and generally improve performance and fix some oversights in message serialization. Most notably:LSPSMessageVisitor
logic a bitrequest_id_to_method_map
if we receive a request, as remembering this mapping is only required client side.get
) said mapping client-side, weremove
it once we receive the response, stopping the map from growing indefinitely.GetInfoResponses
in LSPS1, which was previously overlookedLSPSMethod
enum and track everything by this enum andRequestId
rather than as(String, String)
, which is more efficient and allows the compiler to typecheck more things.LSPSMessage::Invalid
, i.e., now properly parsing the errors rather than erroring and hence sending one in return.lsps0/msgs.rs
tolsps0/ser.rs
, which generally separates concerns better and allows LSPS0 to mirror the structure of LSPS1/LSPS2, i.e., have only its actual message types inmsgs.rs
.Note to reviewers: as this PR is rather invasive as a whole (in particular the last commit), I'd recommend reviewing commit-by-commit, which should be much more digestible.