-
Notifications
You must be signed in to change notification settings - Fork 108
Add support for nested gRPC callouts. #240
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
Conversation
1c6bb0b
to
6991ce7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! This is a great find (and something that we've missed in #56, EDIT: because it predates gRPC support added in #100 and #101).
However, this fix is incomplete, since it only covers on_grpc_receive()
callback, and only for gRPC calls and not for gRPC streams.
Could you also fix the case for nested gRPC streams in on_grpc_receive()
and for nested gRPC calls and streams in on_grpc_close()
?
4606852
to
cd53a9e
Compare
Thanks for reviewing this, @PiotrSikora. I just added fixes for nested gRPC streams and |
d488c1f
to
8429900
Compare
…_stream() from within gRPC event listeners. Signed-off-by: andytesti <atesti@salesforce.com>
8429900
to
3085e79
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
Signed-off-by: andytesti <atesti@salesforce.com>
Signed-off-by: andytesti <atesti@salesforce.com>
Panic with message "proxy-wasm-rust-sdk/src/dispatcher.rs:121:14:\nalready borrowed: BorrowMutError" is triggered when
dispatch_grpc_call()
is invoked fromon_grpc_call_response()
. The problem is originated by aself.grpc_callouts.borrow_mut()
being invoked inside anif
condition, that keeps theRefMut
alive until thetrue
branch ends.This PR adopts the same approach than on_http_call_response(), by invoking the
RefCell::borrow_mut()
outside theif
condition.