From 45a96e24303122636c27b2cee6f720db009b008a Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Mon, 14 Apr 2025 13:01:09 -0400 Subject: [PATCH] feat: Run remote codejail even if unexpected exception in local safe_exec During dark launch of remote codejail, we want to ensure we always run both local and remote execution -- otherwise we're missing data for the remote side in an important situation. This will help answer the question of whether the unexpected exception happens on both sides, even though it may not look exactly the same due to differences in how unexpected errors are handled. An example input that provokes this in unsafe execution mode is `raise BaseException("hi")`; in safe execution mode, printing to `sys.__stdout__` should also produce an appropriate error. --- xmodule/capa/safe_exec/safe_exec.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/xmodule/capa/safe_exec/safe_exec.py b/xmodule/capa/safe_exec/safe_exec.py index 882db11fd1..828c3e160f 100644 --- a/xmodule/capa/safe_exec/safe_exec.py +++ b/xmodule/capa/safe_exec/safe_exec.py @@ -159,6 +159,8 @@ def safe_exec( raise SafeExecException(emsg) return + cacheable = True # unless we get an unexpected error + # Create the complete code we'll run. code_prolog = CODE_PROLOG % random_seed @@ -196,11 +198,17 @@ def safe_exec( limit_overrides_context=limit_overrides_context, slug=slug, ) - except SafeExecException as e: + except BaseException as e: # Saving SafeExecException e in exception to be used later. exception = e emsg = str(e) + if not isinstance(exception, SafeExecException): + # Something unexpected happened, so don't cache this evaluation. + # (We may decide to cache these in the future as well; this is just + # preserving existing behavior during a refactor of error handling.) + cacheable = False else: + exception = None emsg = None # Run the code in both the remote codejail service as well as the local codejail @@ -235,10 +243,10 @@ def safe_exec( # Put the result back in the cache. This is complicated by the fact that # the globals dict might not be entirely serializable. - if cache: + if cache and cacheable: cleaned_results = json_safe(globals_dict) cache.set(key, (emsg, cleaned_results)) # If an exception happened, raise it now. - if emsg: + if exception: raise exception