Notification rate limiting#110
Conversation
- Add batching system to prevent notification storms (3s rate limit, 5s batch window) - Queue rapid notifications and show batch summaries - Debug logs show device names for adverts, sender/channel for messages (no content leaks) - Remove unused _maxBatchSize constant Context: Added after getting notification-flooded while evaluating RF flood management. The irony.
|
@rsp2k looks like there are some conflicts, can you please review and mark ready for review after you have done so? Thanks very much 😎 |
|
I'll try it out. thanks for making the PR. |
I made a mistake and removed this
|
Will need to add the text strings to the l10n translations system, to the fixed text. |
Resolved conflict in notification_service.dart: - Kept _getNotificationIdentifier (needed for rate limiting debug logs) - Dropped _truncateMessage (main simplified channel message preview)
Addresses PR zjs81#110 review feedback to use the translations system: - Add notification strings to app_en.arb (plurals for batch summary) - Update NotificationService to use lookupAppLocalizations() - Wire locale from MaterialApp to NotificationService - Regenerate localization files New strings added (English only, translations needed): - notification_activityTitle: "MeshCore Activity" - notification_messagesCount: "{count} message(s)" - notification_channelMessagesCount: "{count} channel message(s)" - notification_newNodesCount: "{count} new node(s)" - notification_newTypeDiscovered: "New {type} discovered" - notification_receivedNewMessage: "Received new message"
# Conflicts: # lib/services/notification_service.dart
Translated notification_activityTitle, notification_messagesCount, notification_channelMessagesCount, notification_newNodesCount, notification_newTypeDiscovered, and notification_receivedNewMessage to: bg, de, es, fr, it, nl, pl, pt, ru, sk, sl, sv, uk, zh Includes proper ICU plural forms for Slavic languages (few/many/other) and Slovenian dual form.
|
Addressed the l10n feedback - notification strings are now integrated into the translations system. Changes:
Includes proper ICU plural handling for Slavic languages (few/many/other forms) and Slovenian dual form. Build passes |
|
Can you please also run dart format on your changes? 😊 |
|
Ya I just didn't know to do that. |
446564
left a comment
There was a problem hiding this comment.
It feels odd that only the last message in a channel is in the notification, but I suppose you can't have them all.
I haven't noticed any issues running this and scanned over the code.
But it looks good to me, and when there are too many like on reconnect it aggregates into a count.
Resolves conflicts in l10n files by keeping both: - notification_* strings (from dev/PR zjs81#110) - settings_gpxExport* strings (from main)
First time I connected to the Idaho mesh, the app blasted me with 29 separate notifications - one for every node already on the network. Kinda ironic that the flood management protocol flooded my phone.
Now it batches rapid notifications with a 5-second window and shows a single summary ("29 new nodes") instead. Added debug logging that shows device names without exposing message content, so logs are safe to share in bug reports.