Skip to content

Commit

Permalink
Add InvalidPayload state
Browse files Browse the repository at this point in the history
  • Loading branch information
kaapstorm committed Aug 10, 2024
1 parent 74e6676 commit 73f811c
Show file tree
Hide file tree
Showing 8 changed files with 39 additions and 23 deletions.
1 change: 1 addition & 0 deletions corehq/apps/reports/filters/select.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def options(self):
State.Pending,
State.Cancelled,
State.Fail,
State.InvalidPayload,
]]


Expand Down
2 changes: 2 additions & 0 deletions corehq/motech/repeaters/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,15 @@ class State(IntegerChoices):
Success = 4, _('Succeeded')
Cancelled = 8, _('Cancelled')
Empty = 16, _('Empty') # There was nothing to send. Implies Success.
InvalidPayload = 32, _('Invalid Payload') # Implies Cancelled.


RECORD_PENDING_STATE = State.Pending
RECORD_SUCCESS_STATE = State.Success
RECORD_FAILURE_STATE = State.Fail
RECORD_CANCELLED_STATE = State.Cancelled
RECORD_EMPTY_STATE = State.Empty
RECORD_INVALIDPAYLOAD_STATE = State.InvalidPayload


class UCRRestrictionFFStatus(IntegerChoices):
Expand Down
19 changes: 10 additions & 9 deletions corehq/motech/repeaters/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,7 @@ def requeue(self):
# preserves the value of `self.failure_reason`.
if self.succeeded:
self.state = State.Pending
elif self.state == State.Cancelled:
elif self.state in (State.Cancelled, State.InvalidPayload):
self.state = State.Fail
self.next_check = datetime.utcnow()
self.max_possible_tries = self.num_attempts + MAX_BACKOFF_ATTEMPTS
Expand Down Expand Up @@ -1120,13 +1120,13 @@ def _add_failure_attempt(self, message, max_attempts, retry=True):
self.next_check = (attempt.created_at + wait) if state == State.Fail else None
self.save()

def add_payload_exception_attempt(self, message, tb_str):
def add_payload_error_attempt(self, message, tb_str):
self.attempt_set.create(
state=State.Cancelled,
state=State.InvalidPayload,
message=message,
traceback=tb_str,
)
self.state = State.Cancelled
self.state = State.InvalidPayload
self.next_check = None
self.save()

Expand Down Expand Up @@ -1210,7 +1210,7 @@ def fire(self, force_send=False):
except Exception as e:
log_repeater_error_in_datadog(self.domain, status_code=None,
repeater_type=self.repeater_type)
self.handle_payload_error(str(e))
self.handle_payload_error(str(e), tb_str=traceback.format_exc())
finally:
return self.state
return None
Expand Down Expand Up @@ -1265,8 +1265,8 @@ def handle_failure(self, response):
def handle_exception(self, exception):
self.add_client_failure_attempt(str(exception))

def handle_payload_error(self, message):
self.add_client_failure_attempt(message, retry=False)
def handle_payload_error(self, message, tb_str=''):
self.add_payload_error_attempt(message, tb_str)

