fix: server crash when re-enabling push notifications#41341
Conversation
Re-enabling the Push_enable setting called Push.configure() a second time, which throws by design and crashed the server via an unhandled promise rejection in the settings watcher. Push is now unconfigured when the setting is disabled (APN connection shut down, configured flag reset), so re-enabling runs a clean configure again.
|
Looks like this PR is ready to merge! 🎉 |
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🦋 Changeset detectedLatest commit: b50984b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
A rejected shutdown promise behind void would itself become an unhandled rejection. Await it wrapped in try/catch; connection reference is still cleared synchronously.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #41341 +/- ##
===========================================
- Coverage 69.11% 69.08% -0.04%
===========================================
Files 3759 3759
Lines 147859 147885 +26
Branches 26420 26397 -23
===========================================
- Hits 102196 102169 -27
- Misses 41179 41228 +49
- Partials 4484 4488 +4
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
Proposed changes (including videos or screenshots)
Toggling the
Push_enablesetting off and back on crashed the server:The
Push_enablesettings watcher callsPush.configure()on every enable, butconfigure()throws by design when called twice. The throw inside the async watcher callback becomes an unhandled promise rejection, which crashes the process whenEXIT_UNHANDLEDPROMISEREJECTIONis set.Push is now unconfigured when the setting is disabled: the APN connection is shut down (
shutdownAPN()) and theisConfiguredflag is reset, so re-enabling runs a cleanconfigure()again. As a bonus, settings changed while push was disabled (certs, gateway) are picked up on re-enable, and no stale APN connection stays alive while push is disabled. The double-configure guard remains in place to catch genuine programming errors.Issue(s)
Steps to test or reproduce
Push_enable)UnHandledPromiseRejection: Configure should not be called more than once!(and exits whenEXIT_UNHANDLEDPROMISEREJECTIONis set). After: setting toggles cleanly, log linePush service disabled: shutting down existing connectionsappears on disable.Further comments
Unit tests added for
shutdownAPNinapn.spec.ts.Summary by CodeRabbit
https://rocketchat.atlassian.net/browse/CORE-64