From 548eb2110757e8356fcb42a43867c5de5f0b9ffc Mon Sep 17 00:00:00 2001 From: Vu Anh Phung Date: Fri, 13 Nov 2026 08:27:35 +0000 Subject: [PATCH] fix(kernel): decode a failed async status poll into the driver error On a runAsync: true statement, waitUntilReady() and status() awaited the napi AsyncStatement.status() without decodeNapiKernelError, so a failed poll (for example, retries exhausted on HTTP 500) reached the caller as a raw Error whose message is the __databricks_error__:{...} envelope. Decode it like the other napi calls. Co-authored-by: Isaac Signed-off-by: Vu Anh Phung --- CHANGELOG.md | 1 + lib/kernel/KernelOperationBackend.ts | 17 +++++++++++++++-- tests/unit/kernel/execution.test.ts | 27 +++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 34532b62..b22fb903 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Kernel backend: a failed status poll on a `runAsync: true` statement (for example, retries exhausted on HTTP 5xx) now throws the mapped driver error, such as `HiveDriverError: HTTP 500: Internal Server Error`, instead of a raw `Error` whose message is the internal `__databricks_error__:{...}` envelope. - Thrift CloudFetch downloads are now bounded end to end by `retriesTimeout` (default 15 minutes), including every attempt and retry backoff. Previously a stalled download, where the response headers or body never arrived, blocked the fetch forever. A download still running when the deadline expires is aborted and fails with `RetryError` (`TIMEOUT_EXCEEDED`, message `CloudFetch result download failed: Retry timeout exceeded`). (#520) - Thrift connections made through `proxy` now apply `checkServerCertificate`, `customCaCert` and `clientCert`/`clientKey`. Previously the proxied path ignored them and verified against Node's process-wide trust store, so a certificate outside the configured trust set could be accepted. **Compatibility:** proxied connections now verify unless `checkServerCertificate: false` is set; as on direct connections, `NODE_TLS_REJECT_UNAUTHORIZED=0` no longer turns verification off. For an `https` proxy these options also apply to the proxy's own certificate. (#485) - Honor the client `useCloudFetch: false` default on the kernel backend so complete results can be fetched without cloud-storage downloads. (#574) diff --git a/lib/kernel/KernelOperationBackend.ts b/lib/kernel/KernelOperationBackend.ts index 41f15de2..1a42e446 100644 --- a/lib/kernel/KernelOperationBackend.ts +++ b/lib/kernel/KernelOperationBackend.ts @@ -426,7 +426,7 @@ export default class KernelOperationBackend implements IOperationBackend { // `numModifiedRows` from the inline count on the terminal response), so // surface them — mirroring the sync path. Reading them does not fetch the // result. - const state = statusStringToOperationState(await this.asyncStatement.status()); + const state = await this.pollAsyncStatus(); if (state === OperationState.Succeeded) { this.asyncSucceeded = true; return { state, hasResultSet: true, ...(await this.readRichStatusFields()) }; @@ -624,7 +624,7 @@ export default class KernelOperationBackend implements IOperationBackend { failIfNotActive(this.lifecycle); // eslint-disable-next-line no-await-in-loop - const state = statusStringToOperationState(await this.asyncStatement!.status()); + const state = await this.pollAsyncStatus(); // Record success before the progress callback runs, so a callback that // throws on the Succeeded tick still leaves `isFinished()` true. if (state === OperationState.Succeeded) { @@ -721,6 +721,19 @@ export default class KernelOperationBackend implements IOperationBackend { } } + /** + * One `AsyncStatement.status()` poll. A failed poll (e.g. retries exhausted on + * a 5xx) rejects with the binding's error envelope, which is decoded here like + * every other napi call. + */ + private async pollAsyncStatus(): Promise { + try { + return statusStringToOperationState(await this.asyncStatement!.status()); + } catch (err) { + throw decodeNapiKernelError(err); + } + } + /** * Drive `awaitResult()` on a Failed statement to surface the kernel's typed * SQL-error envelope. Falls back to a generic error if `awaitResult()` diff --git a/tests/unit/kernel/execution.test.ts b/tests/unit/kernel/execution.test.ts index 79157f77..7c640bcb 100644 --- a/tests/unit/kernel/execution.test.ts +++ b/tests/unit/kernel/execution.test.ts @@ -122,6 +122,8 @@ class FakeAsyncStatement { public awaitResultError: Error | null = null; + public statusError: Error | null = null; + // Successive status() returns drain this queue; the last value sticks. private readonly states: string[]; @@ -136,6 +138,9 @@ class FakeAsyncStatement { public async status(): Promise { this.statusCalls += 1; + if (this.statusError) { + throw this.statusError; + } return this.states.length > 1 ? (this.states.shift() as string) : this.states[0]; } @@ -1465,6 +1470,28 @@ describe('KernelOperationBackend — async (submitStatement) path', () => { expect((thrown as Error).message).to.match(/TABLE_OR_VIEW_NOT_FOUND/); }); + it('waitUntilReady() and status() decode a failed status poll instead of leaking the envelope', async () => { + const envelope = `__databricks_error__:${JSON.stringify({ + code: 'Internal', + httpStatus: 500, + message: 'HTTP 500: Internal Server Error', + })}`; + for (const call of ['waitUntilReady', 'status'] as const) { + const stmt = new FakeAsyncStatement('Running'); + stmt.statusError = new Error(envelope); + const op = makeAsyncOp(stmt); + let thrown: unknown; + try { + // eslint-disable-next-line no-await-in-loop + await (call === 'waitUntilReady' ? op.waitUntilReady() : op.status(false)); + } catch (err) { + thrown = err; + } + expect(thrown, call).to.be.instanceOf(HiveDriverError); + expect((thrown as Error).message, call).to.equal('HTTP 500: Internal Server Error'); + } + }); + // A server-driven terminal state MUST throw OperationStateError (not a plain // HiveDriverError) so the DBSQLOperation facade — which only mirrors its // cancelled/closed flags when `err instanceof OperationStateError` — stays in