fix: recover from stale modal handles after app deletion

This commit is contained in:
Ken Simpson
2026-04-23 14:47:43 -07:00
parent e9245af948
commit 55e37d1380
4 changed files with 248 additions and 9 deletions
+1
View File
@@ -149,6 +149,7 @@ Remote mode requires:
The normal remote path is deployed-app lookup by name. On first remote use, the extension can auto-deploy the stable cloud entrypoint from [`comfyui_modal_sync_cloud.py`](comfyui_modal_sync_cloud.py) if the configured app does not exist yet.
If that deployed Modal app is later deleted manually, the next lookup failure now invalidates the stale in-process deploy cache and auto-deploys a fresh copy instead of surfacing a hard "app not found" error.
If a previously looked-up `RemoteEngine` handle later fails during `hydrate()` or the first `remote()` call because the app was deleted out-of-band, the invoke path now treats that as the same missing-app condition, auto-deploys, and retries instead of first surfacing a spurious prompt failure.
If a previous attempt imported that cloud entrypoint only partially before failing, the loader now discards the stale cached module and retries the import cleanly before auto-deploy.
The local and remote subgraph runners also canonicalize malformed singleton-list wrappers in rewritten prompts, including socket indexes like `["node", [0]]`, scalar inputs like `[4]`, and wrapped execute node ids like `["12"]`, before handing the prompt to ComfyUI.
When the remote PromptExecutor still fails, the Modal worker now logs the normalized execute-node ids, boundary outputs, suspicious remaining wrapped prompt inputs, and the underlying ComfyUI `execution_error` payload to make the bad field visible.
+1
View File
@@ -208,4 +208,5 @@
- Frontend mapped-progress lanes cannot be keyed only by `laneId` at prompt scope. Different remote components legitimately reuse local worker lane numbers like `0` and `1`, so the UI has to namespace lane ownership by component representative or concurrent components will clear each other's purple bars and appear to flicker between active and ready states.
- Streamed remote progress should suppress native proxy `executing` churn, but not the proxy's terminal completion. If the frontend drops every native phase update after streamed progress starts, a single-node remote component can stay stuck purple because it only ever receives streamed `finalizing` and never consumes the completion event that should turn it green.
- The in-process "already auto-deployed" cache cannot be treated as authoritative once operators can delete the Modal app out-of-band. A later `app not found` lookup has to invalidate that cache entry and redeploy instead of assuming the earlier successful deploy still exists remotely.
- With deployed `modal.Cls` handles, "app missing" is not only a lookup-time failure. A stale handle can also survive lookup and then fail later during `hydrate()` or the first `remote()` call. The invoke path has to treat that the same way as a missing-app lookup, auto-deploy, and retry before surfacing an error locally.
- Combined first-run and second-run performance logs are now clear enough to prioritize work. In this workflow, uploads and volume reload were already cheap, while the recurring non-sampling bottleneck was static-phase model loading (`UNETLoader` + `CLIPLoader` at roughly `24-25s` total per run). The second run also showed that local proxy caching can skip the static proxy dispatch while still leaving a large replay cost inside the mapped phase, which makes two no-brainer optimizations obvious: reuse heavy loader outputs on the same worker, and avoid replaying the whole static producer branch just to reconstruct session-backed bridge values.
+66 -9
View File
@@ -3809,7 +3809,7 @@ async def _invoke_bound_remote_engine_async(
)
cancellation_event = threading.Event()
future = _REMOTE_MODAL_CALL_EXECUTOR.submit(
_invoke_remote_engine_payload,
_invoke_remote_engine_payload_with_recovery,
remote_engine,
dict(payload),
kwargs_payload,
@@ -4203,6 +4203,27 @@ def _invoke_remote_engine_warmup(remote_engine: Any, warmup_request: dict[str, A
return warmup_method(warmup_request)
def _invoke_remote_engine_warmup_with_recovery(
remote_engine: Any,
warmup_request: dict[str, Any],
) -> Any:
"""Retry one warmup call after auto-deploy when a stale deployed handle vanishes."""
lookup_error_types = _modal_lookup_error_types()
settings = get_settings()
try:
return _invoke_remote_engine_warmup(remote_engine, warmup_request)
except lookup_error_types as exc:
if not settings.auto_deploy or not _is_missing_modal_deployment_error(exc):
raise
logger.warning(
"Modal warmup invocation failed for component=%s because the deployed app was missing at call time: %s. Recreating the app and retrying.",
warmup_request.get("component_id"),
exc,
)
recovered_remote_engine = _auto_deploy_modal_app(warmup_request, exc)
return _invoke_remote_engine_warmup(recovered_remote_engine, warmup_request)
def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any:
"""Warm one Modal container slot using deployed or ephemeral app state."""
if modal is None:
@@ -4213,12 +4234,15 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any:
if lookup_error_types:
try:
remote_engine = _lookup_deployed_remote_engine(warmup_request)
return _invoke_remote_engine_warmup(remote_engine, warmup_request)
return _invoke_remote_engine_warmup_with_recovery(remote_engine, warmup_request)
except lookup_error_types as exc:
if settings.auto_deploy:
remote_engine = _auto_deploy_modal_app(warmup_request, exc)
try:
return _invoke_remote_engine_warmup(remote_engine, warmup_request)
return _invoke_remote_engine_warmup_with_recovery(
remote_engine,
warmup_request,
)
except lookup_error_types as retry_exc:
exc = retry_exc
if not settings.allow_ephemeral_fallback:
@@ -4228,7 +4252,7 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any:
) from exc
else:
remote_engine = _lookup_deployed_remote_engine(warmup_request)
return _invoke_remote_engine_warmup(remote_engine, warmup_request)
return _invoke_remote_engine_warmup_with_recovery(remote_engine, warmup_request)
cloud_module = _load_modal_cloud_module()
cloud_app = getattr(cloud_module, "app", None)
@@ -4240,7 +4264,7 @@ def _invoke_modal_warmup_blocking(warmup_request: dict[str, Any]) -> Any:
run_context = cloud_app.run() if hasattr(cloud_app, "run") else nullcontext()
with run_context:
remote_engine = cloud_remote_engine()
return _invoke_remote_engine_warmup(remote_engine, warmup_request)
return _invoke_remote_engine_warmup_with_recovery(remote_engine, warmup_request)
def _run_prompt_warmup_slot(
@@ -4566,6 +4590,39 @@ def _invoke_remote_engine_payload(
)
def _invoke_remote_engine_payload_with_recovery(
remote_engine: Any,
payload: dict[str, Any],
kwargs_payload: bytes,
cancellation_event: threading.Event | None,
) -> bytes:
"""Retry one payload call after auto-deploy when a stale deployed handle vanishes."""
lookup_error_types = _modal_lookup_error_types()
settings = get_settings()
try:
return _invoke_remote_engine_payload(
remote_engine,
payload,
kwargs_payload,
cancellation_event,
)
except lookup_error_types as exc:
if not settings.auto_deploy or not _is_missing_modal_deployment_error(exc):
raise
logger.warning(
"Modal payload invocation failed for component=%s because the deployed app was missing at call time: %s. Recreating the app and retrying.",
payload.get("component_id"),
exc,
)
recovered_remote_engine = _auto_deploy_modal_app(payload, exc)
return _invoke_remote_engine_payload(
recovered_remote_engine,
payload,
kwargs_payload,
cancellation_event,
)
def _invoke_modal_payload_blocking(
payload: dict[str, Any],
kwargs_payload: bytes,
@@ -4585,7 +4642,7 @@ def _invoke_modal_payload_blocking(
settings.app_name,
payload.get("component_id"),
)
return _invoke_remote_engine_payload(
return _invoke_remote_engine_payload_with_recovery(
remote_engine,
payload,
kwargs_payload,
@@ -4600,7 +4657,7 @@ def _invoke_modal_payload_blocking(
settings.app_name,
payload.get("component_id"),
)
return _invoke_remote_engine_payload(
return _invoke_remote_engine_payload_with_recovery(
remote_engine,
payload,
kwargs_payload,
@@ -4628,7 +4685,7 @@ def _invoke_modal_payload_blocking(
settings.app_name,
payload.get("component_id"),
)
return _invoke_remote_engine_payload(
return _invoke_remote_engine_payload_with_recovery(
remote_engine,
payload,
kwargs_payload,
@@ -4652,7 +4709,7 @@ def _invoke_modal_payload_blocking(
run_context = cloud_app.run() if hasattr(cloud_app, "run") else nullcontext()
with run_context:
remote_engine = cloud_remote_engine()
result = _invoke_remote_engine_payload(
result = _invoke_remote_engine_payload_with_recovery(
remote_engine,
payload,
kwargs_payload,
+180
View File
@@ -2812,6 +2812,186 @@ def test_remote_modal_redeploys_when_cached_app_was_deleted(
assert deploy_calls == [("comfy-modal-sync", "main")]
def test_remote_modal_redeploys_when_deployed_handle_disappears_during_payload_invoke(
remote_modal_app_module: Any,
monkeypatch: Any,
) -> None:
"""A stale deployed engine handle should auto-redeploy when invocation hydrate fails."""
class FakeLookupError(Exception):
"""Stand-in for Modal deployed lookup failures."""
deploy_calls: list[tuple[str | None, str | None]] = []
class FakeRemoteEngine:
"""Minimal deployed remote engine instance with stale-handle behavior."""
def __init__(self, *, stale: bool) -> None:
"""Record whether this engine should fail once as a stale handle."""
self._stale = stale
self.execute_payload = types.SimpleNamespace(remote=self._remote)
def _remote(self, payload: dict[str, Any], kwargs_payload: bytes) -> bytes:
"""Raise the missing-app error for stale handles and otherwise succeed."""
del payload, kwargs_payload
if self._stale:
raise FakeLookupError(
"Lookup failed for Cls 'RemoteEngine' from the 'comfy-modal-sync' app: "
"App 'comfy-modal-sync' not found in environment 'main'."
)
return b"remote-response"
class FakeApp:
"""Minimal deployable cloud app double."""
def deploy(
self,
*,
name: str | None = None,
environment_name: str | None = None,
**_: Any,
) -> "FakeApp":
"""Record the redeploy request and mark future handles as fresh."""
deploy_calls.append((name, environment_name))
FakeModal.redeployed = True
return self
class FakeModal:
"""Minimal modal SDK double with stale-handle invocation failures."""
redeployed = False
exception = types.SimpleNamespace(
NotFoundError=FakeLookupError,
ExecutionError=FakeLookupError,
InvalidError=FakeLookupError,
)
class Cls:
"""Namespace for deployed class lookups."""
@staticmethod
def from_name(app_name: str, class_name: str) -> Any:
"""Return stale handles before redeploy and fresh handles after."""
del app_name, class_name
return lambda **kwargs: FakeRemoteEngine(stale=not FakeModal.redeployed)
@staticmethod
def enable_output() -> Any:
"""Provide a no-op output context manager."""
return nullcontext()
monkeypatch.setattr(remote_modal_app_module, "modal", FakeModal)
monkeypatch.setattr(
remote_modal_app_module,
"_load_modal_cloud_module",
lambda: types.SimpleNamespace(app=FakeApp()),
)
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "true")
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
remote_modal_app_module.get_settings.cache_clear()
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
try:
response = remote_modal_app_module._invoke_modal_payload_blocking(
{"component_id": "component-1"},
b"{}",
)
finally:
remote_modal_app_module.get_settings.cache_clear()
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
assert response == b"remote-response"
assert deploy_calls == [("comfy-modal-sync", "main")]
def test_remote_modal_redeploys_when_deployed_handle_disappears_during_warmup(
remote_modal_app_module: Any,
monkeypatch: Any,
) -> None:
"""A stale deployed warmup handle should auto-redeploy instead of surfacing a warmup error."""
class FakeLookupError(Exception):
"""Stand-in for Modal deployed lookup failures."""
deploy_calls: list[tuple[str | None, str | None]] = []
class FakeRemoteEngine:
"""Minimal deployed remote engine instance with stale-handle warmup behavior."""
def __init__(self, *, stale: bool) -> None:
"""Expose a warmup method that fails for stale handles."""
self._stale = stale
self.warmup_for_request = types.SimpleNamespace(remote=self._remote)
def _remote(self, payload: dict[str, Any]) -> dict[str, Any]:
"""Raise the missing-app error for stale handles and otherwise succeed."""
if self._stale:
raise FakeLookupError(
"Lookup failed for Cls 'RemoteEngine' from the 'comfy-modal-sync' app: "
"App 'comfy-modal-sync' not found in environment 'main'."
)
return {"component_id": str(payload.get("component_id"))}
class FakeApp:
"""Minimal deployable cloud app double."""
def deploy(
self,
*,
name: str | None = None,
environment_name: str | None = None,
**_: Any,
) -> "FakeApp":
"""Record the redeploy request and mark future handles as fresh."""
deploy_calls.append((name, environment_name))
FakeModal.redeployed = True
return self
class FakeModal:
"""Minimal modal SDK double with stale-handle warmup failures."""
redeployed = False
exception = types.SimpleNamespace(
NotFoundError=FakeLookupError,
ExecutionError=FakeLookupError,
InvalidError=FakeLookupError,
)
class Cls:
"""Namespace for deployed class lookups."""
@staticmethod
def from_name(app_name: str, class_name: str) -> Any:
"""Return stale handles before redeploy and fresh handles after."""
del app_name, class_name
return lambda **kwargs: FakeRemoteEngine(stale=not FakeModal.redeployed)
@staticmethod
def enable_output() -> Any:
"""Provide a no-op output context manager."""
return nullcontext()
monkeypatch.setattr(remote_modal_app_module, "modal", FakeModal)
monkeypatch.setattr(
remote_modal_app_module,
"_load_modal_cloud_module",
lambda: types.SimpleNamespace(app=FakeApp()),
)
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "true")
monkeypatch.setenv("MODAL_ENVIRONMENT", "main")
remote_modal_app_module.get_settings.cache_clear()
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
try:
response = remote_modal_app_module._invoke_modal_warmup_blocking(
{"component_id": "component-1::warmup:0"},
)
finally:
remote_modal_app_module.get_settings.cache_clear()
remote_modal_app_module._MODAL_AUTO_DEPLOY_STATES.clear()
assert response == {"component_id": "component-1::warmup:0"}
assert deploy_calls == [("comfy-modal-sync", "main")]
def test_remote_modal_auto_deploy_is_shared_across_concurrent_first_run_callers(
remote_modal_app_module: Any,
monkeypatch: Any,