Skip to content

Core: Adding additional information in RestException - #14927

Merged
nastra merged 20 commits into
apache:mainfrom
masfworld:restexception-descriptive
Feb 4, 2026
Merged

Core: Adding additional information in RestException#14927
nastra merged 20 commits into
apache:mainfrom
masfworld:restexception-descriptive

Conversation

@masfworld

@masfworld masfworld commented Dec 24, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

There are scenarios where there are RESTException issues but the message is not providing value. For instance:

org.apache.iceberg.exceptions.RESTException: Unable to process: \n\tat
org.apache.iceberg.rest.ErrorHandlers$DefaultErrorHandler.accept(ErrorHandlers.java:250)\n\tat
org.apache.iceberg.rest.ErrorHandlers$TableErrorHandler.accept(ErrorHandlers.java:124)\n\tat
org.apache.iceberg.rest.ErrorHandlers$TableErrorHandler.accept(ErrorHandlers.java:108)\n\tat

So, the idea is to provide code and type in RESTException to get more context.

@github-actions github-actions Bot added the core label Dec 24, 2025
singhpk234
singhpk234 reviewed Dec 24, 2025
View reviewed changes
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java Outdated
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java Outdated
geruh
geruh reviewed Dec 28, 2025
View reviewed changes
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/rest/TestHTTPRequest.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/rest/TestHTTPRequest.java
@masfworld
masfworld requested review from geruh and singhpk234 January 8, 2026 11:08
@masfworld
masfworld requested a review from manuzhang January 21, 2026 09:04
manuzhang
manuzhang reviewed Jan 21, 2026
View reviewed changes
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java
manuzhang
manuzhang reviewed Jan 21, 2026
View reviewed changes
Comment thread core/src/test/java/org/apache/iceberg/rest/TestErrorHandlers.java
@masfworld
masfworld requested a review from manuzhang January 21, 2026 15:54
geruh
geruh reviewed Jan 22, 2026
View reviewed changes
Comment thread core/src/test/java/org/apache/iceberg/rest/TestErrorHandlers.java Outdated
Comment thread core/src/test/java/org/apache/iceberg/rest/TestErrorHandlers.java Outdated
@masfworld
masfworld requested a review from geruh January 23, 2026 08:44
nastra
nastra reviewed Jan 27, 2026
View reviewed changes
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java Outdated
nastra
nastra reviewed Jan 27, 2026
View reviewed changes
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java
nastra
nastra reviewed Jan 27, 2026
View reviewed changes
Comment thread core/src/test/java/org/apache/iceberg/rest/TestErrorHandlers.java Outdated
@masfworld
masfworld requested a review from nastra January 27, 2026 21:26
nastra
nastra reviewed Jan 28, 2026
View reviewed changes
Comment thread core/src/main/java/org/apache/iceberg/rest/ErrorHandlers.java
@masfworld
masfworld requested a review from nastra January 28, 2026 09:29
nastra
nastra reviewed Jan 28, 2026
View reviewed changes
Comment thread core/src/test/java/org/apache/iceberg/rest/TestErrorHandlers.java Outdated
nastra
nastra approved these changes Jan 28, 2026
View reviewed changes

@nastra nastra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with one remaining comment

singhpk234
singhpk234 approved these changes Feb 2, 2026
View reviewed changes

@singhpk234 singhpk234 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM as well, thanks @masfworld !

P.S i think comment to this private static function is a bit overkill :), but all good !

@masfworld

masfworld commented Feb 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks all for the contribution! @nastra can you merge it? Just to make sure that it's included in the next release

geruh
geruh approved these changes Feb 3, 2026
View reviewed changes

@geruh geruh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nastra
nastra merged commit 5970ddd into apache:main Feb 4, 2026
32 checks passed
talatuyarer pushed a commit to talatuyarer/iceberg that referenced this pull request Apr 1, 2026
@masfworld @talatuyarer
Core: Add code/type in RestException (apache#14927)
fea30ff
zhjwpku added a commit to zhjwpku/iceberg-cpp that referenced this pull request Jun 19, 2026
@zhjwpku
fix(rest): align error handlers with Java implementation
9fa698b 
While reviewing apache#614, I noticed that `PlanErrorHandler::Accept`
does not match the behavior of the Java implementation.

A detailed comparison with Java's `ErrorHandlers` revealed several
gaps between iceberg-cpp and Iceberg Java. Some of these are
oversights in the original implementation, while others correspond
to improvements that were made later in Iceberg Java, including:

* apache/iceberg#13143
* apache/iceberg#14927
* apache/iceberg#15051
* apache/iceberg#16059

This PR closes those gaps and brings the error handling behavior in
iceberg-cpp closer to the Java implementation.
zhjwpku added a commit to zhjwpku/iceberg-cpp that referenced this pull request Jun 19, 2026
@zhjwpku
fix(rest): align error handlers with Java implementation
a3e8205 
While reviewing apache#614, I noticed that `PlanErrorHandler::Accept`
does not match the behavior of the Java implementation.

A detailed comparison with Java's `ErrorHandlers` revealed several
gaps between iceberg-cpp and Iceberg Java. Some of these are
oversights in the original implementation, while others correspond
to improvements that were made later in Iceberg Java, including:

* apache/iceberg#13143
* apache/iceberg#14927
* apache/iceberg#15051
* apache/iceberg#16059

This PR closes those gaps and brings the error handling behavior in
iceberg-cpp closer to the Java implementation.
@zhjwpku zhjwpku mentioned this pull request Jun 19, 2026
Merged
zhjwpku added a commit to zhjwpku/iceberg-cpp that referenced this pull request Jun 22, 2026
@zhjwpku
fix(rest): align error handlers with Java implementation
04f7262 
While reviewing apache#614, I noticed that `PlanErrorHandler::Accept`
does not match the behavior of the Java implementation.

A detailed comparison with Java's `ErrorHandlers` revealed several
gaps between iceberg-cpp and Iceberg Java. Some of these are
oversights in the original implementation, while others correspond
to improvements that were made later in Iceberg Java, including:

* apache/iceberg#13143
* apache/iceberg#14927
* apache/iceberg#15051
* apache/iceberg#16059

This PR closes those gaps and brings the error handling behavior in
iceberg-cpp closer to the Java implementation.
wgtmac pushed a commit to apache/iceberg-cpp that referenced this pull request Jun 23, 2026
@zhjwpku
fix(rest): align error handlers with Java implementation (#763)
5c8c4e5 
While reviewing #614, I noticed that `PlanErrorHandler::Accept` does not
match the behavior of the Java implementation.

A detailed comparison with Java's `ErrorHandlers` revealed several gaps
between iceberg-cpp and Iceberg Java. Some of these are oversights in
the original implementation, while others correspond to improvements
that were made later in Iceberg Java, including:

* apache/iceberg#13143
* apache/iceberg#14927
* apache/iceberg#15051
* apache/iceberg#16059

This PR closes those gaps and brings the error handling behavior in
iceberg-cpp closer to the Java implementation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nastra nastra nastra approved these changes
@geruh geruh geruh approved these changes
@singhpk234 singhpk234 singhpk234 approved these changes
@manuzhang manuzhang Awaiting requested review from manuzhang
+1 more reviewer
@donPain donPain donPain approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@masfworld @nastra @manuzhang @geruh @singhpk234 @donPain