fix(cubestore): keep create table error messages concise

[paveltiunov]

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

When a pre-aggregation table failed to create in Cube Store, the error was several screens long:

• The driver wrapped the error with the full CREATE TABLE statement: every column, index and aggregate.
• Cube Store's finalization errors dumped the whole IdRow<Table> debug output, including every presigned S3 location URL.

Changes:

• CubeStoreDriver.createTableWithOptions: the error is now Error during create table <schema.table>: <error>.
• CubeStoreDriver.importStream: the temp-file upload error is now Error during upload of <file>: <error>. The unused SQL-building lines behind it are removed.
• Cube Store table_creator.rs: the timeout and orphaned-jobs finalization errors now include only the table name and id. The id matches the Cube Store log lines.

Example:

Error during create table CUBE_AGG_QA.earnings_member_rollup20240101_...: Internal: Timeout during create table finalization: earnings_member_rollup20240101_... (6122)

Added the create-table-error.test.ts unit test for the driver message. It failed before the fix and passes now. The Rust messages are format! strings on an error path that needs a live cluster to reach, so they have no dedicated test.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GbS6LWQAXSbT5JZqGBoFDE

[claude]

Claude finished @paveltiunov's task in 3m 23s —— View job

---

Review complete — change is sound; 2 medium, 1 low. No blockers.

Full review

• Read diff and changed files in full context
• Trace callers of createTableWithOptions and consumers of the old message text
• Check the Rust finalization paths
• Assess test coverage
• Handle prior review threads
• Post findings

Verdict

The core change is right. tableName is the schema-qualified name at every call site (CubeStoreDriver.ts:298, :348, :454, :493), so the new message stays identifying, and nothing in the repo parses the old Error during create table: <sql>: text — only BaseDriver.ts:569 and RedshiftDriver.ts:308 still use that format, and neither is on the Cube Store path.

One correction to the PR description: the presigned URLs were never in the JS-side message. createTableSqlWithOptions emits LOCATION ?, ? placeholders (CubeStoreDriver.ts:183) and pushes the files into params; e.message interpolated the unformatted sql. The URLs came from the Cube Store side, via the {:?} dump of IdRow<Table> — which table_creator.rs does fix. The JS change still earns its place (the statement itself was hundreds of columns wide), the attribution is just off.

Findings

