-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
fix: send notifications when UK database is down (#6498) #6596
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
base: master
Are you sure you want to change the base?
fix: send notifications when UK database is down (#6498) #6596
Conversation
Implement notification system that works even when database is unavailable. Problem: - When Uptime Kuma's database (external MariaDB/SQLite) becomes unavailable, UK stops functioning and cannot send notifications about its own failure. - Users are not alerted when UK itself is having database connectivity issues. Solution: - Added notification cache system that stores all active notification configs in memory when database is available - Implemented database error detection in both monitor.js and server.js error handlers to catch EHOSTUNREACH, ECONNREFUSED, and other DB errors - Added sendDatabaseDownNotification() method that uses cached notifications to send alerts when database connection fails - Cache automatically refreshes periodically (every 30 minutes) and when notifications are added/updated/deleted - Prevents duplicate notifications for the same database down event Changes: - server/notification.js: Added cache system and database down notification - server/server.js: Enhanced error handler to detect DB errors and trigger notifications, refresh cache on startup - server/model/monitor.js: Added DB error detection in safeBeat error handler - server/jobs.js: Added periodic cache refresh job (every 30 minutes) - test/backend-test/test-database-down-notification.js: Comprehensive test suite covering cache, notifications, error handling This ensures users are always notified when UK itself is having database connectivity issues, even if the database is completely unavailable.
|
hi @CommanderStorm , could you review my pr? |
|
all of the tests passed :) @CommanderStorm |
CommanderStorm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Architecturally, we need to move this to its own file.
Adding new, larger features to the notification.js or monitor.js are not the best of ideas. (the files for this are alreay over 1k lines, which is way too large).
The code is also a bit too spaced out and duplicated and I don't think it is currently quite to the point that it is production ready in terms of "this cannot cause very alarming false positives".
Could you give it another iteration?
louislam
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to think more.
- Will it send to all notification providers? If yes, I don't think it should.
- Need GUI to config which notification providers want to receive this.
- Turn off by default
- Will it send only one notification during a period, or it keep sending notifications as long as it hit the errors? It may be a problem that it can strike the notification providers (rate limit or sms pricing etc.)
- Add send_database_down column to notification table (default: false) - Only notifications with send_database_down enabled receive database down alerts - Add GUI checkbox in NotificationDialog to configure opt-in setting - Implement 24-hour cooldown period to prevent notification spam - Add database error detector to avoid false positives - Update tests to verify opt-in behavior Addresses owner feedback: - Not sending to all providers (opt-in only) - GUI configuration available - Turned off by default - Cooldown prevents spam/rate limiting issues
- Check for missing is_default column and add it if needed - Handle outdated template database in CI/CD environment - Ensures test works even if template DB is missing columns
…martDever02/uptime-kuma into fix/database-down-notification
|
Honestly, I don't think this code or the way it is integrated would be maintainable long term. I am tempted to just close this feature and its feature request since this really would need some architectural changes to work well. |
1 similar comment
|
Honestly, I don't think this code or the way it is integrated would be maintainable long term. I am tempted to just close this feature and its feature request since this really would need some architectural changes to work well. |
Implement notification system that works even when database is unavailable.
Problem:
Solution:
Changes:
This ensures users are always notified when UK itself is having database connectivity issues, even if the database is completely unavailable.
Closes #6498
Contribution by Gittensor, see my contribution statistics at https://gittensor.io/miners/details?githubId=102175066