Skip to content

Refactor transient state in Java client #24219

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Sep 18, 2020
Merged

Refactor transient state in Java client #24219

merged 9 commits into from
Sep 18, 2020

Conversation

@BrennanConroy
Copy link
Member

@BrennanConroy BrennanConroy commented Jul 22, 2020 •
edited
Loading

Moved a bunch of transient state from HubConnection to ConnectionState as well as added the ReconnectingConnectionState which makes it easier to manage the state and will help add reconnect in the future.

Description

  • Fix occasional null ref on connection stopping which can crash client app
  • Fix client "falling back" (after first stop) to use WebSockets when LongPolling is explicitly specified
  • Added diagnostic logging for state transitions

Customer Impact

Fixes #24367 which is reported by a customer who's app is crashing from this bug.
No workarounds.

Regression?

No, looks like it's been there since at least 3.0

Risk

Medium, internal state management was changed a bit to make it easier to maintain and to avoid the app crash.

@BrennanConroy BrennanConroy added the area-signalr Includes: SignalR clients and servers label Jul 22, 2020
@BrennanConroy BrennanConroy added this to the 5.0.0-rc1 milestone Jul 22, 2020
BrennanConroy
BrennanConroy commented Jul 23, 2020
View reviewed changes
src/SignalR/clients/java/signalr/src/main/java/com/microsoft/signalr/HubConnection.java Outdated

CompletableSubject start = CompletableSubject.create();
ConnectionState connectionState = new ConnectionState(this);
this.state.setConnectionState(connectionState);
Copy link
Member Author

@BrennanConroy BrennanConroy Jul 23, 2020

Choose a reason for hiding this comment

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

consider setting this later, when the connection is "real"

@BrennanConroy BrennanConroy mentioned this pull request Jul 28, 2020
Closed
@BrennanConroy BrennanConroy modified the milestones: 5.0.0-rc1, 5.0.0-rc2 Aug 25, 2020
@adeelturk
Copy link

adeelturk commented Aug 29, 2020 •
edited
Loading

please merge this as i am struggling alot with my app , and we are close to make decision either to move on to socket.io .

@BrennanConroy BrennanConroy changed the base branch from master to release/5.0-rc2 September 1, 2020 16:33
@BrennanConroy BrennanConroy force-pushed the brecon/javatransient branch 2 times, most recently from d2a287e to 5b86745 Compare September 8, 2020 16:37
@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Sep 14, 2020
@ghost
Copy link

ghost commented Sep 14, 2020

Hello human! Please make sure you've included the Shiproom Template in a comment or (preferably) the PR description. Also, make sure this PR is not marked as a draft and is ready-to-merge.

@leecow leecow added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Sep 15, 2020
@JunTaoLuo JunTaoLuo merged commit 3ee23fb into release/5.0-rc2 Sep 18, 2020
@JunTaoLuo JunTaoLuo deleted the brecon/javatransient branch September 18, 2020 00:59
@github-actions github-actions bot locked and limited conversation to collaborators Dec 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@halter73 halter73 halter73 approved these changes

@wtgodbe wtgodbe Awaiting requested review from wtgodbe

Assignees

No one assigned

Labels

area-signalr Includes: SignalR clients and servers Servicing-approved Shiproom has approved the issue

Projects

None yet

Milestone

5.0.0-rc2

Development

Successfully merging this pull request may close these issues.

[SignalR] java.lang.NullPointerException thrown in HubConnection (Java Client)

6 participants

@BrennanConroy @adeelturk @halter73 @JunTaoLuo @leecow