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

Pass response contexts from RPC middleware to HTTP middleware #1509

Open
shunsukew opened this issue Dec 27, 2024 · 6 comments · May be fixed by #1514
Open

Pass response contexts from RPC middleware to HTTP middleware #1509

shunsukew opened this issue Dec 27, 2024 · 6 comments · May be fixed by #1514

Comments

@shunsukew
Copy link

shunsukew commented Dec 27, 2024

I am developing a rate-limiting RPC middleware. Currently, jsonrpsee framework utilizes MethodResponse Extensions in the RPC layer and http::Response Extensions in the HTTP layer, with these extensions operating independently of each other.

I would like to propagate context information from the RPC layer to the HTTP layer to achieve functionality such as returning a status code other than 200 (429 in my case) or, at the very least, adding an HTTP header to indicate rate limiting based on data in contexts.

What would be the best approach to transfer context (Extensions) information from a jsonrpsee RPC middleware to a hyper HTTP middleware?

@shunsukew shunsukew changed the title Pass context from RPC middleware to HTTP middleware Pass response context from RPC middleware to HTTP middleware Dec 27, 2024
@shunsukew shunsukew changed the title Pass response context from RPC middleware to HTTP middleware Pass response contexts from RPC middleware to HTTP middleware Dec 27, 2024
@niklasad1
Copy link
Member

I think we could copy the extensions from the MethodResponse to the HTTP response to fix that and then you would need some HTTP middleware to check whether some error occurred and set desired HTTP status code on the response.

This shouldn't be that hard to fix

@shunsukew
Copy link
Author

shunsukew commented Dec 29, 2024

That would be really helpful. Thank you so much.
A single RPC call functions as expected, however, batch responses may require merging extension data. Addressing potential conflicts of the same Type data across calls within a batch is a subject for discussion.

@niklasad1
Copy link
Member

niklasad1 commented Jan 8, 2025

Addressing potential conflicts of the same Type data across calls within a batch is a subject for discussion.

Yeah, that one is tricky because the Extensions works based on the rust type ids and it's not possible to merge two types in a clean way I think i.e, just overwrite them with the most recent one...

I had in mind that the user could inject a Vec or another collection that each call RPC call/request can push to it but we can't do that internally in jsonrpsee without some hacks like merging the rust collection types ourselves such as Vec<String>, Vec<usize> and so on...

For now, I don't have a good solution except make user responsible to "inject unique types" because it may be overwritten in a batch but annoying....

@niklasad1 niklasad1 linked a pull request Jan 8, 2025 that will close this issue
@shunsukew
Copy link
Author

shunsukew commented Jan 9, 2025

Thank you so much for updates.

I think overwriting is good practical options under the current situation.
One idea I have is jsonrpsee introduces new definition RpcExtensions and only insert this type to Http middleware extensions regardless whether it is single or batch, where RpcExtensions is a map of call id and AnyMap.

If users want to use AnyMap from jsonrpsee in hyper http layer, they can iterate over RpcExtensions.

@niklasad1
Copy link
Member

niklasad1 commented Jan 9, 2025

One idea I have is jsonrpsee introduces new definition RpcExtensions and only insert this type to Http middleware extensions regardless whether it is single or batch, where RpcExtensions is a map of call id and AnyMap.

If users want to use AnyMap from jsonrpsee in hyper http layer, they can iterate over RpcExtensions.

Yes, that would work but I propose we go ahead with this simple approach first i.e, just overwrite but worth looking into replacing the http::Extensions for a nicer API for batches such for users to append to that collection instead of overwriting.

@shunsukew
Copy link
Author

shunsukew commented Jan 9, 2025

I agree. My implementation now depends on your PR branch and working good. Thank you so much

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 a pull request may close this issue.

2 participants