comments

Author: benpankow

python_modules/dagster/dagster/_core/errors.py

@@ -280,9 +280,9 @@ def user_code_error_boundary(
     """
     check.callable_param(msg_fn, "msg_fn")
     check.class_param(error_cls, "error_cls", superclass=DagsterUserCodeExecutionError)
-    from dagster._utils.error import log_and_redact_stacktrace_if_enabled
+    from dagster._utils.error import redact_user_stacktrace_if_enabled
 
-    with log_and_redact_stacktrace_if_enabled(), raise_execution_interrupts():
+    with redact_user_stacktrace_if_enabled(), raise_execution_interrupts():
         if log_manager:
             log_manager.begin_python_log_capture()
         try:

python_modules/dagster/dagster/_core/execution/plan/utils.py

@@ -42,13 +42,13 @@ def op_execution_error_boundary(
     as respecting the RetryPolicy if present.
     """
     from dagster._core.execution.context.system import StepExecutionContext
-    from dagster._utils.error import log_and_redact_stacktrace_if_enabled
+    from dagster._utils.error import redact_user_stacktrace_if_enabled
 
     check.callable_param(msg_fn, "msg_fn")
     check.class_param(error_cls, "error_cls", superclass=DagsterUserCodeExecutionError)
     check.inst_param(step_context, "step_context", StepExecutionContext)
 
-    with log_and_redact_stacktrace_if_enabled(), raise_execution_interrupts():
+    with redact_user_stacktrace_if_enabled(), raise_execution_interrupts():
         step_context.log.begin_python_log_capture()
         retry_policy = step_context.op_retry_policy
 

python_modules/dagster/dagster/_utils/error.py

@@ -103,7 +103,12 @@ def _should_redact_user_code_error() -> bool:
 
 
 @contextlib.contextmanager
-def log_and_redact_stacktrace_if_enabled():
+def redact_user_stacktrace_if_enabled():
+    """Context manager which, if a user has enabled redacting user code errors, logs exceptions raised from within,
+    and clears the stacktrace from the exception. It also marks the exception to be redacted if it was to be persisted
+    or otherwise serialized to be sent to Dagster Plus. This is useful for preventing sensitive information from
+    being leaked in error messages.
+    """
     if not _should_redact_user_code_error():
         yield
     else:
@@ -118,6 +123,8 @@ def log_and_redact_stacktrace_if_enabled():
 
             if not existing_error_id:
                 error_id = str(uuid.uuid4())
+
+                # Track the error ID for this exception so we can redact it later
                 error_id_by_exception.set({**error_id_by_exception.get(), id(e): error_id})
                 masked_logger = logging.getLogger(_REDACTED_ERROR_LOGGER_NAME)
 
@@ -126,6 +133,7 @@ def log_and_redact_stacktrace_if_enabled():
                     exc_info=exc_info,
                 )
 
+            # Redact the stacktrace to ensure it will not be passed to Dagster Plus
             raise e.with_traceback(None) from None
 
 

python_modules/dagster/dagster_tests/core_tests/test_mask_user_code_errors.py

@@ -99,6 +99,8 @@ def hunter2():
 
                 hunter2()
         except Exception as e:
+            # Mimics behavior of resource teardown, which runs in a
+            # user_code_error_boundary after the user code raises an error
             with user_code_error_boundary(
                 error_cls=DagsterUserCodeExecutionError,
                 msg_fn=lambda: "teardown after we raised hunter2 error",