feat: propagate Java stack traces from JVM upcalls into DataFusionError#94
Open
LantaoJin wants to merge 1 commit into
Open
feat: propagate Java stack traces from JVM upcalls into DataFusionError#94LantaoJin wants to merge 1 commit into
LantaoJin wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Rationale for this change
When a Java-implemented upcall throws — scalar UDFs via
JniBridge.invokeScalarUdf, table providers viaJniBridge.invokeTableScan— the native side captures onlyclass.getName()andgetMessage()viajthrowable_to_stringand surfaces it asDataFusionError::Execution(...). Since #81 routes that through to a typedExecutionException, but the original Java stack trace is gone by then. A user whose UDF throwsNullPointerExceptiondeep insideevaluate(...)sees:with no way to find the failing line in their own code. The information exists; it's just discarded.
What changes are included in this PR?
This PR captures the Java-formatted trace (via
Throwable.printStackTrace(PrintWriter)to aStringWriter) and appends it to the existing message. Verbosity is configurable perSessionContextso production paths that treat exception bodies as untrusted user input can opt out:The default is
FULL-- the issue's stated default -- so callers who don't touch the setter immediately gain stack traces with no API change.Out of scope (called out in #55, deferred):
Throwableascauseon the typedExecutionExceptionfrom feat: typed DataFusionException hierarchy across the JNI boundary #81). Needs a registry-handle for the throwable that survives the JNI return; complementary follow-up.Are these changes tested?
Yes. 10 new tests in
ExceptionVerbosityTest.Are there any user-facing changes?
Yes, additive only -- default behaviour does change: the message string surfaced inside
ExecutionException.getMessage()now includes the Java stack trace below the existing"Java UDF 'name' threw class: message"header. Callers that already onlystartsWith("Java UDF '...' threw")-style match keep working; anything stricter (an exact-equals on the message) would need the newMESSAGEverbosity to lock the pre-#55 format.ExceptionVerbosity.SessionContextBuilder.exceptionVerbosity(...)setter; default unchanged for callers that don't invoke it (their behaviour changes — they now get traces by default).