IGNITE-28000 Reduce verbosity of client stacktraces#7854
Open
tmgodinho wants to merge 25 commits intoapache:mainfrom
Open
IGNITE-28000 Reduce verbosity of client stacktraces#7854tmgodinho wants to merge 25 commits intoapache:mainfrom
tmgodinho wants to merge 25 commits intoapache:mainfrom
Conversation
tmgodinho
force-pushed
the
ignite-28000
branch
5 times, most recently
from
b1023bc to
974eb7b
Compare
** Add null edge case for cause mapper ** Add utility methods
…n KeyValueViewImpl
There was a problem hiding this comment.
Pull request overview
This PR targets IGNITE-28000 by reducing thin-client exception verbosity and shifting “public exception” shaping to higher-level API boundaries, updating both runtime code and test expectations accordingly.
Changes:
- Refactors client/server exception propagation (e.g.,
TcpClientChannel#readError,ViewUtils#sync/ensurePublicException, client SQL/compute/table) to reduce nested causes and duplicate stack traces. - Adds “copy” support for some exceptions with extra fields and adjusts exception mappers/matchers used by tests.
- Updates integration/unit tests across modules to validate the new exception shape and messaging (including stack-trace hint behavior).
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/table/src/main/java/org/apache/ignite/internal/table/KeyValueViewImpl.java | Avoid re-wrapping MarshallerException when building marshaller. |
| modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItPkOnlyTableCrossApiTest.java | Updates assertions to new public-exception matching style. |
| modules/security/src/integrationTest/java/org/apache/ignite/internal/ssl/ItSslTest.java | Updates SSL client connection error expectations. |
| modules/runner/src/integrationTest/java/org/apache/ignite/internal/table/ItDataConsistencyTest.java | Handles ClientRetriableTransactionException in IT loops. |
| modules/core/src/testFixtures/java/org/apache/ignite/internal/TraceableExceptionMatcher.java | Extends matcher to allow custom trace-id matching and null-cause matching. |
| modules/core/src/testFixtures/java/org/apache/ignite/internal/IgniteExceptionTestUtils.java | Adds helpers to match server-sent stack-trace strings / hint-only messages. |
| modules/core/src/test/java/org/apache/ignite/internal/util/ViewUtilsTest.java | Updates tests for changed ensurePublicException cause/stack behavior. |
| modules/core/src/main/java/org/apache/ignite/internal/util/ViewUtils.java | Refactors sync and ensurePublicException behavior and exception copying. |
| modules/compute/src/integrationTest/java/org/apache/ignite/internal/compute/ItComputeTestEmbedded.java | Introduces client-type awareness in compute exception assertions. |
| modules/compute/src/integrationTest/java/org/apache/ignite/internal/compute/ItComputeTestClient.java | Sets client type for compute ITs. |
| modules/compute/src/integrationTest/java/org/apache/ignite/internal/compute/ItComputeStandaloneTest.java | Sets client type for standalone compute ITs. |
| modules/compute/src/integrationTest/java/org/apache/ignite/internal/compute/ItComputeBaseTest.java | Refactors compute exception matchers to support embedded vs thin client behavior. |
| modules/client/src/test/java/org/apache/ignite/client/ConnectionTest.java | Adjusts error-message expectations for connection failures. |
| modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientTransactionKilledException.java | Adds copy method for field-preserving exception copying. |
| modules/client/src/main/java/org/apache/ignite/internal/client/table/ClientTable.java | Ensures public exceptions at the table API layer for async ops. |
| modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSql.java | Reuses ViewUtils#sync for sync SQL methods. |
| modules/client/src/main/java/org/apache/ignite/internal/client/compute/ClientCompute.java | Reuses ViewUtils#sync and removes local sync helper. |
| modules/client/src/main/java/org/apache/ignite/internal/client/TcpClientChannel.java | Simplifies server error decoding and reduces nested causes. |
| modules/client/src/main/java/org/apache/ignite/internal/client/ClientRetriableTransactionException.java | Makes the exception public and adjusts constructor to carry message. |
| modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientTransactionsTest.java | Updates transaction IT assertions to new public-exception shape. |
| modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientConnectionTest.java | Updates “hint present” exception test to new matcher style. |
| modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientComputeTypeCheckMarshallingTest.java | Updates compute marshalling IT assertions to new public-exception matcher. |
| modules/client/src/integrationTest/java/org/apache/ignite/internal/client/ItThinClientComputeTest.java | Updates compute IT exception assertions to new public-exception matchers. |
| modules/cli/src/main/java/org/apache/ignite/internal/cli/core/exception/handler/SqlExceptionHandler.java | Adjusts CLI error rendering for connection/auth failures under new exception messages. |
| modules/api/src/main/java/org/apache/ignite/sql/SqlBatchException.java | Adds copy method for exception copying while preserving update counters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ptupitsyn
reviewed
Mar 30, 2026
** Update javadoc on ViewUtils#ensurePublicException ** Fix details logic in SqlExceptionHandler ** Fix orphan assertion in ItPkOnlyTableCrossApiTest, ItSslTest, and ItThinClientComputeTest ** Extracted method from lambda in TcpClientChannel#readError
** Removed missing TODO
Contributor
|
@tmgodinho in the provided examples we are missing an important part of the stack trace: Can you please double-check? |
Contributor
Author
|
@ptupitsyn Java usually folds stacktraces that are similar to the top exception. |
ptupitsyn
requested changes
Apr 14, 2026
| } catch (TransactionException e) { | ||
| // Don't need to rollback manually if got IgniteException. | ||
| fails.increment(); | ||
| } catch (ClientRetriableTransactionException e) { |
Contributor
There was a problem hiding this comment.
ClientRetriableTransactionException is internal and must be never observed by the user.
Contributor
Author
There was a problem hiding this comment.
That's a good point, sorry.
I'll need to review all these RetriableTransactionException
| var msg = invalidCredentialsException.getMessage(); | ||
|
|
||
| String details; | ||
| var headerIdx = msg.indexOf('\n'); |
Contributor
Author
There was a problem hiding this comment.
I don't think they can.
Those InvalidCredentialsException always have their message set via the constructor, and the constructor does not mark it as nullable.
Do you think it's worth to make the explicit check??
Contributor
There was a problem hiding this comment.
On the protocol level null message is allowe (see TcpClientChannel.readError)
Contributor
Author
There was a problem hiding this comment.
Yeah, there it is allowed. But I don't think the instances of InvalidCredentialsException in particular will.
I've protected it anyway.
…ableTransactionException and ClientTransactionKilledException as internal
…ng if views comply with the public exception guidelines.
** It was not forcing public exceptions.
** Protected SqkExceptionHandler msg transform against NPE.
Contributor
Author
|
We might consider rolling out the |
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.
https://issues.apache.org/jira/browse/IGNITE-28000
What was done:
Examples
ItThinClientTransactionsTest#testUpdateInReadOnlyTxThrows
Before
After
Compute Job exception
Before:
After:
Marhsaller Exception
Before:
After: