-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
fix(replays): Move code requiring exception-prone variables into try block #64658
Conversation
@@ -238,6 +239,23 @@ def process_message(buffer: RecordingBuffer, message: bytes) -> None: | |||
decoded_message["retention_days"], | |||
parsed_recording_data, | |||
) | |||
|
|||
if replay_actions is not None: | |||
buffer.replay_action_events.append(replay_actions) |
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.
do we need to move the message to the DLQ somehow? catch
seems to only log it
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.
Only if the message is retry-able after programmer intervention. So something like a service outage isn't a good fit for a DLQ. We intentionally crash loop in those scenarios and wait for the service to become available.
We wouldn't want to DLQ the message that caused INC-630 because we have no capability of parsing it. It should be silently dropped (we log for tracking but otherwise don't care). The replay message is still partially processed. The rage clicks are not sent but we still have the core functionality of the message processor working.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #64658 +/- ##
=======================================
Coverage 81.36% 81.37%
=======================================
Files 5237 5236 -1
Lines 231143 231166 +23
Branches 45350 45355 +5
=======================================
+ Hits 188064 188102 +38
+ Misses 37226 37214 -12
+ Partials 5853 5850 -3
|
Fixes INC-630. Variables are not defined if an exception is raised causing an unhandled crashloop.