fix(grpc): the unary server interceptor was already set and may not be reset #419
Conversation
There can be only one interceptor set by `grpc.[With](Unary|Stream)Interceptor`, and a runtime error occurs if an interceptor is already defined and a second is attempted using the same API. Switch to using the chaining API, which appends to the existing list, and co-operates cleanly with the non-chained interceptor API. Also, adjust the gRPC test to verify adequate behavior occurs. Fixes #416
RomainMuller
changed the title
| assert.True(t, interceptedDirect.Load(), "original interceptor was not called") | ||
| assert.True(t, interceptedChain.Load(), "original chained interceptor was not called") |
There was a problem hiding this comment.
@RomainMuller Are the messages correct? If intercepted* are true, doesn't it mean they were called? The same for the same messages below.
There was a problem hiding this comment.
The message is for when it fails... But I'm a litle ambivalent as to whether they should mention what should've happened or what was observed that is a failure
There was a problem hiding this comment.
Oh, you are right! I usually go with something like original interceptor expected a call, it didn't happen. Anyway, it's fine as long as it's understandable in the testing logs' context.
| assert.True(t, interceptedDirect.Load(), "original interceptor was not called") | ||
| assert.True(t, interceptedChain.Load(), "original chained interceptor was not called") |
There was a problem hiding this comment.
Oh, you are right! I usually go with something like original interceptor expected a call, it didn't happen. Anyway, it's fine as long as it's understandable in the testing logs' context.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #419 +/- ##
==========================================
+ Coverage 63.97% 64.67% +0.70%
==========================================
Files 180 180
Lines 10894 10924 +30
==========================================
+ Hits 6969 7065 +96
+ Misses 3401 3314 -87
- Partials 524 545 +21
|
There can be only one interceptor set by
grpc.[With](Unary|Stream)Interceptor, and a runtime error occurs if an interceptor is already defined and a second is attempted using the same API. Switch to using the chaining API, which appends to the existing list, and co-operates cleanly with the non-chained interceptor API.Also, adjust the gRPC test to verify adequate behavior occurs.
Fixes #416