Surface remote Modal runtime failures directly

This commit is contained in:
Ken Simpson
2026-06-22 19:54:23 -07:00
parent 49e24a83ce
commit 8be50662ed
3 changed files with 75 additions and 2 deletions
+1 -1
View File
@@ -8,7 +8,7 @@
- Absorbing a preview tap means closing over its non-transportable boundary in both directions. A local `VAEDecode` preview branch cannot be moved remote while leaving its `VAELoader` local, and once that loader is remote, any remaining local `VAEEncode` or `VAEDecode` consumers of the same `VAE` must join the component too; otherwise the planner just moves the illegal `VAE` boundary from remote input to remote output.
- Preview taps should suppress split proxies only when the final component has no local re-entry dependency. If a component output feeds a local node such as `BetterGrok` and that local node feeds back into the same remote component, a single proxy creates a synthetic `proxy -> local -> same proxy` dependency cycle, so the component still has to split into ordered phases.
- Durable session bridge replay is acceptable for cheap loader/encoder recovery paths, but replaying a sampler bridge is usually a planning or affinity failure. When a downstream phase loses the live or warm bridge value for a sampler-producing component, the runtime now fails loudly instead of silently rerunning that sampler phase.
- Modal `ExecutionError` is used for both lookup/deployment failures and real remote runtime failures. Auto-deploy recovery must not key only on broad strings like `not found`; if the message contains a remote traceback or `RemoteSubgraphExecutionError`, surface the remote failure directly instead of importing the cloud deploy module and replacing the real error with an existing-app guard.
- Modal `ExecutionError` is used for both lookup/deployment failures and real remote runtime failures. Auto-deploy recovery must not key only on broad strings like `not found`, and both the stale-handle retry layer and the outer deployed-invocation catch need the same check; if the message contains a remote traceback or `RemoteSubgraphExecutionError`, surface the remote failure directly instead of importing the cloud deploy module and replacing the real error with an existing-app guard.
- Refocus replay cannot depend only on buffered Modal websocket events. Native ComfyUI completion events such as `execution_success` may be missed while the browser is backgrounded, so the frontend now reconciles temporary Modal UI state against ComfyUI's `/queue` and `/history/{prompt_id}` endpoints before leaving old node colors or the global pill visible.
## 2026-05-20
+7 -1
View File
@@ -2063,6 +2063,8 @@ def _is_missing_modal_deployment_error(exc: BaseException) -> bool:
)
):
return False
if "not deployed" in message:
return True
if "not found" not in message:
return False
return any(
@@ -5069,7 +5071,8 @@ def _invoke_modal_payload_blocking(
cancellation_event,
)
except lookup_error_types as exc:
if settings.auto_deploy:
missing_deployment = _is_missing_modal_deployment_error(exc)
if settings.auto_deploy and missing_deployment:
remote_engine = _auto_deploy_modal_app(payload, exc)
try:
logger.info(
@@ -5085,6 +5088,9 @@ def _invoke_modal_payload_blocking(
)
except lookup_error_types as retry_exc:
exc = retry_exc
missing_deployment = _is_missing_modal_deployment_error(exc)
if not missing_deployment:
raise
if not settings.allow_ephemeral_fallback:
raise ModalRemoteInvocationError(
"Remote execution requires a deployed Modal app or a successful first-run auto-deploy. "
+67
View File
@@ -3347,6 +3347,73 @@ def test_remote_modal_does_not_classify_remote_execution_error_as_missing_app(
assert remote_modal_app_module._is_missing_modal_deployment_error(missing_lookup_error)
def test_remote_modal_does_not_redeploy_after_remote_execution_error(
remote_modal_app_module: Any,
monkeypatch: Any,
) -> None:
"""The deployed invocation path should not auto-deploy after a remote runtime failure."""
class FakeLookupError(Exception):
"""Stand-in for Modal errors that share the lookup error type."""
class FakeExecuteMethod:
"""Minimal Modal method handle that raises a remote execution failure."""
def remote(self, payload: dict[str, Any], kwargs_payload: bytes) -> bytes:
"""Raise the wrapped remote traceback from the deployed call."""
del payload, kwargs_payload
raise FakeLookupError(
"Could not deserialize remote exception due to local error:\n"
"Here is the remote traceback:\n"
"comfyui_modal_sync_cloud.RemoteSubgraphExecutionError: "
"Object of type CLIP is not JSON serializable\n"
"App 'comfy-modal-sync' not found in environment 'main'."
)
class FakeRemoteEngine:
"""Minimal deployed remote engine instance."""
execute_payload = FakeExecuteMethod()
runtime_version = types.SimpleNamespace(
remote=lambda: {"protocol_version": remote_modal_app_module._REMOTE_APP_PROTOCOL_VERSION}
)
class FakeModal:
"""Minimal Modal SDK double with shared lookup/runtime error types."""
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 a deployed class successfully."""
del app_name, class_name
return lambda **kwargs: FakeRemoteEngine()
def fail_load_cloud_module() -> Any:
"""Fail if runtime errors attempt auto-deploy."""
raise AssertionError("remote execution errors must not trigger auto-deploy")
monkeypatch.setattr(remote_modal_app_module, "modal", FakeModal)
monkeypatch.setattr(remote_modal_app_module, "_load_modal_cloud_module", fail_load_cloud_module)
monkeypatch.setenv("COMFY_MODAL_AUTO_DEPLOY", "true")
remote_modal_app_module.get_settings.cache_clear()
try:
with pytest.raises(FakeLookupError, match="Object of type CLIP"):
remote_modal_app_module._invoke_modal_payload_blocking(
{"component_id": "component-1"},
b"{}",
)
finally:
remote_modal_app_module.get_settings.cache_clear()
def test_remote_modal_redeploys_when_cached_app_was_deleted(
remote_modal_app_module: Any,
monkeypatch: Any,