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

Honor user-provided Json IContractResolver while maintaining marshaling capabilities #783

Merged
merged 7 commits into from
Mar 22, 2022

Conversation

matteo-prosperi
Copy link
Member

@matteo-prosperi matteo-prosperi commented Mar 22, 2022

Addressing failures similar to

StreamJsonRpc.RemoteMethodNotFoundException : Unable to find method 'xxx' on {no object} for the following reasons: Deserializing JSON-RPC argument with name "observer" and position 1 to type "xxx" failed: Could not create an instance of type xxx. Type is an interface or abstract class and cannot be instantiated. Path 'params[1].__jsonrpc_marshaled'.

when calling APIs under the following conditions:

  • using Newtonsoft.Json
  • the API has marshalable (IObserver<>, IDisposable or interfaces with RpcMarshalableAttribute) objects as parameters or return values
  • the user replaces JsonMessageFormatter's ContractResolver.

This issue has worsened since #777 due to more behavior being delegated to the ContractResolver

This PR integrates the ContractResolver provided by the user with MarshalContractResolver before the first serialization or deserialization.

@codecov-commenter
Copy link

Codecov Report

Merging #783 (91adfe6) into main (381dbe8) will increase coverage by 0.04%.
The diff coverage is 100.00%.

❗ Current head 91adfe6 differs from pull request most recent head 8110046. Consider uploading reports for the commit 8110046 to get more accurate results

@@            Coverage Diff             @@
##             main     #783      +/-   ##
==========================================
+ Coverage   91.81%   91.86%   +0.04%     
==========================================
  Files          61       61              
  Lines        5342     5360      +18     
==========================================
+ Hits         4905     4924      +19     
+ Misses        437      436       -1     
Impacted Files Coverage Δ
src/StreamJsonRpc/JsonMessageFormatter.cs 91.57% <100.00%> (+0.23%) ⬆️
src/StreamJsonRpc/MessageHandlerBase.cs 95.18% <0.00%> (-1.21%) ⬇️
src/StreamJsonRpc/JsonRpc.cs 94.03% <0.00%> (+0.26%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 381dbe8...8110046. Read the comment docs.

@matteo-prosperi matteo-prosperi enabled auto-merge (squash) March 22, 2022 22:35
@matteo-prosperi matteo-prosperi merged commit dd9e0c2 into main Mar 22, 2022
@matteo-prosperi matteo-prosperi deleted the dev/maprospe/json_contract_resolver_issue branch March 22, 2022 22:40
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.

3 participants