fix: reset UAID if too many messages are pending #728
Conversation
Current coverage is 100% (diff: 100%)@@ master #728 diff @@
=====================================
Files 48 48
Lines 10146 10184 +38
Methods 0 0
Messages 0 0
Branches 0 0
=====================================
+ Hits 10146 10184 +38
Misses 0 0
Partials 0 0
|
| """errBack for handling excessive messages per UAID""" | ||
| fail.trap(MessageOverloadException) | ||
| self.ap_settings.router.drop_user(self.ps.uaid) | ||
| self.send_close() |
There was a problem hiding this comment.
Ah, it's sendClose. Odd that it didn't get flagged in the tests.
There was a problem hiding this comment.
maybe we should make websocket tests' tearDown fail the test if a log.failure is called and wasn't marked as being expected
since websocket ends up eating exceptions and just closing the connection in some cases
| @@ -145,6 +145,9 @@ def add_shared_args(parser): | |||
| env_var="HUMAN_LOGS") | |||
| parser.add_argument('--no_aws', help="Skip AWS meta information checks", | |||
| action="store_true", default=False) | |||
| parser.add_argument('--msg_limit', help="Max limit for messages per uaid " | |||
| "before reset", type=int, default="1000", | |||
| @@ -986,6 +1000,9 @@ def finish_webpush_notifications(self, result): | |||
| self.ps.updates_sent[str(notif.channel_id)].append(notif) | |||
| msg = notif.websocket_format() | |||
| messages_sent = True | |||
| update_cnt += 1 | |||
There was a problem hiding this comment.
This count should be cleared after all the messages out of storage have been processed to ensure any later requests to check storage get a fresh starting point.
There was a problem hiding this comment.
Fine doing that, but wouldn't the arg fall out of scope pretty fast once we're out of this function? (I note that pycharm greys out the var as unused once we're past the loop.)
There was a problem hiding this comment.
Oh, sorry, this won't work. We only fetch 10 at a time, so the max number this will ever be here is 10.
There was a problem hiding this comment.
Ah! Ok, that makes more sense then. I'll move the counter bit further out.
jrconlin
force-pushed
the
bug/473
branch
2 times, most recently
from
875f0e1 to
5e15742
Compare
| def error_message_overload(self, fail): | ||
| """errBack for handling excessive messages per UAID""" | ||
| fail.trap(MessageOverloadException) | ||
| self.ap_settings.router.drop_user(self.ps.uaid) |
There was a problem hiding this comment.
Shouldn't this be run in a thread?
There was a problem hiding this comment.
Ah, looks like it should be in a force_retry to make sure it runs even if a client drops.
closes #473