Skip to content

Conversation

@masfworld
Copy link

There are scenarios where there are RESTException issues but the message is 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
case 409:
throw new AlreadyExistsException("%s", error.message());
case 422:
throw new RESTException("Unable to process: %s", error.message());
Copy link
Contributor

Choose a reason for hiding this comment

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

The namespace handler had a inconsistency in the 422 case, maybe we can remove this to fall back to the new behavior in DefaultErrorHandler?

.withMessage("Invalid input")
.build();

assertThatThrownBy(() -> ErrorHandlers.defaultErrorHandler().accept(error))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some tests with out the message and type as they're optional

@masfworld masfworld requested review from geruh and singhpk234 January 8, 2026 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants