Skip to content

Conversation

@BrennanConroy
Copy link
Member

Fixes #31491

We should backport this to 5.0.

@BrennanConroy BrennanConroy added area-signalr Includes: SignalR clients and servers feature-client-java Related to the SignalR Java client labels Apr 13, 2021
@BrennanConroy BrennanConroy added this to the 6.0-preview4 milestone Apr 13, 2021

public void timeoutHandshakeResponse(long timeout, TimeUnit unit) {
handshakeTimeout = Executors.newSingleThreadScheduledExecutor();
handshakeTimeout.schedule(() -> {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious why this uses an executor to implement the timeout instead of using RxJava's timeout operator?
connectionState.transport.send(handshake).timeout(..., ...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably implemented before RxJava was used.

Keeping it as-is so it's easier to backport to 5.0

private void errorHandshake(Exception error) {
lock.lock();
try {
// If onError is called on a completed subject the global error handler is called
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way we could hook the global error handler with our tests and fail if it ever gets called? Would that have been able to catch this?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pointer, adapted something similar for junit 5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-signalr Includes: SignalR clients and servers feature-client-java Related to the SignalR Java client

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SignalR Java SDK throwing UndeliverableException

4 participants