fix(cubejs-client-core): surface cubeSql error chunks instead of dropping rows

[keydunov]

Problem

The cubesql endpoint streams JSONL — a schema line, then data lines — and on a post-processing failure appends a trailing { "error": "...", "requestId": "..." } chunk. The non-streaming cubeSql() parser mapped every non-schema line through JSON.parse(d).data:

const [schema, ...data] = response.error.split('\n');
data.filter(d => d.trim().length).map(d => JSON.parse(d).data).reduce((a,b) => a.concat(b), [])

For an error chunk, JSON.parse(d).data is undefined, so it gets concatenated as a phantom undefined row and the failure is silently swallowed — callers receive a "successful" result with a null row instead of an error. Downstream this surfaced as e.g. a workbook table crashing on the null row, with the actual cast-error message lost.

cubeSqlStream() already classifies error chunks correctly (it yields { type: 'error' }); only the non-streaming cubeSql() was affected.

Fix

Classify each JSONL line and throw on an error chunk, mirroring cubeSqlStream(). Genuine parse failures (malformed schema/data line) still fall back to throwing the raw response body, preserving existing behavior.

Testing

Added a regression test to CubeApi.test.ts using the exact real-world payload (schema line followed by a post-processing cast-error chunk) asserting cubeSql() rejects with the clean message rather than resolving with a phantom row. All cubeSql tests pass.

🤖 Generated with Claude Code

[keydunov]

Consumer-side fixes (ai-engineer inline parsers + console-ui table hardening) live in cubedevinc/cubejs-enterprise#12668.

[claude]

Claude finished @keydunov's task in 43s —— View job

---

Code Review

The fix correctly addresses the silent error-swallowing bug. The new for...of loop classifies each JSONL line and throws on an error chunk — mirroring cubeSqlStream (index.ts:886, 917). The regression test uses a realistic payload and asserts the right behavior.

Review details — looks good, a few small observations

What's good

• Behavior now matches cubeSqlStream() for error chunks (consistency between the two APIs).
• Schema-parse failure path is preserved (raw response.error thrown), so existing behavior for malformed schemas is unchanged.
• The added comment in index.ts:813-817 clearly captures why the previous code silently produced phantom rows — exactly the kind of non-obvious context worth keeping.
• Test uses the real-world payload from the incident; good regression coverage.

Minor observations (non-blocking)

1. parsed.data truthy-check at index.ts:824 — if (parsed.data) skips falsy values including null, 0, "". In practice data is always an array from the server, so this is fine, but a stricter Array.isArray(parsed.data) would be slightly more defensive and document the contract. Not worth changing unless you want belt-and-suspenders.

2. Chunk classification order — currently error is checked before data. Matches cubeSqlStream semantics (error chunks have no data field), so this is correct. Worth noting that if a future server change ever sends { data: [...], error: "..." } in the same chunk, this would throw and discard the partial data — but that's the right call (fail loud).

3. Test isolation — cubeSqlResponseBodyWithError is declared at the describe block scope alongside the existing fixture; nicely consistent with the existing pattern.

Security / Performance

Nothing to flag. The change is parse-logic only; no new I/O, no surface area for injection, and the iteration cost is identical to the prior .filter().map().reduce() chain.

• branch [`artyom/fix-cubesql-error-swallow`](https://github.com/cube-js/cube/tree/artyom/fix-cubesql-error-swallow)

[claude]

Claude finished @keydunov's task in 29s —— View job

---

Code Review

The latest commit (f860788) is an eslint-only follow-up to the substantive fix already reviewed in this thread. The fix and test still look correct.

Review details

Verdict

LGTM — no new issues. 0 high / 0 medium / 0 low.

Re-reading the diff

• packages/cubejs-client-core/src/index.ts:799-823 — classification loop is unchanged from the prior review; error chunk is checked before data, mirroring cubeSqlStream() semantics at index.ts:886/917. Malformed-JSON fallback (line 807) still preserves the original raw-body behavior.
• packages/cubejs-client-core/test/CubeApi.test.ts:490-506 — regression test uses the real-world payload and asserts the clean error message bubbles up instead of a phantom undefined row.
• The new commit only resolves eslint complaints; no behavioral delta.

Code quality / security / perf

Nothing to flag. Parse-only change, no new I/O or untrusted-input expansion. Iteration cost matches the prior .filter().map().reduce().

Testing

Coverage is adequate for the bug class. A nice-to-have (non-blocking) would be a test for an error chunk appearing between data chunks (data, then error, then more data) to lock in fail-loud-and-discard-partial semantics, but the current test exercises the reported real-world shape.

Prior thread

Already reviewed and approved by @vasilev-alex.

• branch [`artyom/fix-cubesql-error-swallow`](https://github.com/cube-js/cube/tree/artyom/fix-cubesql-error-swallow)