feat(cubesql): Universally prefix all errors

[MazterQyou]

Check List

• Tests have been run in packages where changes have been made if available
• Linter has been run for changed code
• Tests for the changes have been added if not covered yet
• Docs have been added / updated if required

Description of Changes Made

This PR makes an effort to universally prefix all errors with preset messages, and unwrap error onions wherever possible.

[claude]

Claude finished @MazterQyou's task in 6m 3s —— View job

---

Code Review: Universally prefix all errors

• Gather context and read changed files
• Review core error infrastructure (error.rs, compile/error.rs)
• Review Postgres error handling changes
• Review backend-native transport changes
• Review test and snapshot changes
• Post comprehensive review feedback

---

Summary

This is a well-structured PR that introduces a systematic error taxonomy across CubeSQL. The core idea — replacing the old 2-variant User/Internal classification with 10 specific error categories and adding uniform Display prefixes — is sound and addresses a real observability gap.

What works well

1. Error onion unwrapping: The new From<DataFusionError> and From<ArrowError> implementations properly unwrap nested error types (Plan, Execution, SQL, NotImplemented, etc.) into the correct CubeSQL categories instead of flattening everything to Internal. This preserves valuable context.

2. ContinueWait as a typed variant: Replacing fragile e.message.to_lowercase().contains("continue wait") string checks with a proper ContinueWait enum variant is a significant reliability improvement.

3. Bidirectional From conversions: Adding From<CubeError> for CompilationError alongside the existing reverse direction enables the ? operator to work naturally, eliminating many .map_err(|e| CompilationError::internal(e.to_string())) wrappers that were flattening error context.

4. meta() accessor: The CubeErrorCauseType::meta() helper cleanly centralizes the metadata extraction pattern.

5. Postgres error code mapping consolidation: The old df_to_error_response/arrow_to_error_response recursive unwrapping chain is replaced by converting to CubeError first and mapping cause types to PG codes. Much cleaner.

6. Error reclassification: Correct fixes like CubeError::user → CubeError::internal for transport parsing failures, JS bridge deserialization errors, and "unexpected response" errors. These are genuinely internal issues, not user-caused.

Issues & suggestions

Medium priority:

• http_error.rs catch-all hides user-actionable errors — The _ => "Internal Server Error" catch-all now covers SQLParser, Planning, and Unsupported errors which contain user-actionable information. If this HTTP gateway is user-facing, consider allowing these categories to expose their messages. See inline comment.

• PG error code for Planning errors — SyntaxErrorOrAccessRuleViolation (42000) may not be the best fit for all planning errors (e.g., join validation failures, missing cube members). See inline comment.

Low priority / nits:

• Variant naming convention — RESTAPI and SQLParser don't follow Rust PascalCase convention. See inline comment.

• Boilerplate growth — Each CompilationError method (PartialEq, to_backtrace, message, with_message, with_meta) repeats the full variant list. A macro could reduce this. See inline comment.

• Minor double-description — rust_decimal::Error formats as "Internal Error: Decimal Error: ...". See inline comment.

Testing

The test changes look correct:

• Error message assertions updated to match new prefix format (e.g., "Planning Error: ..." instead of "Error during planning: SQLCompilationError: Internal: ...")
• Snapshot files updated consistently
• The message() method continues to return the raw message without prefix, while to_string()/Display includes the prefix — test assertions use the right one

Security

No security concerns. The http_error.rs catch-all correctly hides non-User error details from HTTP responses, preventing information leakage.

Overall

Solid improvement to error handling infrastructure. The error taxonomy is well-thought-out and the conversion pipelines are correct. The main risk area is ensuring downstream consumers (logs, HTTP responses, PG wire protocol) get the right level of detail — the inline comments highlight the specific spots to verify.

[claude]

Minor: double-description in formatted message — This produces CubeError::internal("Decimal Error: ..."), which when displayed via CubeError::Display becomes "Internal Error: Decimal Error: ...". The nested prefix reads a bit awkwardly. Consider either:

• Dropping the "Decimal Error: " prefix and relying on the Internal Error: prefix from Display, or
• Using a dedicated error variant if decimal errors need distinct handling

This is very minor and non-blocking.

[claude]

Good refactor — Consolidating the error-to-PG-code mapping into a single cube_to_error_response based on CubeErrorCauseType is much cleaner than the old recursive df_to_error_response / arrow_to_error_response chain that tried to unwrap errors manually.

A few observations on the PG error code mappings:

• Planning → SyntaxErrorOrAccessRuleViolation (42000): This is the class-level code. Cube planning errors can be broader than syntax issues (e.g., join validation failures). Consider whether InternalError might be more accurate for some planning failures.
• User → InvalidSqlStatement (26000): This PG code specifically means "invalid SQL statement name" (prepared statement naming). This mapping predates this PR, but if you're touching error codes anyway, 42000 might be semantically closer for general user errors.
• ContinueWait → SqlStatementNotYetComplete (03000): Creative use of this code. The comment "Should never happen" is reassuring — worth adding a log::warn! if it does trigger unexpectedly.

[claude]

Suggestion: consider a macro or helper for PartialEq — This block is very repetitive. Each new variant adds 3 lines of identical structure. A macro like:

macro_rules! match_variant_eq {
    ($self:expr, $other:expr, $($Variant:ident),+) => {
        match ($self, $other) {
            $( (CompilationError::$Variant(left, ..), CompilationError::$Variant(right, ..)) => left == right, )+
            (CompilationError::ContinueWait, CompilationError::ContinueWait) => true,
            _ => false,
        }
    }
}

could reduce this significantly. Same applies to to_backtrace(), to_backtrace_mut(), message(), with_message(), and with_meta(). Not blocking — just noting that the boilerplate will grow linearly with each new error variant.

[claude]

Good: eliminates fragile string matching — The old code checked e.message.to_lowercase().contains("continue wait") to detect continue-wait signals. Using a dedicated CubeError::continue_wait() with a ContinueWait cause type is much more robust and self-documenting.

Note: there are still string-based "continue wait" checks in this file (lines 463 and 612). These handle errors coming from the JS side where you can't control the error type. Worth a follow-up comment explaining why these remain string-based (i.e., JS bridge limitation).

[claude]

Correct change — The catch-all _ => is necessary now that CubeErrorCauseType has many more variants. All non-User errors correctly return HTTP 500 with a generic message, preventing internal error details from leaking to HTTP clients.

Note: this means new error types like SQLParser, Planning, Unsupported etc. will all show "Internal Server Error" to HTTP callers, even though some (like SQLParser) are clearly user-caused. If this gateway is user-facing, you may want to expand which cause types show their real message (e.g., SQLParser, Unsupported, Planning could arguably be safe to expose).