Return 503 from reset endpoint if deck config can't be cleared yet.

robot-server/robot_server/service/legacy/routers/settings.py

@@ -1,6 +1,6 @@
 from dataclasses import asdict
 import logging
-from typing import cast, Any, Dict, Optional, Union
+from typing import cast, Any, Dict, List, Optional, Union
 
 from starlette import status
 from fastapi import APIRouter, Depends
@@ -206,6 +206,7 @@ async def get_settings_reset_options(
     description="Perform a factory reset of some robot data",
     responses={
         status.HTTP_403_FORBIDDEN: {"model": LegacyErrorResponse},
+        status.HTTP_503_SERVICE_UNAVAILABLE: {"model": LegacyErrorResponse},
     },
 )
 async def post_settings_reset_options(
@@ -230,6 +231,9 @@ async def post_settings_reset_options(
         ).as_error(status.HTTP_403_FORBIDDEN)
 
     options = set(k for k, v in factory_reset_commands.items() if v)
+
+    failed_commands: List[reset_util.ResetOptionId] = []
+
     reset_util.reset(options, robot_type)
 
     if factory_reset_commands.get(reset_util.ResetOptionId.runs_history, False):
@@ -242,18 +246,27 @@ async def post_settings_reset_options(
         if deck_configuration_store:
             await deck_configuration_store.delete()
         else:
-            log.warning(
-                "Deck configuration store is not available. Skipping its reset."
-            )
+            failed_commands.append(reset_util.ResetOptionId.deck_configuration)
 
-    # TODO (tz, 5-24-22): The order of a set is undefined because set's aren't ordered.
-    # The message returned to the client will be printed in the wrong order.
-    message = (
-        "Options '{}' were reset".format(", ".join(o.name for o in options))
-        if options
-        else "Nothing to do"
-    )
-    return V1BasicResponse(message=message)
+    if failed_commands:
+        raise LegacyErrorResponse(
+            message=f"Some options could not be reset: {failed_commands}",
+            errorCode=ErrorCodes.GENERAL_ERROR.value.code,
+        ).as_error(
+            # 503 because this condition can happen if someone tries to reset something
+            # before our persistence layer has fully initialized. It will start working
+            # after initialization finishes.
+            status.HTTP_503_SERVICE_UNAVAILABLE
+        )
+    else:
+        # TODO (tz, 5-24-22): The order of a set is undefined because set's aren't ordered.
+        # The message returned to the client will be printed in the wrong order.
+        message = (
+            "Options '{}' were reset".format(", ".join(o.name for o in options))
+            if options
+            else "Nothing to do"
+        )
+        return V1BasicResponse(message=message)
 
 
 @router.get(

robot-server/tests/integration/http_api/test_deck_configuration.tavern.yaml

@@ -148,17 +148,16 @@ stages:
   # We test this here even though there are separate Tavern tests for POST /settings/reset
   # because this is a convenient place to check that the reset actually takes effect.
   - name: Reset the deck configuration
-    # FIXME(mm, 2024-02-07): This delay is mitigating a race condition in the POST /settings/reset
-    # handler. If this request comes in before the server's persistence layer has fully initialized,
-    # the handler silently ignores the request. The handler should instead block or raise a 503 "busy"
-    # error.
-    delay_before: 2
     request:
       url: '{ot3_server_base_url}/settings/reset'
       method: POST
       json:
         deckConfiguration: true
     response: {} # Expecting any non-error response.
+    # Retry on failure because if this request comes in before the server's persistence
+    # layer has fully initialized, it will return 503.
+    max_retries: 5
+    delay_after: 0.5
 
   - name: Get the deck configuration after the reset and make sure it's immediately gone back to the default
     request: