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
Fix a bug where an IllegalStateException
is raised on a non-ready EndpointGroup
#3338
Conversation
…EndpointGroup` Motivation: If an `EndpointGroup` is not initialized with initial values, a gRPC client will raise an `IllegalStateException` with the following message: ```java io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1HalfClosed@3830513a java.lang.IllegalStateException: Should call init(endpoint) before invoking this method. ``` A `WebClient` waits a connection timeout for the `EndpointGroup` to have at least one Endpoint. However, a gRPC client does not wait for the `EndPointGroup` and start a gRPC request immediately. Modifications: - Add `DefaultClientRequestContext.whenIntialized()` which completes when a `ctx` is initialized with an `EndpointGroup`. - Defer a gRPC call until a `whenIntialized()` is completed. Result: - You no longer see an `IllegalStateException` if a gRPC call is sent to a non-ready `EndpointGroup` - Fixes line#3332
Codecov Report
@@ Coverage Diff @@
## master #3338 +/- ##
============================================
- Coverage 74.17% 74.13% -0.05%
- Complexity 13255 13269 +14
============================================
Files 1158 1158
Lines 50536 50597 +61
Branches 6469 6485 +16
============================================
+ Hits 37483 37508 +25
- Misses 9743 9774 +31
- Partials 3310 3315 +5 Continue to review full report at Codecov.
|
core/src/main/java/com/linecorp/armeria/client/DefaultClientRequestContext.java
Show resolved
Hide resolved
grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java
Outdated
Show resolved
Hide resolved
grpc/src/main/java/com/linecorp/armeria/internal/client/grpc/ArmeriaClientCall.java
Outdated
Show resolved
Hide resolved
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!
core/src/main/java/com/linecorp/armeria/client/DefaultClientRequestContext.java
Show resolved
Hide resolved
} | ||
|
||
private void execute(Runnable task) { | ||
if (endpointInitialized || ctx.whenInitialized().isDone()) { |
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.
We could use assert
for this condition if we always guard this method with needsDirectInvocation()
.
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.
endpointInitialized
could be either true
, false
here.
Because endpointInitialized || ctx.whenInitialized().isDone()
and ctx.eventLoop().inEventLoop()
are connected by &&
condition in needsDirectInvocation()
.
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.
Ah, indeed. Sorry for a bad comment 😅
IllegalStateException
is raised on a non-ready `…IllegalStateException
is raised on a non-ready EndpointGroup
Thanks, @ikhoon! |
…EndpointGroup`
Motivation:
If an
EndpointGroup
is not initialized with initial values,a gRPC client will raise an
IllegalStateException
with the following message:A
WebClient
waits a connection timeout for theEndpointGroup
to have at least one Endpoint by @trustin's work (#2837)However, a gRPC client does not wait for the
EndPointGroup
and start a gRPC request immediately.Modifications:
DefaultClientRequestContext.whenIntialized()
which completeswhen a
ctx
is initialized with anEndpointGroup
.whenIntialized()
is completed.Result:
IllegalStateException
if a gRPC call is sent to a non-readyEndpointGroup
IllegalStateException
when making a gRPC call to a non-readyEndpointGroup
. #3332