drpcmux: fix server interceptor selection logic #13
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the interceptor selection logic where the wrong criteria was being used to determine which interceptor type to apply. The fix changes the logic to select interceptors based on input type rather than output type, and updates function signatures to remove unnecessary context parameters from stream handlers.
- Changes interceptor selection from using
data.unitary(output type) todata.in1 != streamType(input type) - Removes redundant context parameters from stream handler signatures since context is available through
stream.Context() - Updates parameter names for better clarity (e.g.,
intoreq)
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| drpcmux/interceptor.go | Updates function signatures to remove context parameters from stream handlers and improves parameter naming |
| drpcmux/handle_rpc.go | Fixes the core bug by changing interceptor selection logic from output-based to input-based criteria |
drpcmux/handle_rpc.go
Outdated
| } else if data.in1 == streamType && m.streamInterceptor != nil { | ||
| out, err = m.streamInterceptor(stream, rpc, | ||
| func(st drpc.Stream) (interface{}, error) { | ||
| return data.receiver(data.srv, st.Context(), st, stream) |
There was a problem hiding this comment.
The stream parameter is being passed twice - once as 'st' (the input stream from the handler) and once as 'stream' (the original stream). This could be confusing and may indicate incorrect usage. Verify that both parameters are needed or if one should be removed.
| return data.receiver(data.srv, st.Context(), st, stream) | |
| return data.receiver(data.srv, st.Context(), st) |
Copilot uses AI. Check for mistakes.
shubhamdhama
force-pushed
the
fix-server-interc-selection
branch
from
ef5c556 to
e1fc085
Compare
This commit fixes a bug in the interceptor selection logic in HandleRPC. The issue was that we were passing a stream to the receiver for the case where the input is unitary and the output is a stream. The fix is to receive the message from the stream within the final receiver after going through the stream interceptor pipeline. This also means we no longer receive the message outside the switch statement.
shubhamdhama
force-pushed
the
fix-server-interc-selection
branch
from
e1fc085 to
eb2923d
Compare
|
@shubhamdhama - if you don't have a PR with tests already to cover interceptors logic, please create a tracking ticket. |
PR #14: drpcmux: add tests for server interceptors This PR has it covered. |
This commit fixes a bug in the interceptor selection logic in HandleRPC.
The issue was that we were passing a stream to the receiver for the case
where the input is unitary and the output is a stream.
The fix is to receive the message from the stream within the final receiver
after going through the stream interceptor pipeline. This also means we no
longer receive the message outside the switch statement.