Skip to content

Commit c3a706e

Browse files
committed
feat(sea): enrich kernel error mapping with displayMessage/diagnosticInfo/errorDetailsJson
The napi error envelope already emits displayMessage / diagnosticInfo / errorDetailsJson, but SeaErrorMapping.buildKernelMetadata dropped them. Surface them under kernelMetadata so SEA stays at parity with: - Thrift, which carries the same data via OperationStateError.response (TGetOperationStatusResp.displayMessage / .errorMessage / .errorDetailsJson) - the Python use_kernel connector, which forwards all of them onto its exceptions Also make the kernelMetadata attach-guard a key-count check so future fields are covered automatically. Adds unit tests incl. explicit Thrift-parity assertions. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <madhavendra.rathore@databricks.com>
1 parent 45f202d commit c3a706e

2 files changed

Lines changed: 119 additions & 9 deletions

File tree

lib/sea/SeaErrorMapping.ts

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,18 @@ export interface KernelMetadata {
6868
httpStatus?: number;
6969
retryable?: boolean;
7070
queryId?: string;
71+
/**
72+
* Rich server-error diagnostics from the SEA terminal-error payload. These
73+
* mirror the fields the **Thrift** backend already surfaces via
74+
* `OperationStateError.response` (`TGetOperationStatusResp.displayMessage` /
75+
* `.errorMessage` / `.errorDetailsJson`) and that the Python `use_kernel`
76+
* connector forwards onto its exceptions — so SEA stays at parity rather
77+
* than dropping them. Kernel sources: `Error.display_message` /
78+
* `.diagnostic_info` / `.error_details_json`.
79+
*/
80+
displayMessage?: string;
81+
diagnosticInfo?: string;
82+
errorDetailsJson?: string;
7183
}
7284

