Skip to content
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.

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

Fixes + unit tests for streaming PubSub implementation #1415

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

WhitWaldo
Copy link
Contributor

Description

Fixed the linked issue which called out the following problems:

  • Sometimes inbound messages from the Dapr runtime are null which cause a NullReferenceError during delegate processing. This has been updated to add a null check and skip such messages.
  • The implementation used an await Task.WhenAny which caused the whole of the operation to be blocking. This was unintentional and has been corrected.

While fixing these, I realized that the project was devoid of several useful unit tests that could ensure that the rest of the class was operating as it should, so I took the opportunity to add a few internal fields and methods (available only to the test project) that would assist in some of this validation.

Finally, I modified the AddDaprPubSubClient extension method to pass an IConfiguration? into the DaprPublishSubscribeClientBuilder so it can optionally source the various configuration-based values from there during registration.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1412

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • [N/A] Extended the documentation - done in another issue

@WhitWaldo WhitWaldo added this to the v1.15 milestone Nov 28, 2024
@WhitWaldo WhitWaldo self-assigned this Nov 28, 2024
@WhitWaldo WhitWaldo requested review from a team as code owners November 28, 2024 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NullReferenceException when trying to sub with streaming subscriptions
1 participant