def cancel(self):
self.state = State.Cancelled
Expand Down Expand Up @@ -1338,13 +1338,14 @@ def get_payload(repeater: Repeater, repeat_record: RepeatRecord) -> str:
status_code=None,
repeater_type=repeater.__class__.__name__
)
repeat_record.add_payload_exception_attempt(
repeat_record.add_payload_error_attempt(
message=str(err),
tb_str=traceback.format_exc()
)
raise


# TODO: Unused outside of tests
def send_request(
repeater: Repeater,
repeat_record: RepeatRecord,
Expand Down Expand Up @@ -1408,7 +1409,7 @@ def later_might_be_better(resp):


def has_failed(record):
return record.state in (State.Fail, State.Cancelled)
return record.state in (State.Fail, State.Cancelled, State.InvalidPayload)


def format_response(response):
Expand Down
16 changes: 11 additions & 5 deletions corehq/motech/repeaters/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,18 +142,24 @@ def is_retry(repeat_record):
def update_repeater(repeat_record_states, repeater_id):
"""
Determines whether the repeater should back off, based on the
results of `fire_repeat_record()` tasks.
results of ``_process_repeat_record()`` tasks.
"""
try:
repeater = Repeater.objects.get(id=repeater_id)
if any(s == State.Success for s in repeat_record_states):
# At least one repeat record was sent successfully.
# At least one repeat record was sent successfully. The
# remote endpoint is healthy.
repeater.reset_backoff()
elif all(s in (State.Empty, None) for s in repeat_record_states):
# Nothing was sent. Don't update the repeater.
elif all(s in (State.Empty, State.InvalidPayload, None)
for s in repeat_record_states):
# We can't tell anything about the remote endpoint.
# _process_repeat_record() can return None if it is called
# with a repeat record whose state is Success or Empty. That
# can't happen in this workflow, but None is included for
# completeness.
pass
else:
# All sent payloads failed.
# All sent payloads failed. Try again later.
repeater.set_backoff()
finally:
lock = Lock(f'process_repeater_{repeater_id}')
Expand Down
11 changes: 6 additions & 5 deletions corehq/motech/repeaters/tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
MAX_BACKOFF_ATTEMPTS,
RECORD_CANCELLED_STATE,
RECORD_FAILURE_STATE,
RECORD_INVALIDPAYLOAD_STATE,
RECORD_PENDING_STATE,
RECORD_SUCCESS_STATE,
State,
Expand Down Expand Up @@ -384,17 +385,17 @@ def test_add_client_failure_attempt_no_retry(self):
self.assertEqual(self.repeat_record.attempts[0].message, message)
self.assertEqual(self.repeat_record.attempts[0].traceback, '')

def test_add_payload_exception_attempt(self):
def test_add_payload_error_attempt(self):
message = 'ValueError: Schema validation failed'
tb_str = 'Traceback ...'
self.repeat_record.add_payload_exception_attempt(message=message,
tb_str=tb_str)
self.assertEqual(self.repeat_record.state, RECORD_CANCELLED_STATE)
self.repeat_record.add_payload_error_attempt(message=message,
tb_str=tb_str)
self.assertEqual(self.repeat_record.state, RECORD_INVALIDPAYLOAD_STATE)
# Note: Our payload issues do not affect how we deal with their
# server issues:
self.assertEqual(self.repeat_record.num_attempts, 1)
self.assertEqual(self.repeat_record.attempts[0].state,
RECORD_CANCELLED_STATE)
RECORD_INVALIDPAYLOAD_STATE)
self.assertEqual(self.repeat_record.attempts[0].message, message)
self.assertEqual(self.repeat_record.attempts[0].traceback, tb_str)

Expand Down
6 changes: 3 additions & 3 deletions corehq/motech/repeaters/tests/test_repeater.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,8 +658,8 @@ def test_get_payload_exception(self):
state_or_none = repeat_record.fire()

self.assertEqual(repeat_record.failure_reason, 'Boom!')
self.assertEqual(repeat_record.state, State.Cancelled)
self.assertEqual(state_or_none, State.Cancelled)
self.assertEqual(repeat_record.state, State.InvalidPayload)
self.assertEqual(state_or_none, State.InvalidPayload)

def test_payload_exception(self):
case = CommCareCase.objects.get_case(CASE_ID, self.domain)
Expand All @@ -668,7 +668,7 @@ def test_payload_exception(self):
rr.fire()

repeat_record = RepeatRecord.objects.get(id=rr.id)
self.assertEqual(repeat_record.state, State.Cancelled)
self.assertEqual(repeat_record.state, State.InvalidPayload)
self.assertEqual(repeat_record.failure_reason, "Payload error")

def test_failure(self):
Expand Down
6 changes: 5 additions & 1 deletion corehq/motech/repeaters/views/repeat_record_display.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
from django.utils.translation import gettext as _

from corehq.motech.repeaters.const import (
RECORD_INVALIDPAYLOAD_STATE,
RECORD_CANCELLED_STATE,
RECORD_EMPTY_STATE,
RECORD_FAILURE_STATE,
RECORD_PENDING_STATE,
RECORD_SUCCESS_STATE,
RECORD_EMPTY_STATE,
)
from corehq.util.timezones.conversions import ServerTime

Expand Down Expand Up @@ -70,6 +71,9 @@ def _get_state_tuple(record):
elif record.state == RECORD_EMPTY_STATE:
label_cls = 'success'
label_text = _('Empty')
elif record.state == RECORD_INVALIDPAYLOAD_STATE:
label_cls = 'danger'
label_text = _('Invalid Payload')
else:
label_cls = ''
label_text = ''
Expand Down
1 change: 1 addition & 0 deletions corehq/motech/repeaters/views/tests/test_repeat_records.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class view:
# updated if the keys in this dict change
'Pending': 1,
'Fail': 0,
'InvalidPayload': 0,
'Success': 0,
'Cancelled': 0,
'Empty': 0,
Expand Down

0 comments on commit 73f811c

Please sign in to comment.