7385
/**
@@ -209,6 +221,16 @@ function buildKernelMetadata(parsed: Record<string, unknown>): KernelMetadata {
209221
if (typeof parsed.queryId === 'string') {
210222
meta.queryId = parsed.queryId;
211223
}
224+
// Rich diagnostics — Thrift/Python parity (were previously dropped).
225+
if (typeof parsed.displayMessage === 'string') {
226+
meta.displayMessage = parsed.displayMessage;
227+
}
228+
if (typeof parsed.diagnosticInfo === 'string') {
229+
meta.diagnosticInfo = parsed.diagnosticInfo;
230+
}
231+
if (typeof parsed.errorDetailsJson === 'string') {
232+
meta.errorDetailsJson = parsed.errorDetailsJson;
233+
}
212234
return meta;
213235
}
214236

@@ -288,14 +310,9 @@ export function decodeNapiKernelError(err: unknown): Error {
288310
const meta = buildKernelMetadata(envelope);
289311
// Skip the namespace attachment entirely when no fields validated
290312
// through — keeps `err.kernelMetadata` absent rather than `{}` for
291-
// simple envelopes (the common case).
292-
if (
293-
meta.errorCode !== undefined ||
294-
meta.vendorCode !== undefined ||
295-
meta.httpStatus !== undefined ||
296-
meta.retryable !== undefined ||
297-
meta.queryId !== undefined
298-
) {
313+
// simple envelopes (the common case). Key-count check so new
314+
// `KernelMetadata` fields are covered automatically.
315+
if (Object.keys(meta).length > 0) {
299316
defineErrorMetadata(jsErr, 'kernelMetadata', meta);
300317
}
301318
return jsErr;

tests/unit/sea/error-mapping.test.ts

Lines changed: 94 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { expect } from 'chai';
2-
import { mapKernelErrorToJsError, KernelErrorCode, KernelErrorShape } from '../../../lib/sea/SeaErrorMapping';
2+
import {
3+
mapKernelErrorToJsError,
4+
decodeNapiKernelError,
5+
KernelErrorCode,
6+
KernelErrorShape,
7+
ErrorWithSqlState,
8+
} from '../../../lib/sea/SeaErrorMapping';
39
import HiveDriverError from '../../../lib/errors/HiveDriverError';
410
import AuthenticationError from '../../../lib/errors/AuthenticationError';
511
import OperationStateError, { OperationStateErrorCode } from '../../../lib/errors/OperationStateError';
@@ -219,3 +225,90 @@ describe('SeaErrorMapping.mapKernelErrorToJsError', () => {
219225
});
220226
});
221227
});
228+
229+
describe('SeaErrorMapping.decodeNapiKernelError — diagnostics enrichment (Thrift/Python parity)', () => {
230+
// The wire sentinel the napi binding prefixes onto structured kernel errors
231+
// (mirrors `__databricks_error__:` in SeaErrorMapping.ts / native/sea/src/error.rs).
232+
const SENTINEL = '__databricks_error__:';
233+
const envelope = (fields: Record<string, unknown>): Error => new Error(SENTINEL + JSON.stringify(fields));
234+
235+
it('surfaces displayMessage / diagnosticInfo / errorDetailsJson in kernelMetadata (were dropped before)', () => {
236+
const err = decodeNapiKernelError(
237+
envelope({
238+
code: 'SqlError',
239+
message: '[TABLE_OR_VIEW_NOT_FOUND] table not found',
240+
sqlState: '42P01',
241+
errorCode: 'TABLE_OR_VIEW_NOT_FOUND',
242+
vendorCode: 0,
243+
displayMessage: 'TABLE_OR_VIEW_NOT_FOUND: `main`.`x` cannot be found',
244+
diagnosticInfo: 'org.apache.spark.sql.AnalysisException: ...',
245+
errorDetailsJson: '{"errorClass":"TABLE_OR_VIEW_NOT_FOUND"}',
246+
}),
247+
) as ErrorWithSqlState;
248+
249+
// Class parity with Thrift: a server SQL failure → OperationStateError(Error).
250+
expect(err).to.be.instanceOf(OperationStateError);
251+
expect((err as OperationStateError).errorCode).to.equal(OperationStateErrorCode.Error);
252+
253+
// The three previously-dropped diagnostics are now surfaced.
254+
expect(err.kernelMetadata?.displayMessage).to.equal('TABLE_OR_VIEW_NOT_FOUND: `main`.`x` cannot be found');
255+
expect(err.kernelMetadata?.diagnosticInfo).to.match(/AnalysisException/);
256+
expect(err.kernelMetadata?.errorDetailsJson).to.equal('{"errorClass":"TABLE_OR_VIEW_NOT_FOUND"}');
257+
});
258+
259+
it('keeps the existing envelope fields alongside the new diagnostics', () => {
260+
const err = decodeNapiKernelError(
261+
envelope({
262+
code: 'SqlError',
263+
message: 'boom',
264+
sqlState: '42000',
265+
errorCode: 'PARSE_SYNTAX_ERROR',
266+
vendorCode: 1234,
267+
httpStatus: 400,
268+
retryable: false,
269+
queryId: '01ef-abcd',
270+
displayMessage: 'Syntax error',
271+
}),
272+
) as ErrorWithSqlState;
273+
expect(err.sqlState).to.equal('42000');
274+
expect(err.kernelMetadata?.errorCode).to.equal('PARSE_SYNTAX_ERROR');
275+
expect(err.kernelMetadata?.vendorCode).to.equal(1234);
276+
expect(err.kernelMetadata?.queryId).to.equal('01ef-abcd');
277+
expect(err.kernelMetadata?.displayMessage).to.equal('Syntax error');
278+
});
279+
280+
it('attaches kernelMetadata even when ONLY a diagnostic field is present', () => {
281+
// Previously the attach-guard enumerated only errorCode/vendorCode/httpStatus/
282+
// retryable/queryId, so a diagnostics-only envelope dropped them silently.
283+
const err = decodeNapiKernelError(
284+
envelope({ code: 'Internal', message: 'kaboom', diagnosticInfo: 'stack trace here' }),
285+
) as ErrorWithSqlState;
286+
expect(err.kernelMetadata?.diagnosticInfo).to.equal('stack trace here');
287+
});
288+
289+
it('omits kernelMetadata entirely for a bare envelope (no optional fields)', () => {
290+
const err = decodeNapiKernelError(envelope({ code: 'Internal', message: 'plain' })) as ErrorWithSqlState;
291+
expect(err.kernelMetadata).to.equal(undefined);
292+
});
293+
294+
it('Thrift parity: SEA exposes the same diagnostics Thrift carries via OperationStateError.response', () => {
295+
// Thrift's OperationStateError carries the full TGetOperationStatusResp on
296+
// `.response` (displayMessage + sqlState + numeric errorCode + diagnostics).
297+
// SEA must expose the equivalent set so a consumer gets parity regardless of
298+
// backend: same class + sqlState + a diagnostics surface.
299+
const err = decodeNapiKernelError(
300+
envelope({
301+
code: 'SqlError',
302+
message: 'div by zero',
303+
sqlState: '22012',
304+
vendorCode: 0,
305+
displayMessage: 'Division by zero',
306+
diagnosticInfo: 'ArithmeticException',
307+
}),
308+
) as ErrorWithSqlState;
309+
expect(err).to.be.instanceOf(OperationStateError); // class parity (Thrift: OperationStateError)
310+
expect(err.sqlState).to.equal('22012'); // Thrift: response.sqlState
311+
expect(err.kernelMetadata?.displayMessage).to.equal('Division by zero'); // Thrift: response.displayMessage
312+
expect(err.kernelMetadata?.diagnosticInfo).to.equal('ArithmeticException'); // Thrift: response.errorMessage
313+
});
314+
});

0 commit comments

Comments
 (0)