Skip to content

Commit 88e57d4

Browse files
keydunovclaude
andauthored
fix(cubejs-client-core): surface cubeSql error chunks instead of dropping rows (cube-js#11097)
* fix(cubejs-client-core): surface cubeSql error chunks instead of dropping rows The cubesql endpoint streams JSONL (schema line, then data lines), and on a post-processing failure appends a trailing `{ error }` chunk. The non-streaming `cubeSql()` parser mapped every non-schema line through `JSON.parse(d).data`, so the error line became an `undefined` phantom row and the failure was silently swallowed — callers saw a "successful" result with a null row. Classify each line and throw on an `error` chunk, mirroring how `cubeSqlStream()` already handles it. Genuine parse failures still fall back to the raw response body. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(cubejs-client-core): satisfy eslint in cubeSql error handling Drop the `no-continue` violation by inverting the loop guard, and switch the regression test's error strings to single quotes per the lint rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3eae361 commit 88e57d4

2 files changed

Lines changed: 70 additions & 9 deletions

File tree

packages/cubejs-client-core/src/index.ts

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -786,19 +786,47 @@ class CubeApi {
786786

787787
const [schema, ...data] = response.error.split('\n');
788788

789+
let parsedSchema: any;
789790
try {
790-
const parsedSchema = JSON.parse(schema);
791-
return {
792-
schema: parsedSchema.schema,
793-
data: data
794-
.filter((d: string) => d.trim().length)
795-
.map((d: string) => JSON.parse(d).data)
796-
.reduce((a: any, b: any) => a.concat(b), []),
797-
...(parsedSchema.lastRefreshTime ? { lastRefreshTime: parsedSchema.lastRefreshTime } : {}),
798-
};
791+
parsedSchema = JSON.parse(schema);
799792
} catch (err) {
793+
// Schema line isn't valid JSON — the whole `error` payload is a real error.
800794
throw new Error(response.error);
801795
}
796+
797+
const rows: any[] = [];
798+
799+
for (const line of data) {
800+
if (line.trim().length) {
801+
let parsed: any;
802+
try {
803+
parsed = JSON.parse(line);
804+
} catch (err) {
805+
// A non-JSON line after a valid schema means a malformed payload — fall
806+
// back to surfacing the raw response rather than dropping rows silently.
807+
throw new Error(response.error);
808+
}
809+
810+
// The stream can interleave an error chunk after the schema (e.g. a
811+
// post-processing/cast error surfaced mid-result). Such a line has no
812+
// `data`, so the previous `JSON.parse(d).data` concat pushed an `undefined`
813+
// "phantom" row and silently swallowed the failure. Surface it instead —
814+
// matching how `cubeSqlStream` classifies `error` chunks.
815+
if (parsed.error) {
816+
throw new Error(parsed.error);
817+
}
818+
819+
if (parsed.data) {
820+
rows.push(...parsed.data);
821+
}
822+
}
823+
}
824+
825+
return {
826+
schema: parsedSchema.schema,
827+
data: rows,
828+
...(parsedSchema.lastRefreshTime ? { lastRefreshTime: parsedSchema.lastRefreshTime } : {}),
829+
};
802830
},
803831
options,
804832
callback

packages/cubejs-client-core/test/CubeApi.test.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -427,6 +427,21 @@ describe('CubeApi cubeSql', () => {
427427
JSON.stringify({ data: [['Active']] }),
428428
].join('\n');
429429

430+
// The backend streams a schema chunk, then (on a post-processing failure) an error
431+
// chunk. The error must surface as a rejection instead of being concatenated as an
432+
// `undefined` phantom row.
433+
const cubeSqlResponseBodyWithError = [
434+
JSON.stringify({
435+
schema: [
436+
{ name: 'created_date', column_type: 'String' },
437+
],
438+
}),
439+
JSON.stringify({
440+
error: 'Post-Processing Error: Cast error: Error parsing \'2026-05-01\' as timestamp',
441+
requestId: '2fbe44e4-df6f-420d-ae39-376c802323b4-span-1',
442+
}),
443+
].join('\n');
444+
430445
test('should parse lastRefreshTime from response', async () => {
431446
vi.spyOn(HttpTransport.prototype, 'request').mockImplementation(() => ({
432447
subscribe: (cb) => Promise.resolve(cb({
@@ -471,6 +486,24 @@ describe('CubeApi cubeSql', () => {
471486
expect(res.schema).toEqual([{ name: 'status', column_type: 'String' }]);
472487
expect(res.data).toEqual([['Active']]);
473488
});
489+
490+
test('should surface an error chunk that follows the schema instead of swallowing it', async () => {
491+
vi.spyOn(HttpTransport.prototype, 'request').mockImplementation(() => ({
492+
subscribe: (cb) => Promise.resolve(cb({
493+
status: 200,
494+
text: () => Promise.resolve(JSON.stringify({ error: cubeSqlResponseBodyWithError })),
495+
} as any,
496+
async () => undefined as any))
497+
}));
498+
499+
const cubeApi = new CubeApi('token', {
500+
apiUrl: 'http://localhost:4000/cubejs-api/v1',
501+
});
502+
503+
await expect(
504+
cubeApi.cubeSql('SELECT created_date FROM deals')
505+
).rejects.toThrow('Post-Processing Error: Cast error: Error parsing \'2026-05-01\' as timestamp');
506+
});
474507
});
475508

476509
describe('CubeApi with baseRequestId', () => {

0 commit comments

Comments
 (0)