Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
17 changes: 15 additions & 2 deletions lib/kernel/KernelOperationBackend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()) };
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<OperationState> {
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()`
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/kernel/execution.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];

Expand All @@ -136,6 +138,9 @@ class FakeAsyncStatement {

public async status(): Promise<string> {
this.statusCalls += 1;
if (this.statusError) {
throw this.statusError;
}
return this.states.length > 1 ? (this.states.shift() as string) : this.states[0];
}

Expand Down Expand Up @@ -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
Expand Down
Loading