Skip to content

[release/7.0] Fix sending to another client from OnConnectedAsync #47204

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 2 commits into from
Apr 4, 2023
Merged

[release/7.0] Fix sending to another client from OnConnectedAsync #47204

merged 2 commits into from
Apr 4, 2023

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Mar 14, 2023 •
edited by BrennanConroy
Loading

Backport of #47203 to release/7.0

/cc @BrennanConroy

Fix sending to another client from OnConnectedAsync

Description

If you send a message to a client (not the currently connecting client) from OnConnectedAsync it will send to the currently connecting client instead.

Fixes #47189

Customer Impact

Issue reported by customer. This breaks their scenario of sending to another connection when a new client connects.

Worked in 6.0 (and older versions), broke in 7.0.

Regression?

  • Yes
  • No

Regressed in 7.0

Risk

  • High
  • Medium
  • Low

Single character fix, was a typo/copy-paste error. Added new test for the broken scenario.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • N/A

@ghost ghost added the area-signalr Includes: SignalR clients and servers label Mar 14, 2023
@ghost ghost added this to the 7.0.x milestone Mar 14, 2023
@ghost
Copy link

ghost commented Mar 14, 2023

Hi @github-actions[bot]. If this is not a tell-mode PR, please make sure to follow the instructions laid out in the servicing process document.
Otherwise, please add tell-mode label.

@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Mar 14, 2023
@ghost
Copy link

ghost commented Mar 14, 2023

Hi @github-actions[bot]. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

BrennanConroy
BrennanConroy reviewed Mar 14, 2023
View reviewed changes
@BrennanConroy BrennanConroy added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Mar 15, 2023
@ghost
Copy link

ghost commented Mar 15, 2023

Hi @github-actions[bot]. This PR was just approved to be included in the upcoming servicing release. Somebody from the @dotnet/aspnet-build team will get it merged when the branches are open. Until then, please make sure all the CI checks pass and the PR is reviewed.

@sebastienros
Copy link
Member

sebastienros commented Mar 15, 2023

I believe this one has to be held off and wait for the next window?

@wtgodbe
Copy link
Member

wtgodbe commented Mar 15, 2023

I believe this one has to be held off and wait for the next window?

Correct

@RussKie
Copy link
Contributor

RussKie commented Mar 22, 2023

We expect to merge this in early April (when branches open), right?

@wtgodbe
Copy link
Member

wtgodbe commented Mar 22, 2023

We expect to merge this in early April (when branches open), right?

Yes

@ghost
Copy link

ghost commented Mar 30, 2023

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no breaking changes are introduced, please leave an /azp run comment here to rerun the CI pipeline and confirm success before merging the change.

@ghost ghost added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Mar 30, 2023
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Apr 4, 2023
@ghost ghost added the area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework label Apr 4, 2023
@wtgodbe
Copy link
Member

wtgodbe commented Apr 4, 2023

/azp run

@azure-pipelines
Copy link

azure-pipelines bot commented Apr 4, 2023

Azure Pipelines successfully started running 3 pipeline(s).

@wtgodbe wtgodbe merged commit 569741c into release/7.0 Apr 4, 2023
@wtgodbe wtgodbe deleted the backport/pr-47203-to-release/7.0 branch April 4, 2023 21:18
@ghost ghost modified the milestones: 7.0.x, 7.0.6 Apr 4, 2023
@rbhanda rbhanda modified the milestones: 7.0.6, 7.0.7 Jun 1, 2023
@BrennanConroy BrennanConroy mentioned this pull request Jun 9, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@BrennanConroy BrennanConroy BrennanConroy left review comments

@captainsafia captainsafia captainsafia approved these changes

@halter73 halter73 Awaiting requested review from halter73

Assignees

No one assigned

Labels

area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework area-signalr Includes: SignalR clients and servers Servicing-approved Shiproom has approved the issue

Projects

None yet

Milestone

7.0.7

Development

Successfully merging this pull request may close these issues.

7 participants

@sebastienros @wtgodbe @RussKie @captainsafia @BrennanConroy @rbhanda