Skip to content

refactor: consolidate rest error response model #359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Fokko merged 1 commit into from
Nov 28, 2025
Merged

refactor: consolidate rest error response model #359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions src/iceberg/catalog/rest/error_handlers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ const std::shared_ptr<DefaultErrorHandler>& DefaultErrorHandler::Instance() {
return instance;
}

Status DefaultErrorHandler::Accept(const ErrorModel& error) const {
Status DefaultErrorHandler::Accept(const ErrorResponse& error) const {
switch (error.code) {
case 400:
if (error.type == kIllegalArgumentException) {
Expand Down Expand Up @@ -69,7 +69,7 @@ const std::shared_ptr<NamespaceErrorHandler>& NamespaceErrorHandler::Instance()
return instance;
}

Status NamespaceErrorHandler::Accept(const ErrorModel& error) const {
Status NamespaceErrorHandler::Accept(const ErrorResponse& error) const {
switch (error.code) {
case 400:
if (error.type == kNamespaceNotEmptyException) {
Expand All @@ -93,7 +93,7 @@ const std::shared_ptr<DropNamespaceErrorHandler>& DropNamespaceErrorHandler::Ins
return instance;
}

Status DropNamespaceErrorHandler::Accept(const ErrorModel& error) const {
Status DropNamespaceErrorHandler::Accept(const ErrorResponse& error) const {
if (error.code == 409) {
return NamespaceNotEmpty(error.message);
}
Expand All @@ -106,7 +106,7 @@ const std::shared_ptr<TableErrorHandler>& TableErrorHandler::Instance() {
return instance;
}

Status TableErrorHandler::Accept(const ErrorModel& error) const {
Status TableErrorHandler::Accept(const ErrorResponse& error) const {
switch (error.code) {
case 404:
if (error.type == kNoSuchNamespaceException) {
Expand All @@ -125,7 +125,7 @@ const std::shared_ptr<ViewErrorHandler>& ViewErrorHandler::Instance() {
return instance;
}

Status ViewErrorHandler::Accept(const ErrorModel& error) const {
Status ViewErrorHandler::Accept(const ErrorResponse& error) const {
switch (error.code) {
case 404:
if (error.type == kNoSuchNamespaceException) {
Expand All @@ -145,7 +145,7 @@ const std::shared_ptr<TableCommitErrorHandler>& TableCommitErrorHandler::Instanc
return instance;
}

Status TableCommitErrorHandler::Accept(const ErrorModel& error) const {
Status TableCommitErrorHandler::Accept(const ErrorResponse& error) const {
switch (error.code) {
case 404:
return NoSuchTable(error.message);
Expand All @@ -167,7 +167,7 @@ const std::shared_ptr<ViewCommitErrorHandler>& ViewCommitErrorHandler::Instance(
return instance;
}

Status ViewCommitErrorHandler::Accept(const ErrorModel& error) const {
Status ViewCommitErrorHandler::Accept(const ErrorResponse& error) const {
switch (error.code) {
case 404:
return NoSuchView(error.message);
Expand Down
21 changes: 9 additions & 12 deletions src/iceberg/catalog/rest/error_handlers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,11 @@ class ICEBERG_REST_EXPORT ErrorHandler {
public:
virtual ~ErrorHandler() = default;

// TODO(Li Feiyang):removing ErrorModel as the inner layer and directly using
// ErrorResponse

/// \brief Process an error response and return an appropriate Error.
///
/// \param error The error model parsed from the HTTP response body
/// \param error The error response parsed from the HTTP response body
/// \return An Error object with appropriate ErrorKind and message
virtual Status Accept(const ErrorModel& error) const = 0;
virtual Status Accept(const ErrorResponse& error) const = 0;
};

/// \brief Default error handler for REST API responses.
Expand All @@ -52,7 +49,7 @@ class ICEBERG_REST_EXPORT DefaultErrorHandler : public ErrorHandler {
/// \brief Returns the singleton instance
static const std::shared_ptr<DefaultErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

protected:
constexpr DefaultErrorHandler() = default;
Expand All @@ -64,7 +61,7 @@ class ICEBERG_REST_EXPORT NamespaceErrorHandler : public DefaultErrorHandler {
/// \brief Returns the singleton instance
static const std::shared_ptr<NamespaceErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

protected:
constexpr NamespaceErrorHandler() = default;
Expand All @@ -76,7 +73,7 @@ class ICEBERG_REST_EXPORT DropNamespaceErrorHandler final : public NamespaceErro
/// \brief Returns the singleton instance
static const std::shared_ptr<DropNamespaceErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

private:
constexpr DropNamespaceErrorHandler() = default;
Expand All @@ -88,7 +85,7 @@ class ICEBERG_REST_EXPORT TableErrorHandler final : public DefaultErrorHandler {
/// \brief Returns the singleton instance
static const std::shared_ptr<TableErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

private:
constexpr TableErrorHandler() = default;
Expand All @@ -100,7 +97,7 @@ class ICEBERG_REST_EXPORT ViewErrorHandler final : public DefaultErrorHandler {
/// \brief Returns the singleton instance
static const std::shared_ptr<ViewErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

private:
constexpr ViewErrorHandler() = default;
Expand All @@ -112,7 +109,7 @@ class ICEBERG_REST_EXPORT TableCommitErrorHandler final : public DefaultErrorHan
/// \brief Returns the singleton instance
static const std::shared_ptr<TableCommitErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

private:
constexpr TableCommitErrorHandler() = default;
Expand All @@ -124,7 +121,7 @@ class ICEBERG_REST_EXPORT ViewCommitErrorHandler final : public DefaultErrorHand
/// \brief Returns the singleton instance
static const std::shared_ptr<ViewCommitErrorHandler>& Instance();

Status Accept(const ErrorModel& error) const override;
Status Accept(const ErrorResponse& error) const override;

private:
constexpr ViewCommitErrorHandler() = default;
Expand Down
2 changes: 1 addition & 1 deletion src/iceberg/catalog/rest/http_client.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ Status HandleFailureResponse(const cpr::Response& response,
// TODO(gangwu): response status code is lost, wrap it with RestError.
ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.text));
ICEBERG_ASSIGN_OR_RAISE(auto error_response, ErrorResponseFromJson(json));
return error_handler.Accept(error_response.error);
return error_handler.Accept(error_response);
}
return {};
}
Expand Down
44 changes: 17 additions & 27 deletions src/iceberg/catalog/rest/json_internal.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,43 +91,34 @@ Result<CatalogConfig> CatalogConfigFromJson(const nlohmann::json& json) {
return config;
}

nlohmann::json ToJson(const ErrorModel& error) {
nlohmann::json ToJson(const ErrorResponse& error) {
nlohmann::json error_json;
error_json[kMessage] = error.message;
error_json[kType] = error.type;
error_json[kCode] = error.code;
SetContainerField(error_json, kStack, error.stack);

nlohmann::json json;
json[kMessage] = error.message;
json[kType] = error.type;
json[kCode] = error.code;
SetContainerField(json, kStack, error.stack);
json[kError] = std::move(error_json);
return json;
}

Result<ErrorModel> ErrorModelFromJson(const nlohmann::json& json) {
ErrorModel error;
Result<ErrorResponse> ErrorResponseFromJson(const nlohmann::json& json) {
ICEBERG_ASSIGN_OR_RAISE(auto error_json, GetJsonValue<nlohmann::json>(json, kError));

ErrorResponse error;
// NOTE: Iceberg's Java implementation allows missing required fields (message, type,
// code) during deserialization, which deviates from the REST spec. We enforce strict
// validation here.
ICEBERG_ASSIGN_OR_RAISE(error.message, GetJsonValue<std::string>(json, kMessage));
ICEBERG_ASSIGN_OR_RAISE(error.type, GetJsonValue<std::string>(json, kType));
ICEBERG_ASSIGN_OR_RAISE(error.code, GetJsonValue<uint32_t>(json, kCode));
ICEBERG_ASSIGN_OR_RAISE(error.stack,
GetJsonValueOrDefault<std::vector<std::string>>(json, kStack));
ICEBERG_ASSIGN_OR_RAISE(error.message, GetJsonValue<std::string>(error_json, kMessage));
ICEBERG_ASSIGN_OR_RAISE(error.type, GetJsonValue<std::string>(error_json, kType));
ICEBERG_ASSIGN_OR_RAISE(error.code, GetJsonValue<uint32_t>(error_json, kCode));
ICEBERG_ASSIGN_OR_RAISE(
error.stack, GetJsonValueOrDefault<std::vector<std::string>>(error_json, kStack));
ICEBERG_RETURN_UNEXPECTED(error.Validate());
return error;
}

nlohmann::json ToJson(const ErrorResponse& response) {
nlohmann::json json;
json[kError] = ToJson(response.error);
return json;
}

Result<ErrorResponse> ErrorResponseFromJson(const nlohmann::json& json) {
ErrorResponse response;
ICEBERG_ASSIGN_OR_RAISE(auto error_json, GetJsonValue<nlohmann::json>(json, kError));
ICEBERG_ASSIGN_OR_RAISE(response.error, ErrorModelFromJson(error_json));
ICEBERG_RETURN_UNEXPECTED(response.Validate());
return response;
}

nlohmann::json ToJson(const CreateNamespaceRequest& request) {
nlohmann::json json;
json[kNamespace] = request.namespace_.levels;
Expand Down Expand Up @@ -339,7 +330,6 @@ Result<ListTablesResponse> ListTablesResponseFromJson(const nlohmann::json& json
}

ICEBERG_DEFINE_FROM_JSON(CatalogConfig)
ICEBERG_DEFINE_FROM_JSON(ErrorModel)
ICEBERG_DEFINE_FROM_JSON(ErrorResponse)
ICEBERG_DEFINE_FROM_JSON(ListNamespacesResponse)
ICEBERG_DEFINE_FROM_JSON(CreateNamespaceRequest)
Expand Down
1 change: 0 additions & 1 deletion src/iceberg/catalog/rest/json_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ Result<Model> FromJson(const nlohmann::json& json);
/// \note Don't forget to add `ICEBERG_DEFINE_FROM_JSON` to the end of
/// `json_internal.cc` to define the `FromJson` function for the model.
ICEBERG_DECLARE_JSON_SERDE(CatalogConfig)
ICEBERG_DECLARE_JSON_SERDE(ErrorModel)
ICEBERG_DECLARE_JSON_SERDE(ErrorResponse)
ICEBERG_DECLARE_JSON_SERDE(ListNamespacesResponse)
ICEBERG_DECLARE_JSON_SERDE(CreateNamespaceRequest)
Expand Down
2 changes: 1 addition & 1 deletion src/iceberg/catalog/rest/type_fwd.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@

namespace iceberg::rest {

struct ErrorModel;
struct ErrorResponse;

class ErrorHandler;
class HttpClient;
Expand Down
18 changes: 4 additions & 14 deletions src/iceberg/catalog/rest/types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,37 +52,27 @@ struct ICEBERG_REST_EXPORT CatalogConfig {
};

/// \brief JSON error payload returned in a response with further details on the error.
struct ICEBERG_REST_EXPORT ErrorModel {
struct ICEBERG_REST_EXPORT ErrorResponse {
std::string message; // required
std::string type; // required
uint32_t code; // required
std::vector<std::string> stack;

/// \brief Validates the ErrorModel.
/// \brief Validates the ErrorResponse.
Status Validate() const {
if (message.empty() || type.empty()) {
return Invalid("Invalid error model: missing required fields");
return Invalid("Invalid error response: missing required fields");
}

if (code < 400 || code > 600) {
return Invalid("Invalid error model: code {} is out of range [400, 600]", code);
return Invalid("Invalid error response: code {} is out of range [400, 600]", code);
}

// stack is optional, no validation needed
return {};
}
};

/// \brief Error response body returned in a response.
struct ICEBERG_REST_EXPORT ErrorResponse {
ErrorModel error; // required

/// \brief Validates the ErrorResponse.
// We don't validate the error field because ErrorModel::Validate has been called in the
// FromJson.
Status Validate() const { return {}; }
};

/// \brief Request to create a namespace.
struct ICEBERG_REST_EXPORT CreateNamespaceRequest {
Namespace namespace_; // required
Expand Down
Loading
Loading