Add --trace to the phabricator command if it fails

This required rewriting @Retry

Fixes #400

Author: tomrittervg

apis/phabricator.py

@@ -41,8 +41,10 @@ def submit_patches(self, bug_id, has_patches):
         phab_revisions = []
 
         @retry
-        def submit_to_phabricator(rev_id):
+        def submit_to_phabricator(rev_id, retry_attempt=None):
             cmd = [_arc(), "diff", "--verbatim", "--conduit-uri", self.url]
+            if retry_attempt > 1:
+                cmd.append("--trace")
             if rev_id:
                 cmd.append(rev_id)
             cmd.append("--")

components/utilities.py

@@ -5,6 +5,7 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import copy
+import inspect
 import pickle
 import functools
 import time
@@ -144,28 +145,25 @@ def decorate(func):
 # Retry calling a function `times` times, sleeping between each tries, with an exponential backoff
 # This is to be used on API calls, that are likely to fail
 
-RETRY_TIMES = 10
+def retry(_func=None, *, times=10, sleep_s=1, exp=2, exceptions=(Exception,), attempt_kw="retry_attempt"):
+    def decorator(func):
+        sig = inspect.signature(func)
+        accepts_attempt = attempt_kw in sig.parameters
 
-
-def retry(_func=None, *, sleep_s=1, exp=2):
-    def decorator_retry(func):
         @functools.wraps(func)
-        def wrapper_retry(*args, **kwargs):
-            global RETRY_TIMES
-            retries_try = RETRY_TIMES
-            sleep_duration = sleep_s
-            while retries_try > 0:
+        def wrapper(*args, **kwargs):
+            backoff = sleep_s
+            for attempt in range(1, times + 1):
                 try:
-                    return func(*args, **kwargs)
-                except BaseException as e:
-                    retries_try -= 1
-                    time.sleep(sleep_duration)
-                    sleep_duration *= exp
-                    if retries_try == 0:
-                        raise e
-        return wrapper_retry
-
-    if _func is None:
-        return decorator_retry
-    else:
-        return decorator_retry(_func)
+                    call_kwargs = dict(kwargs)
+                    if accepts_attempt:
+                        call_kwargs[attempt_kw] = attempt
+                    return func(*args, **call_kwargs)
+                except exceptions:
+                    if attempt == times:
+                        raise  # preserves the original traceback
+                    time.sleep(backoff)
+                    backoff *= exp
+        return wrapper
+
+    return decorator if _func is None else decorator(_func)

tests/functionality_all_platforms.py

@@ -32,7 +32,7 @@
 from apis.taskcluster import TaskclusterProvider
 from apis.phabricator import PhabricatorProvider
 
-from tests.functionality_utilities import SHARED_COMMAND_MAPPINGS, TRY_OUTPUT, TRY_LOCKED_OUTPUT, CONDUIT_EDIT_OUTPUT, MockedBugzillaProvider, treeherder_response
+from tests.functionality_utilities import SHARED_COMMAND_MAPPINGS, TRY_OUTPUT, TRY_LOCKED_OUTPUT, ARC_OUTPUT, CONDUIT_EDIT_OUTPUT, MockedBugzillaProvider, treeherder_response
 from tests.mock_commandprovider import TestCommandProvider
 from tests.mock_libraryprovider import MockLibraryProvider
 from tests.mock_treeherder_server import MockTreeherderServerFactory, TYPE_HEALTH
@@ -662,7 +662,7 @@ def treeherder(request_type, fullpath):
 
         def try_submit(cmd):
             nonlocal try_fails
-            if try_fails == 0:
+            if try_fails < 2:
                 try_fails += 1
                 raise Exception("No worky!")
             if "./mach try auto" in cmd:
@@ -697,6 +697,37 @@ def try_submit(cmd):
         finally:
             self._cleanup(u, expected_values)
 
+    # Fail the first time, then work.
+    @logEntryExitHeaderLine
+    def testPhabRetryFunctionality(self):
+        try_fails = 0
+
+        def phab_submit(cmd):
+            nonlocal try_fails
+            if try_fails == 0:
+                try_fails += 1
+                raise Exception("No worky!")
+            if "--trace" not in cmd:
+                raise Exception("Expected to see --trace in the phabricator command")
+            return ARC_OUTPUT % (83000 + 50, 83000 + 50)
+
+        library_filter = 'dav1d'
+        (u, expected_values, _check_jobs) = self._setup(
+            library_filter,
+            lambda b: ["try_rev|2021-02-09 15:30:04 -0500|2021-02-12 17:40:01 +0000"],
+            lambda: 50,  # get_filed_bug_id_func,
+            lambda b: {},  # filed_bug_ids_func
+            AssertFalse,  # treeherder_response
+            command_callbacks={'phab_submit': phab_submit}
+        )
+        try:
+            # Run it
+            u.run(library_filter=library_filter)
+            # Check that we created the job successfully
+            _check_jobs(JOBSTATUS.AWAITING_SECOND_PLATFORMS_TRY_RESULTS, JOBOUTCOME.PENDING)
+        finally:
+            self._cleanup(u, expected_values)
+
     @logEntryExitHeaderLine
     def testAllNewFuzzyPathJobs(self):
         @treeherder_response