-
Notifications
You must be signed in to change notification settings - Fork 13.1k
[FIX] Uncaught error by listening stream-notify-room without subscription #27020
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
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #27020 +/- ##
===========================================
+ Coverage 40.31% 40.95% +0.64%
===========================================
Files 827 802 -25
Lines 18250 17819 -431
Branches 2031 1972 -59
===========================================
- Hits 7357 7298 -59
+ Misses 10598 10228 -370
+ Partials 295 293 -2
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
@sampaiodiego you're more familiar with this code, can you help us reviewing? 🙏 |
apps/meteor/server/modules/notifications/notifications.module.ts
Outdated
Show resolved
Hide resolved
|
can you also please create a test for this fix? |
The cases where this error could impact the end user (i.e. anonymous viewing a room) have more things to be fixed, therefore a successful test might not be possible without expanding the PR scope a little bit.
Proposed changes (including videos or screenshots)
fix this:

Issue(s)
Steps to test or reproduce
Further comments