Medium — CubeStoreDriver.ts:395: the same SQL dump survives in the upload-failure path. Posted inline (anchored on line 198, since 395 isn't in the diff). A failed POST to /upload-temp-file still interpolates the whole CREATE TABLE statement into an error about an HTTP upload. Removing it also retires the discarded createTableSql call at line 372 and the createTableSqlWithoutLocation variable at 374 with its eslint-disable no-unused-vars.

Medium — table_creator.rs:230/:244/:254: the table id is gone from the client-visible error. Posted inline. The adjacent log::error! and log::warn! identify the table by table.get_id(), and {:?} on IdRow<Table> used to carry it, so an operator could previously join the Cube-side error to the Cube Store log. get_table_name() is the bare name with no schema, and a retried build produces several ids under one name.

Low — no Rust-side test. create-table-error.test.ts covers the JS wrapper only; the three table_creator.rs messages have no test asserting the new format. Given they're format! strings on an error path that needs a live cluster to reach, that's a reasonable omission — noting it because the PR checklist claims tests were added for the changes.

Test verification

I could not run create-table-error.test.ts. This checkout has no node_modules and no built dist/, and jest.config.js matches <rootDir>/dist/test/*.(test|spec).(ts|js) — the suite needs a full workspace install plus tsc before it runs at all. Reviewed statically instead: the @cubejs-backend/native mock matches the precedent in websocket-connection.test.ts:20, the WebSocketConnection constructor is lazy so new FailingCubeStoreDriver() opens no socket and leaks no handle, the zero-arg query override is a valid narrowing of the base signature, and rejects.toThrow(new Error(...)) compares the message exactly — so the assertion does fail if the SQL comes back. I did not run cargo check on cubestore either; the Rust edit is a {:?} → {} swap over &String, type-correct by inspection.

Review threads

The listing returned 0 of my own threads (single page, hasNextPage: false) — no prior threads, so nothing to resolve and nothing to re-affirm.

· branch `pavel-claude/hopeful-faraday-mxax3e`

[claude]

Claude finished @paveltiunov's task in 3m 13s —— View job

---

Approve with one low-severity note — 0 high, 0 medium, 1 low. Posted inline.

Review details

• Read PR diff
• Read changed files in full context
• Verified Rust symbols and imports
• Verified test wiring
• Handled prior review threads
• Posted findings

Verified

• rust/cubestore/cubestore/src/sql/table_creator.rs — Table::get_table_name exists (metastore/table.rs:261) and returns &String; IdRow/Table are already imported (line 10–11). orphaned_error at line 739 is a free fn used before its definition, which Rust allows. No {:?} table dumps remain in the finalization path — the only surviving one is line 348 on a ColumnDef, unrelated.
• packages/cubejs-cubestore-driver/test/create-table-error.test.ts — the package's jest.config.js matches dist/test/*.(test|spec).(ts|js) and tsconfig.json includes test, so the new file compiles and runs alongside websocket-connection.test.ts. toThrow(new Error(...)) compares the message exactly, so it does pin the full string rather than a substring. CubeStoreDriver's constructor only constructs WebSocketConnection (which is lazy — no socket until initWebSocket), so the test needs no release() and leaks no handle.
• Removing this.createTableSql(table, columns) and createTableSqlWithoutLocation from importStream is pure dead-code removal: BaseDriver.createTableSql (cubejs-base-driver/src/BaseDriver.ts:574) only builds a string and cannot throw, so nothing validated earlier than it does now.
• No consumer parses these strings. The only other Error during create table: producers are BaseDriver.ts:569 and RedshiftDriver.ts:308, both independent, and nothing matches on the text.

Note on the PR description

The description says the old error carried "every presigned S3 LOCATION URL". It didn't, on either path:

• createTableWithOptions built the message from sql, and createTableSqlWithOptions emits LOCATION ?, ? with the URLs kept in params (CubeStoreDriver.ts:183-185). The formatSql inlining happens inside query(), after the string the error used.
• The upload error's createTableSqlWithoutLocation was built from an options that had no files yet, so it had no LOCATION clause at all.

The change is still worth making — the column list alone made these multi-screen — but nobody should read this PR as having closed a credentials-in-logs hole. If that's the actual concern, the path to look at is table_creator.rs:551, CubeError::user(format!("Create table failed: {}", e)), where e is the import job's own error and can carry the location.

Threads

No prior threads — the listing returned 0 claude-authored unresolved threads, so none were resolved and none re-affirmed.

· branch `pavel-claude/hopeful-faraday-mxax3e`

[paveltiunov]

integration (24.x, snowflake, true) and integration (26.x, snowflake, true) are failing, but not because of this PR. The Snowflake CI account now rejects logins with Multi-factor authentication is required for this account. Log in to Snowsight to enroll. (SnowflakeDriver.test.ts). The same two jobs fail on master at this PR's base commit 9070984 (run 35762779385). This PR doesn't touch the Snowflake driver, and no code change can fix this: the CI Snowflake user needs to be exempted from MFA or switched to key-pair auth.

---

Generated by Claude Code

[claude]

Claude finished @paveltiunov's task in 2m 51s —— View job

---

Approve — no blocking findings (0 high, 0 medium, 2 low, both out of scope / optional)

Full review

• Read the PR diff
• Read the changed files in full
• Check other call sites of the removed/changed error paths
• Review the new test
• Resolve stale review threads / dedupe findings
• Post review

What I verified

CubeStoreDriver.ts:198 — the wrapper now carries tableName only. The lost SQL is recoverable: Cube Store logs every failing command in full (rust/cubestore/cubestore/src/http/mod.rs:737, :838), and the one client-side failure that never reaches the server, MessageTooLargeError, already names both sizes itself (packages/cubejs-cubestore-driver/src/WebSocketConnection.ts:425-427). So no diagnostic is actually gone. This is the same point paveltiunov already made on the existing thread; nothing to add.

CubeStoreDriver.ts:369-371 (removed lines) — confirmed dead, not just unused. createTableSql → fromGenericType, and BaseDriver.fromGenericType (packages/cubejs-base-driver/src/BaseDriver.ts:595) is return columnType — it never throws, so the discarded call had no validating side effect and removing it does not move a fast failure to after the upload. createTableSqlWithOptions (CubeStoreDriver.ts:141-187) only reads options; it does not mutate the object that importStream later passes to createTableWithOptions at :450. grep finds no remaining reference to createTableSqlWithoutLocation.

table_creator.rs:230-234, :244, :252, :739-745 — IdRow and table::Table are both already in the use crate::metastore::{...} list at :10-13, so the module-level orphaned_error compiles where it sits. grep '{:?}' over the file leaves only :348 (Unexpected column expression), which is not a table dump — the debug-print of IdRow<Table>, and with it the presigned S3 URLs, is gone from every finalization path. The Err(e) branch at :254 propagates finalize_external_table's own errors, and those are already concise (Create table failed: {} at :551).

Test — new FailingCubeStoreDriver() is safe to construct with no server: WebSocketConnection's constructor (:105-112) only stores config; the socket is opened lazily in initWebSocket. The @cubejs-backend/native mock covers the only symbol the import chain pulls (parseCubestoreResultMessage). rejects.toThrow(new Error(...)) compares the message exactly, so the assertion does fail if the SQL is reintroduced — it is a real regression test, not a substring check.

I could not run the test or yarn tsc: node_modules is empty in this checkout and installing is not permitted here.

Low (not posted inline)

1. packages/cubejs-base-driver/src/BaseDriver.ts:569 and packages/cubejs-redshift-driver/src/RedshiftDriver.ts:308 still do e.message = \Error during create table: ${createTableSql}: ${e.message}`` — the same screens-long error this PR removes, on the non-Cube-Store path. Out of scope for this PR; worth a follow-up if the noise shows up there too. Fix this →

2. The second message change — Error during upload of ${fileName} at packages/cubejs-cubestore-driver/src/CubeStoreDriver.ts:391 — has no test; only createTableWithOptions is asserted. Covering it needs a node-fetch mock returning a non-200 with a JSON body, which is more setup than the one-line string is worth. Fine to leave.

Review threads

The unresolved-thread listing returned no prior threads of mine (0 unresolved threads on the PR at all), so nothing was resolved and nothing was re-affirmed. Both findings above came back POST, i.e. not duplicates — I am recording them here rather than inline because neither is actionable within this PR's scope.

· branch [`pavel-claude/hopeful-faraday-mxax3e`](https://github.com/cube-js/cube/tree/pavel-claude/hopeful-faraday-mxax3e)

[codecov]

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.67%. Comparing base (928e492) to head (9db43ec).

Files with missing lines	Patch %	Lines
...ges/cubejs-cubestore-driver/src/CubeStoreDriver.ts	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11961      +/-   ##
==========================================
+ Coverage   61.42%   61.67%   +0.24%     
==========================================
  Files         247      247              
  Lines       19834    19832       -2     
  Branches     4046     4046              
==========================================
+ Hits        12184    12231      +47     
+ Misses       7076     7027      -49     
  Partials      574      574              

Flag	Coverage Δ
cube-backend	61.67% <50.00%> (+0.24%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.