[TS Client] Fix CompletionMessage result when passing false or null #45169
Conversation
BrennanConroy
added
area-signalr
|
Or other javascript falsy values like |
Isn't javascript great... 😆 |
|
|
||
| private _writeCompletion(completionMessage: CompletionMessage): ArrayBuffer { | ||
| const resultKind = completionMessage.error ? this._errorResult : completionMessage.result ? this._nonVoidResult : this._voidResult; | ||
| const resultKind = completionMessage.error ? this._errorResult : |
There was a problem hiding this comment.
What if completionMessage.error is an empty string instead of undefined or null? Should that be treated as an error result still?
There was a problem hiding this comment.
Yeah, any non-null value should be considered an error. I don't think there is anyway today for an empty string to be set, but even if it was that should be considered an error.
In theory we should throw for both error and result being set, but since I want to backport this change, and we control the creation of completion messages it would basically only be an assert, I'm not going to add that.
ghost
added this to the
|
Offline sign-off from @halter73 |
|
Hi @amcasey. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context. |
|
/backport to release/7.0 |
|
Started backporting to release/7.0: https://github.com/dotnet/aspnetcore/actions/runs/3681657067 |
Fixes #45168
Our check for if a result existed for a completion message didn't handle
falseornullresults.