Skip to content

Conversation

@github-actions
Copy link
Contributor

@github-actions github-actions bot commented Dec 13, 2022

Backport of #45169 to release/7.0

/cc @BrennanConroy

[TS Client] Fix CompletionMessage result when passing false or null

Description

Returning false or "" or 0 would result in an unexpected exception on the server which the user would need to handle, instead it should give the value to the user code.

Fixes #45168

Customer Impact

Customer reported issue. This has been a bug forever, but it hasn't surfaced until now because it's only likely to show up when sending false from the client side in a streaming method, which is very unusual. It's now a lot easier to return a false value due to the "Client results" feature introduced in 7.0, so we should fix the problem in a 7.0 patch.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Small fix and well understood, just an oversight and missed test coverage which are now resolved.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@ghost ghost added the area-signalr Includes: SignalR clients and servers label Dec 13, 2022
@BrennanConroy BrennanConroy requested review from davidfowl and halter73 and removed request for halter73 January 4, 2023 18:51
@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label Jan 4, 2023
@ghost
Copy link

ghost commented Jan 4, 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 added this to the 7.0.x milestone Jan 4, 2023
@BrennanConroy BrennanConroy added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jan 7, 2023
@BrennanConroy BrennanConroy modified the milestones: 7.0.x, 7.0.3 Jan 7, 2023
@ghost
Copy link

ghost commented Jan 7, 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.

@BrennanConroy
Copy link
Member

Servicing approved over email.
@dotnet/aspnet-build please merge 😃

@dougbu dougbu merged commit ce5d73c into release/7.0 Jan 7, 2023
@dougbu dougbu deleted the backport/pr-45169-to-release/7.0 branch January 7, 2023 02:24
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 Servicing-approved Shiproom has approved the issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants