Retrying subscription transactions with increasing delays

[alecpm]

See jazkarta/nci.content#336 and specifically this comment

Apparently Authorize.net AcceptJS will sometimes just fail to recognize a valid subscription payment nonce token when it's busy and it may be necessary to retry the transaction with a delay of up to 10 seconds. Right now NCI has a 5 second delay on the client side to aid with this, which impacts all transactions. However, since this issue seems specific to subscription transactions and a silent delay on the client seems disconcerting (especially if longer than 5 seconds), I think the delay probably belongs in jaz.shop. Additionally, it's only really possible to implement further incremental delays and retries on the server side.

This should repeat the transaction when it fails with a specific error code up to 4 times with a delay that increments by 2 seconds on each repeat. So if needed, the final transaction should occur 12 seconds after the initial try.

[alecpm]

Assigning to @witekdev and @jessesnyder since you both know this code better than I do and I haven't tested it and don't know how to test it. I'm a bit disconcerted that the docker build github action e2e tests are failing here, but it looks like that's related to how docker is being called.

[cypress]

jazkarta.shop    Run #65

Run Properties:   Failed #65  •  ced32958c8: fix comment

Project	jazkarta.shop
Branch Review	nci-issues/336-recurring-donation-delay
Run status	Failed #65
Run duration	00m 19s
Commit	ced32958c8: fix comment
Committer	witekdev
View all properties for this run ↗︎

---

Test results
Failures	2
Flaky	0
Pending	0
Skipped	0
Passing	1
View all changes introduced in this branch ↗︎

---

Tests for review

  cypress/integration/first_spec.js • 2 failed tests • UI - Chrome

View Output Video

Test		Artifacts
Admin operations > creates a buyable product		Screenshots Video
Admin operations > buys one product		Screenshots Video

[alecpm]

I've done a little testing now and it seems to work well in the sandbox and when I raise an artificial error. Not sure about these test failures, but they seem to be related to infrastructure setup or similar.

[witekdev]

@alecpm this looks structurally good to me

I couldn't test locally with an NCI instance, because I got E_WC_02: A HTTPS connection is required. when attempting to submit the donation form. If that wasn't an issue, FYI, this feature cannot easily be tested with a stock Plone site, because the NCI donation form hard wires certain required parameters that are not present otherwise in jazkarta.shop alone.

https://github.com/jazkarta/nci.content/commit/c72f2eb1441433b0a4b27d3ed466ebf90efef5a9
https://github.com/jazkarta/nci.content/commit/3c533611f5ed813f3dc02a303b8fce5f727c9c80

    @property
    def is_recurring(self):
        return 'is_monthly' in self.request.form

    recurring_months = 12

https://github.com/jazkarta/nci.content/blob/master/nci/content/browser/donation.py

<input type="checkbox" id="is_monthly" name="is_monthly" tal:attributes="checked python:'checked' if 'is_monthly' in request.form else None" />
https://github.com/jazkarta/nci.content/blob/master/nci/content/browser/templates/donation.pt

jazkarta.shop itself only has these defaults:

    is_recurring = False
    recurring_months = None

Using this online python parser I created this dummy test case to test the logic of your code, it seems good
https://onecompiler.com/python/42zd2t5bs

class PException(Exception):
      """A problem with payment processing."""
class ZException(Exception):
      """Another problem."""

import time

def retrystuff(self):
  
  retries = 0

  while retries < 4:
      print("RUNNING")
      try:
          print("TRYING")
          raise PException
          return 
      except PException as e:
          retries += 1
          if retries >= 4:
              print("RAISE E")
              raise e
          # Retry after delay for specific error
          if True:
              delay = 2 * retries * 0.001
              print("RETRYING")
              time.sleep(delay)
              continue
          print("RAISE another E")
          raise e      
      
      
try:
  retrystuff('test')
except PException as e:
  print("EPIC FAIL")
except ZException as e:
  print("Another error")

print("DONE")      

The way you have it setup now it will try max four times, that is probably a good first step to see if this solves the issue they are having.

try 1st
if fail
wait 2 seconds

try 2nd
if fail
wait 4 seconds

try 3rd
if fail
wait 6 seconds

try 4th
abort

[alecpm]

For testing code that requires SSL, I find local-ssl-proxy (installable from npm) helpful. You need to use a Plone VHM url for it to work, but it lets you test auth.net stuff.

Thanks for looking into this. If you think it’s suitable to merge, can you approve the PR?

[witekdev]

For testing code that requires SSL, I find local-ssl-proxy (installable from npm) helpful. You need to use a Plone VHM url for it to work, but it lets you test auth.net stuff.

Good to know, thanks!

Thanks for looking into this. If you think it’s suitable to merge, can you approve the PR?

Sure, happy to merge.
Do you want to remove that, what seems to be redundant line, first?

jazkarta.shop/jazkarta/shop/browser/checkout/authorize_net_accept_js.py

Line 83 in ced3295

raise e

Or do you want to leave it in?

[alecpm]

I don’t think it’s redundant. That line should immediately raise any exceptions that aren’t “Invalid OTS Token”. We only want to retry when we hit that specific exception message.

[witekdev]

I don’t think it’s redundant. That line should immediately raise any exceptions that aren’t “Invalid OTS Token”. We only want to retry when we hit that specific exception message.

I think you're right 👍 I didn't think of it that way, but it makes sense.