diff --git a/.github/workflows/instance_pool_ci.yml b/.github/workflows/instance_pool_ci.yml index 77e5b2c..cb6ba31 100644 --- a/.github/workflows/instance_pool_ci.yml +++ b/.github/workflows/instance_pool_ci.yml @@ -154,5 +154,7 @@ jobs: echo "Reproduce with \`WORKARENA_TEST_SHARD=$WORKARENA_TEST_SHARD WORKARENA_TEST_SEED=$WORKARENA_TEST_SEED\` and the pytest command below." | tee -a "$GITHUB_STEP_SUMMARY" # L1 (test_task_general.py) already runs in full every night in test-l1-tasks + # -n 10 rather than 20: this shard's compositional tasks are heavier per-worker than L1, + # and 20 concurrent browsers routinely overloaded the shared pool (timeouts, 502s) - name: Run shard - run: pytest -n 20 --durations=10 --slowmo 1000 -v -m 'slow or pricy' --ignore=tests/test_task_general.py tests + run: pytest -n 10 --durations=10 --slowmo 1000 -v -m 'slow or pricy' --ignore=tests/test_task_general.py tests diff --git a/src/browsergym/workarena/api/utils.py b/src/browsergym/workarena/api/utils.py index 47a6b2a..d26b441 100644 --- a/src/browsergym/workarena/api/utils.py +++ b/src/browsergym/workarena/api/utils.py @@ -2,12 +2,39 @@ from ..instance import SNowInstance -from requests.exceptions import HTTPError +from requests.exceptions import ConnectionError, HTTPError +from tenacity import retry, retry_if_exception, stop_after_attempt, wait_exponential from time import sleep # ServiceNow API configuration SNOW_API_HEADERS = {"Content-Type": "application/json", "Accept": "application/json"} +# Gateway/proxy status codes that indicate a transient blip on an overloaded instance +# rather than a real API error +_TRANSIENT_STATUS_CODES = {502, 503, 504} + + +def _is_transient_http_error(exception: BaseException) -> bool: + if isinstance(exception, ConnectionError): + return True + return ( + isinstance(exception, HTTPError) + and exception.response is not None + and exception.response.status_code in _TRANSIENT_STATUS_CODES + ) + + +@retry( + retry=retry_if_exception(_is_transient_http_error), + stop=stop_after_attempt(5), + wait=wait_exponential(multiplier=1, min=1, max=10), + reraise=True, +) +def _request_with_retry(**kwargs) -> requests.Response: + response = requests.request(**kwargs) + response.raise_for_status() + return response + def table_api_call( instance: SNowInstance, @@ -52,8 +79,8 @@ def table_api_call( """ - # Query API - response = requests.request( + # Query API (retries transient gateway/connection errors from the shared, load-sensitive instance) + response = _request_with_retry( method=method, url=instance.snow_url + f"/api/now/table/{table}", auth=instance.snow_credentials, @@ -67,9 +94,6 @@ def table_api_call( data = {} params = {"sysparm_query": f"sys_id={sys_id}"} - # Check for HTTP success code (fail otherwise) - response.raise_for_status() - record_exists = False num_retries = 0 if method == "POST" or wait_for_record: @@ -120,12 +144,12 @@ def table_column_info(instance: SNowInstance, table: str) -> dict: """ # Query the Meta API to get most of the column info (e.g., valid choices) - response = requests.get( + response = _request_with_retry( + method="GET", url=instance.snow_url + f"/api/now/ui/meta/{table}", auth=instance.snow_credentials, headers=SNOW_API_HEADERS, ) - response.raise_for_status() meta_info = response.json()["result"]["columns"] # Clean column value choices @@ -164,11 +188,9 @@ def db_delete_from_table(instance: SNowInstance, sys_id: str, table: str) -> Non """ # Query API - response = requests.delete( + _request_with_retry( + method="DELETE", url=instance.snow_url + f"/api/now/table/{table}/{sys_id}", auth=instance.snow_credentials, headers=SNOW_API_HEADERS, ) - - # Check for HTTP code 200 (fail otherwise) - response.raise_for_status() diff --git a/src/browsergym/workarena/tasks/form.py b/src/browsergym/workarena/tasks/form.py index c32e83d..ac3adcb 100644 --- a/src/browsergym/workarena/tasks/form.py +++ b/src/browsergym/workarena/tasks/form.py @@ -526,7 +526,11 @@ def show_field_tab(field): # Check if the record was created if self.check_record_created: # This does not work if multiple forms are created at once. The localStorage returns null after the first form - for attempt in range(5): + # Give this the same budget as other browser-side waits: under load, the record can + # take a while to become queryable via the API even though it was genuinely created. + wait_per_attempt_ms = 1500 + max_attempts = SNOW_BROWSER_TIMEOUT // wait_per_attempt_ms + for attempt in range(max_attempts): # in update tasks, the sys_id is already known as the asset is created from the start if update: sys_id = self.record_sys_id @@ -544,8 +548,8 @@ def show_field_tab(field): )["result"] if len(record) > 0: break - page.wait_for_timeout(1500) - if attempt == 4: + page.wait_for_timeout(wait_per_attempt_ms) + if attempt == max_attempts - 1: raise ValueError("The record was not created.") def _set_required_config_attributes(self, config: dict) -> None: @@ -815,7 +819,8 @@ def cheat(self, page: Page, chat_messages: list[str]) -> None: # On the change request page, additional steps need to be taken to open the form if self.table_label == "change request": self._wait_for_ready(page, iframe_only=True) - iframe.get_by_label("All").click() + # "All" also substring-matches the list view's "Select All" checkbox label + iframe.get_by_label("All").first.click() iframe.get_by_text("Normal").first.click() self._fill_fields(page, iframe, self.task_fields)