This is an automated email from the ASF dual-hosted git repository.
zhengruifeng pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new b9c9451fa68f [SPARK-58629][CONNECT][FOLLOWUP] Avoid empty INTERNAL
gRPC error messages
b9c9451fa68f is described below
commit b9c9451fa68fbe44217dabaf7aed36de1609b17f
Author: haoyangeng-db <[email protected]>
AuthorDate: Thu Aug 20 16:03:29 2026 +0800
[SPARK-58629][CONNECT][FOLLOWUP] Avoid empty INTERNAL gRPC error messages
### What changes were proposed in this pull request?
Follow-up to #57837, which made the fallback (`UNKNOWN`) gRPC error
description non-empty. This applies the same class-name fallback to the
non-fatal `INTERNAL` path.
`ErrorUtils.buildStatusFromThrowable` took its status message from
`SparkConnectService.extractErrorMessage`, which abbreviated `getMessage` and
then mapped `null` to `""`. Since `Utils.abbreviate(null, 2048)` returns
`null`, a non-fatal throwable with no message produced an `INTERNAL` status
with an empty message.
The message now comes from the helper added in #57837, renamed from
`fallbackErrorDescription` to `errorDescription` since it is no longer used
only by the fallback path. `SparkConnectService.extractErrorMessage` and
`convertNullString` are left with no callers and are removed.
### Why are the changes needed?
A message-less non-fatal exception, for example a bare `new
RuntimeException()`, reached the client as an `INTERNAL` error with no
description at all, giving no indication of what failed. This is the same gap
#57837 closed for the fatal path.
### Does this PR introduce _any_ user-facing change?
Yes. For an already-failing request whose non-fatal throwable has no
message, the client now receives the throwable's fully qualified class name
instead of an empty error message. This also covers a throwable whose message
is the empty string, which previously produced an empty message as well.
Successful requests, and errors whose throwable has a non-empty message,
are unchanged.
### How was this patch tested?
Added three cases to `ErrorUtilsSuite` for `buildStatusFromThrowable`: a
throwable with no message and a throwable with an empty message both yield
`java.lang.RuntimeException` as the status message, and a throwable with a
message keeps that message.
The suite could not be run locally because the SBT launcher and Maven
dependencies are unreachable in my development environment, so it runs in CI.
Static checks passed: no remaining reference to the removed helpers anywhere in
the tree, and every touched file's imports are still used.
### Was this patch authored or co-authored using generative AI tooling?
Co-authored with Claude Code
Closes #58134 from haoyangeng-db/spark-connect-internal-error-description.
Authored-by: haoyangeng-db <[email protected]>
Signed-off-by: Ruifeng Zheng <[email protected]>
---
.../sql/connect/service/SparkConnectService.scala | 13 -------------
.../spark/sql/connect/utils/ErrorUtils.scala | 12 +++++++++---
.../spark/sql/connect/utils/ErrorUtilsSuite.scala | 22 ++++++++++++++++++++++
3 files changed, 31 insertions(+), 16 deletions(-)
diff --git
a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala
b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala
index 0c18a7491c1f..1fa9fbeda1e5 100644
---
a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala
+++
b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/service/SparkConnectService.scala
@@ -559,19 +559,6 @@ object SparkConnectService extends Logging {
listenerBus.post(eventBuilder(bindingAddress))
}
-
- def extractErrorMessage(st: Throwable): String = {
- val message = Utils.abbreviate(st.getMessage, 2048)
- convertNullString(message)
- }
-
- def convertNullString(str: String): String = {
- if (str != null) {
- str
- } else {
- ""
- }
- }
}
/**
diff --git
a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
index 01644663e2af..0a5a411093b7 100644
---
a/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
+++
b/sql/connect/server/src/main/scala/org/apache/spark/sql/connect/utils/ErrorUtils.scala
@@ -276,7 +276,7 @@ private[connect] object ErrorUtils extends Logging {
.newBuilder()
.setCode(RPCCode.INTERNAL_VALUE)
.addDetails(ProtoAny.pack(withStackTrace.build()))
- .setMessage(SparkConnectService.extractErrorMessage(st))
+ .setMessage(errorDescription(st))
.build()
}
@@ -287,7 +287,13 @@ private[connect] object ErrorUtils extends Logging {
.exists(_.toString.contains("org.apache.spark.sql.execution.python"))
}
- private def fallbackErrorDescription(e: Throwable): String = {
+ /**
+ * Returns a non-empty description for the given throwable: its abbreviated
message when one is
+ * present, and its fully qualified class name otherwise. Both the INTERNAL
status built for a
+ * non-fatal throwable and the UNKNOWN fallback status use this, so that a
client never receives
+ * an error whose description is empty.
+ */
+ private def errorDescription(e: Throwable): String = {
val message = e.getMessage
if (message != null && message.nonEmpty) {
Utils.abbreviate(message, 2048)
@@ -352,7 +358,7 @@ private[connect] object ErrorUtils extends Logging {
case e: Throwable =>
Status.UNKNOWN
.withCause(e)
- .withDescription(fallbackErrorDescription(e))
+ .withDescription(errorDescription(e))
.asRuntimeException()
}
diff --git
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/utils/ErrorUtilsSuite.scala
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/utils/ErrorUtilsSuite.scala
index 45cc87e14d09..97b9c8db673b 100644
---
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/utils/ErrorUtilsSuite.scala
+++
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/utils/ErrorUtilsSuite.scala
@@ -18,6 +18,7 @@ package org.apache.spark.sql.connect.utils
import java.util.UUID
+import com.google.rpc.Code
import io.grpc.{Status, StatusRuntimeException}
import io.grpc.stub.StreamObserver
@@ -48,4 +49,25 @@ class ErrorUtilsSuite extends SharedSparkSession {
assert(error.getStatus.getCode == Status.Code.UNKNOWN)
assert(error.getStatus.getDescription ==
classOf[InterruptedException].getName)
}
+
+ test("buildStatusFromThrowable uses throwable class when non-fatal throwable
has no message") {
+ val status = ErrorUtils.buildStatusFromThrowable(new RuntimeException(),
None)
+
+ assert(status.getCode == Code.INTERNAL_VALUE)
+ assert(status.getMessage == classOf[RuntimeException].getName)
+ }
+
+ test("buildStatusFromThrowable uses throwable class when non-fatal message
is empty") {
+ val status = ErrorUtils.buildStatusFromThrowable(new RuntimeException(""),
None)
+
+ assert(status.getCode == Code.INTERNAL_VALUE)
+ assert(status.getMessage == classOf[RuntimeException].getName)
+ }
+
+ test("buildStatusFromThrowable keeps the message when the non-fatal
throwable has one") {
+ val status = ErrorUtils.buildStatusFromThrowable(new
RuntimeException("boom"), None)
+
+ assert(status.getCode == Code.INTERNAL_VALUE)
+ assert(status.getMessage == "boom")
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
