Skip to content

Commit

Permalink
[SPARK-46241][PYTHON][CONNECT] Fix error handling routine so it would…
Browse files Browse the repository at this point in the history
…n't fall into infinite recursion

### What changes were proposed in this pull request?

Remove _display_server_stack_trace and always display error stack trace if we have one

### Why are the changes needed?

There is a certain codepath which can make existing error handling fall into infinite recursion. I.e. consider following codepath:

`[Some error happens] -> _handle_error -> _handle_rpc_error -> _display_server_stack_trace -> RuntimeConf.get -> SparkConnectClient.config -> [An error happens] -> _handle_error`.

There can be other similar codepaths

### Does this PR introduce _any_ user-facing change?

Gets rid of occasionally infinite recursive error handling (which can cause downgraded user experience)

### How was this patch tested?
N/A

### Was this patch authored or co-authored using generative AI tooling?
No

Closes apache#44144 from cdkrot/forbid_recursive_error_handling.

Authored-by: Alice Sayutina <alice.sayutina@databricks.com>
Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>
  • Loading branch information
cdkrot authored and HyukjinKwon committed Dec 5, 2023
1 parent 89673da commit c9df53f
Showing 1 changed file with 1 addition and 15 deletions.
16 changes: 1 addition & 15 deletions python/pyspark/sql/connect/client/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1520,20 +1520,6 @@ def _fetch_enriched_error(self, info: "ErrorInfo") -> Optional[pb2.FetchErrorDet
except grpc.RpcError:
return None

def _display_server_stack_trace(self) -> bool:
from pyspark.sql.connect.conf import RuntimeConf

conf = RuntimeConf(self)
try:
if conf.get("spark.sql.connect.serverStacktrace.enabled") == "true":
return True
return conf.get("spark.sql.pyspark.jvmStacktrace.enabled") == "true"
except Exception as e: # noqa: F841
# Falls back to true if an exception occurs during reading the config.
# Otherwise, it will recursively try to get the conf when it consistently
# fails, ending up with `RecursionError`.
return True

def _handle_rpc_error(self, rpc_error: grpc.RpcError) -> NoReturn:
"""
Error handling helper for dealing with GRPC Errors. On the server side, certain
Expand Down Expand Up @@ -1567,7 +1553,7 @@ def _handle_rpc_error(self, rpc_error: grpc.RpcError) -> NoReturn:
info,
status.message,
self._fetch_enriched_error(info),
self._display_server_stack_trace(),
True,
) from None

raise SparkConnectGrpcException(status.message) from None
Expand Down

0 comments on commit c9df53f

Please sign in to comment.