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

Support non-standard fields in Response #1349

Open
thebabush opened this issue Apr 14, 2024 · 4 comments
Open

Support non-standard fields in Response #1349

thebabush opened this issue Apr 14, 2024 · 4 comments

Comments

@thebabush
Copy link

Hello,

I'm interacting with a server that sends additional fields in the response message. The fields contain data I care about, so I can't even simply patch jsonrpsee to ignore them (right now jsonerpsee just asserts due to unknown field).

I'm down to make a PR, but I'd like to know if you could guide me through the most painless way to implement this?

Seems like the easiest way would be to extend Response/Notification/etc to have some kind of unknowns: Value, but that would mean dynamic allocations + requiring a feature flag, which is ugly.

Thanks.

@niklasad1
Copy link
Member

niklasad1 commented Apr 21, 2024

Hey hey,

I'm interacting with a server that sends additional fields in the response message. The fields contain data I care about, so I can't even simply patch jsonrpsee to ignore them (right now jsonerpsee just asserts due to unknown field).

Sorry about that I changed the deserialization of the responses to a manual implementation but forgot about that, fixed in #1353

I'm down to make a PR, but I'd like to know if you could guide me through the most painless way to implement this?

Seems like the easiest way would be to extend Response/Notification/etc to have some kind of unknowns: Value, but that would mean dynamic allocations + requiring a feature flag, which is ugly.

What we could is to introduce another field other: HashMap<String, serde_json::value::RawValue> where it's possible for folks to access custom stuff but kind of annoying allocate stuff that may not be used. We were discussing to hide it behind a feature flag but I'm not sure I tend to be reluctant to add more feature flags because it's so tricky to maintain etc.

Thus, I think it's fine to add to another field other: HashMap<String, serde_json::value::RawValue> to the response type.

@thebabush
Copy link
Author

Thanks for replying.

Thus, I think it's fine to add to another field other: HashMap<String, serde_json::value::RawValue> to the response type.

Problem is RawValue is not sized. Would Value work?

Also, I want to get access to the fields, and process_single_response just returns the result. It doesn't seem too easy to change that.

@niklasad1
Copy link
Member

niklasad1 commented Apr 25, 2024

Yeah, you could use either Box<RawValue> or serde_json::Value for that

thebabush added a commit to thebabush/jsonrpsee that referenced this issue Apr 25, 2024
@thebabush
Copy link
Author

Well, here it is: thebabush@84130ec

However, that hashmap is useless atm.

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

No branches or pull requests

2 participants