From 3f2271ab6923d5e109634dc1dee02f8781791caf Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Mon, 14 Apr 2025 13:30:59 -0400 Subject: [PATCH] feat: Catch all exceptions from codejail dark launch - Catch all exceptions, not just Exception, to better prevent errors from interfering with mainline responses. - Introduce a separate try block around the monitoring code so that bugs there don't cause issues. - Print exception information as well for both sides (but only if not a SafeExecException, which is redundant with emsg). Some formatting changes to log messages as well. Example outputs: For `1/0`: ``` 2025-04-14 17:26:34,239 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:240 - Remote execution in darklaunch mode produces globals={'expect': None, 'ans': '1/0'}, emsg=None, exception=None 2025-04-14 17:26:34,239 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:245 - Local execution in darklaunch mode produces globals={'expect': None, 'ans': '1/0'}, emsg='ZeroDivisionError: division by zero', exception=None ``` For `raise BaseException("hi")`: ``` 2025-04-14 17:26:13,359 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:240 - Remote execution in darklaunch mode produces globals={'expect': None, 'ans': 'raise BaseException("hi")'}, emsg=None, exception=None 2025-04-14 17:26:13,359 INFO 10232 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:245 - Local execution in darklaunch mode produces globals={'expect': None, 'ans': 'raise BaseException("hi")'}, emsg='hi', exception=BaseException('hi') ``` With codejail-service down, and `out = 1 + 2`: ``` 2025-04-14 17:30:28,597 INFO 12484 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:241 - Remote execution in darklaunch mode produces globals={'expect': None, 'ans': 'out = 1 + 2', 'out': 3, 'cfn_return': {'input_list': [{'ok': True, 'msg': 'Output:\n3', 'grade_decimal': 1}]}}, emsg=None, exception=CodejailServiceUnavailable('Codejail API Service is unavailable. Please try again in a few minutes.') 2025-04-14 17:30:28,597 INFO 12484 [xmodule.capa.safe_exec.safe_exec] [user 3] [ip 172.18.0.1] safe_exec.py:246 - Local execution in darklaunch mode produces globals={'expect': None, 'ans': 'out = 1 + 2', 'out': 3, 'cfn_return': {'input_list': [{'ok': True, 'msg': 'Output:\n3', 'grade_decimal': 1}]}}, emsg=None, exception=None ``` --- xmodule/capa/safe_exec/safe_exec.py | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/xmodule/capa/safe_exec/safe_exec.py b/xmodule/capa/safe_exec/safe_exec.py index 828c3e160f..29661f573b 100644 --- a/xmodule/capa/safe_exec/safe_exec.py +++ b/xmodule/capa/safe_exec/safe_exec.py @@ -227,18 +227,30 @@ def safe_exec( "extra_files": extra_files, } with function_trace('safe_exec.remote_exec_darklaunch'): - remote_emsg, _remote_exception = get_remote_exec(data) + remote_emsg, _ = get_remote_exec(data) + except BaseException as e: # pragma: no cover # pylint: disable=broad-except + # Swallow all exceptions and log it in monitoring so that dark launch doesn't cause issues during + # deploy. + remote_emsg = None + remote_exception = e + else: + remote_emsg = None + remote_exception = None + + try: log.info( - f"Remote execution in darklaunch mode produces: {darklaunch_globals} or exception: {remote_emsg}" + f"Remote execution in darklaunch mode produces globals={darklaunch_globals!r}, " + f"emsg={remote_emsg!r}, exception={remote_exception!r}" ) - log.info(f"Local execution in darklaunch mode produces: {globals_dict} or exception: {emsg}") + local_exc_unexpected = None if isinstance(exception, SafeExecException) else exception + log.info( + f"Local execution in darklaunch mode produces globals={globals_dict!r}, " + f"emsg={emsg!r}, exception={local_exc_unexpected!r}") set_custom_attribute('dark_launch_emsg_match', remote_emsg == emsg) set_custom_attribute('remote_emsg_exists', remote_emsg is not None) set_custom_attribute('local_emsg_exists', emsg is not None) - except Exception as e: # pragma: no cover # pylint: disable=broad-except - # Swallows all exceptions and logs it in monitoring so that dark launch doesn't cause issues during - # deploy. - log.exception("Error occurred while trying to remote exec in dark launch mode.") + except BaseException as e: # pragma: no cover # pylint: disable=broad-except + log.exception("Error occurred while trying to report codejail darklauch data.") record_exception() # Put the result back in the cache. This is complicated by the fact that