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 a93ffcfd6374 [SPARK-56926][CONNECT] Guard against null
messageParameters in Connect error proto serialization
a93ffcfd6374 is described below
commit a93ffcfd63747388b18ccefa4ea66d61daac445e
Author: haoyangeng-db <[email protected]>
AuthorDate: Wed May 20 09:03:18 2026 +0800
[SPARK-56926][CONNECT] Guard against null messageParameters in Connect
error proto serialization
### What changes were proposed in this pull request?
Null-guard `sparkThrowableBuilder.putAllMessageParameters` in
`ErrorUtils.throwableToProtoErrors` (Spark Connect server). If a
`SparkThrowable` implementation returns `null` from `getMessageParameters()`,
the unguarded call to `putAllMessageParameters(null)` throws a
`NullPointerException`. With the guard, a `null` map is treated the same as an
empty one.
### Why are the changes needed?
The `SparkThrowable` interface's default `getMessageParameters` returns an
empty map, but an override may violate that implicit non-null contract. When it
does, the resulting NPE inside `FetchErrorDetails` server-side serialization
crashes the RPC, and the client falls back to a degraded error reconstruction
that drops the cause chain, hiding the underlying error from the user.
### Does this PR introduce _any_ user-facing change?
No. Well-behaved `SparkThrowable` implementations are unaffected. A faulty
override that returns `null` now serializes with an empty `messageParameters`
map instead of crashing the RPC.
### How was this patch tested?
Added a regression test in `FetchErrorDetailsHandlerSuite` that uses an
inline `SparkThrowable` whose `getMessageParameters` returns `null` and
verifies the response is built successfully with an empty `messageParameters`
map.
### Was this patch authored or co-authored using generative AI tooling?
Drafted with Claude Code
Closes #55992 from haoyangeng-db/spark-connect-guard-null-message-params.
Authored-by: haoyangeng-db <[email protected]>
Signed-off-by: Ruifeng Zheng <[email protected]>
---
.../apache/spark/sql/connect/utils/ErrorUtils.scala | 8 +++++++-
.../service/FetchErrorDetailsHandlerSuite.scala | 19 +++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
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 619fefe2e7c6..588493aeaa92 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
@@ -161,7 +161,13 @@ private[connect] object ErrorUtils extends Logging {
breakingChangeInfoBuilder.setNeedsAudit(breakingChangeInfo.needsAudit)
sparkThrowableBuilder.setBreakingChangeInfo(breakingChangeInfoBuilder.build())
}
-
sparkThrowableBuilder.putAllMessageParameters(sparkThrowable.getMessageParameters)
+ // The SparkThrowable interface's default getMessageParameters
returns an empty
+ // map, but a faulty override may return null. Guard against that so
the
+ // FetchErrorDetails RPC doesn't crash with a NullPointerException.
+ val messageParameters = sparkThrowable.getMessageParameters
+ if (messageParameters != null) {
+ sparkThrowableBuilder.putAllMessageParameters(messageParameters)
+ }
builder.setSparkThrowable(sparkThrowableBuilder.build())
case _ =>
}
diff --git
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
index c74dea3418f0..813a57b88936 100644
---
a/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
+++
b/sql/connect/server/src/test/scala/org/apache/spark/sql/connect/service/FetchErrorDetailsHandlerSuite.scala
@@ -369,4 +369,23 @@ class FetchErrorDetailsHandlerSuite extends
SharedSparkSession with ResourceHelp
assert(!error.hasSparkThrowable)
assert(error.getMessage == "Regular runtime exception")
}
+
+ test("throwableToFetchErrorDetailsResponse with null getMessageParameters") {
+ // A SparkThrowable override that returns null from getMessageParameters
(in violation
+ // of the interface's implicit non-null contract) must not crash the
FetchErrorDetails
+ // RPC with a NullPointerException.
+ val testError = new Exception("error with null message params") with
SparkThrowable {
+ override def getCondition: String = "NULL_PARAMS_ERROR"
+ override def getErrorClass: String = "NULL_PARAMS_ERROR"
+ override def getMessageParameters: java.util.Map[String, String] = null
+ }
+
+ val response =
+ ErrorUtils.throwableToFetchErrorDetailsResponse(testError,
serverStackTraceEnabled = false)
+
+ assert(response.hasRootErrorIdx)
+ val sparkThrowableProto = response.getErrors(0).getSparkThrowable
+ assert(sparkThrowableProto.getErrorClass == "NULL_PARAMS_ERROR")
+ assert(sparkThrowableProto.getMessageParametersMap.isEmpty)
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
