Skip to content

Commit 550c8e2

Browse files
committed
Revert error parity changes
Signed-off-by: Cathleen Yan <cathleen.yan@databricks.com>
1 parent 92a23ed commit 550c8e2

8 files changed

Lines changed: 62 additions & 305 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,6 @@
55
- Kernel backend (`useKernel: true`): preserve qualified `INTERVAL MONTH` and
66
`INTERVAL DAY` parameter types on the SEA wire by using the kernel raw-parameter
77
path.
8-
- Error handling (all backends): `StatusError` now extends `HiveDriverError` (and
9-
therefore native `Error`). `instanceof Error` / `instanceof HiveDriverError`
10-
catches now match it, SQLSTATE is retained, and server error code `0` is no
11-
longer replaced with `-1`. Kernel request-time SQL failures use this same shape.
128
- Kernel backend source builds (`useKernel: true`, built from `KERNEL_REV`): `getTypeInfo()` now matches the Thrift backend's canonical 18-column, 20-row type-info result. Customer-facing npm installs require a follow-up bump to a published native package containing this Kernel change. ([databricks-sql-kernel#291](https://github.com/databricks/databricks-sql-kernel/pull/291), PECOBLR-4166)
139
- Kernel backend (`useKernel: true`): **Azure Entra (Azure AD) auth is now threaded through the kernel path.** On `authType: 'databricks-oauth'`: **U2M** (no secret) always routes to `OAuthU2m` — the kernel runs one cloud-blind in-house workspace-federated browser flow (it uses the workspace's OIDC-discovered authorize endpoint verbatim), which works against Azure workspaces, so Azure U2M forwards the in-house app (`databricks-sql-connector`) + `sql offline_access` scopes exactly like AWS/GCP, regardless of `useDatabricksOAuthInAzure` (verified E2E against a live Azure workspace). **M2M** (secret): `useDatabricksOAuthInAzure: true` (or non-Azure) → `OAuthM2m` (workspace-OIDC client-credentials); an Azure host with `useDatabricksOAuthInAzure` absent/`false` → the Entra-direct Azure service-principal M2M (`AzureSpM2m`, the Entra SP creds ride `oauthClientId`/`oauthClientSecret`, `azureTenantId` optional and auto-discovered when omitted). On a non-Azure host `useDatabricksOAuthInAzure` is inert. The `AzureSpM2m` path requires a `databricks-sql-kernel` native module that exposes the Azure SP surface — landed on `main` via [databricks-sql-kernel#282](https://github.com/databricks/databricks-sql-kernel/pull/282) (which the pinned `KERNEL_REV` `ef1a6f2` carries; the surface was originally proposed in [#280](https://github.com/databricks/databricks-sql-kernel/pull/280), which never reached `main`); U2M works on any kernel build. (PECOBLR-4141 / PECOBLR-4120)
1410

‎lib/errors/StatusError.ts‎

Lines changed: 11 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,21 @@
1-
import HiveDriverError from './HiveDriverError';
1+
import { TStatus } from '../../thrift/TCLIService_types';
22

3-
/** Protocol-neutral details for constructing a driver StatusError. */
4-
export interface StatusErrorOptions {
5-
message?: string;
6-
code?: number;
7-
sqlState?: string;
8-
infoMessages?: ReadonlyArray<string>;
9-
}
10-
11-
/**
12-
* Legacy shape accepted by the original constructor. Keeping this structural
13-
* type means existing Thrift call sites remain source-compatible without making
14-
* StatusError itself depend on generated Thrift types.
15-
*/
16-
interface LegacyStatusErrorOptions {
17-
statusCode?: number;
18-
errorMessage?: string;
19-
errorCode?: number;
20-
sqlState?: string;
21-
infoMessages?: ReadonlyArray<string>;
22-
}
3+
export default class StatusError implements Error {
4+
public name: string;
235

24-
export default class StatusError extends HiveDriverError {
25-
public name = 'Status Error';
6+
public message: string;
267

278
public code: number;
289

29-
public sqlState?: string;
30-
31-
constructor(options: StatusErrorOptions | LegacyStatusErrorOptions) {
32-
const { message, code } = options as StatusErrorOptions;
33-
const { errorMessage, errorCode } = options as LegacyStatusErrorOptions;
34-
const normalizedMessage = message ?? errorMessage ?? '';
35-
super(normalizedMessage);
10+
public stack?: string;
3611

37-
this.code = code ?? errorCode ?? -1;
38-
this.sqlState = options.sqlState;
12+
constructor(status: TStatus) {
13+
this.name = 'Status Error';
14+
this.message = status.errorMessage || '';
15+
this.code = status.errorCode || -1;
3916

40-
if (Array.isArray(options.infoMessages)) {
41-
this.stack = options.infoMessages.join('\n');
17+
if (Array.isArray(status.infoMessages)) {
18+
this.stack = status.infoMessages.join('\n');
4219
}
4320
}
4421
}

‎lib/kernel/KernelErrorMapping.ts‎

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import HiveDriverError from '../errors/HiveDriverError';
22
import AuthenticationError from '../errors/AuthenticationError';
33
import OperationStateError, { OperationStateErrorCode } from '../errors/OperationStateError';
44
import ParameterError from '../errors/ParameterError';
5-
import StatusError from '../errors/StatusError';
65

76
/**
87
* Sentinel prefix the napi binding's `napi_err_from_kernel` puts on
@@ -28,11 +27,6 @@ export interface KernelErrorShape {
2827
message: string;
2928
/** Optional SQLSTATE — five-char alphanumeric, when the kernel was able to surface it. */
3029
sqlstate?: string;
31-
/**
32-
* Server statement id attached to SQL failures. An empty id means the
33-
* ExecuteStatement request itself was rejected before an operation existed.
34-
*/
35-
queryId?: string;
3630
}
3731

3832
/**
@@ -126,12 +120,10 @@ function defineErrorMetadata<K extends string, V>(error: Error, key: K, value: V
126120
* Cancelled → OperationStateError(Canceled)
127121
* Timeout → OperationStateError(Timeout)
128122
* InvalidArgument → ParameterError
129-
* SqlError, empty queryId → StatusError
130-
* SqlError, non-empty/missing queryId→ OperationStateError(Error)
131123
* NetworkError, Unavailable,
132124
* NotFound, ResourceExhausted,
133125
* DataLoss, Internal,
134-
* InvalidStatementHandle → HiveDriverError
126+
* InvalidStatementHandle, SqlError → HiveDriverError
135127
*
136128
* Unknown `code` values (e.g. if the kernel adds a new variant) fall through
137129
* to HiveDriverError so the driver never silently drops an error. The kernel's
@@ -141,7 +133,7 @@ function defineErrorMetadata<K extends string, V>(error: Error, key: K, value: V
141133
* class is returned.
142134
*/
143135
export function mapKernelErrorToJsError(kErr: KernelErrorShape): ErrorWithSqlState {
144-
const { code, message, sqlstate, queryId } = kErr;
136+
const { code, message, sqlstate } = kErr;
145137

146138
let error: ErrorWithSqlState;
147139

@@ -168,22 +160,16 @@ export function mapKernelErrorToJsError(kErr: KernelErrorShape): ErrorWithSqlSta
168160
break;
169161

170162
case 'SqlError': {
171-
if (queryId === '') {
172-
// SEA uses an empty statement id when ExecuteStatement rejects the
173-
// request before creating an operation. Thrift surfaces that response
174-
// through Status.assert(), so use the same StatusError here.
175-
error = new StatusError({
176-
message,
177-
sqlState: sqlstate,
178-
});
179-
} else {
180-
// A non-empty statement id identifies a real operation that reached a
181-
// failed terminal state. Missing ids retain this conservative default
182-
// for compatibility with older kernels that did not attach queryId.
183-
const stateError = new OperationStateError(OperationStateErrorCode.Error);
184-
stateError.message = message;
185-
error = stateError;
186-
}
163+
// A server-reported SQL execution failure (kernel `SqlError`, e.g. a
164+
// bad query, missing table, divide-by-zero, invalid cast). The Thrift
165+
// backend surfaces the same situation as `OperationStateError(Error)`
166+
// when the operation reaches ERROR_STATE (see DBSQLOperation), so map
167+
// SqlError to the same class for backend parity. OperationStateError
168+
// extends HiveDriverError, so existing `instanceof HiveDriverError`
169+
// catches are unaffected.
170+
const stateError = new OperationStateError(OperationStateErrorCode.Error);
171+
stateError.message = message;
172+
error = stateError;
187173
break;
188174
}
189175

@@ -318,14 +304,10 @@ export function decodeNapiKernelError(err: unknown): Error {
318304
const code = envelope.code as string;
319305
const msg = envelope.message as string;
320306
const sqlState = typeof envelope.sqlState === 'string' ? envelope.sqlState : undefined;
321-
const queryId = typeof envelope.queryId === 'string' ? envelope.queryId : undefined;
322307

323-
const jsErr = mapKernelErrorToJsError({ code, message: msg, sqlstate: sqlState, queryId });
308+
const jsErr = mapKernelErrorToJsError({ code, message: msg, sqlstate: sqlState });
324309

325310
const meta = buildKernelMetadata(envelope);
326-
if (jsErr instanceof StatusError && meta.vendorCode !== undefined) {
327-
jsErr.code = meta.vendorCode;
328-
}
329311
// Skip the namespace attachment entirely when no fields validated
330312
// through — keeps `err.kernelMetadata` absent rather than `{}` for
331313
// simple envelopes (the common case). Key-count check so new

‎tests/e2e/kernel/execution-e2e.test.ts‎

Lines changed: 11 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import { expect } from 'chai';
1616
import { DBSQLClient, DBSQLParameter, DBSQLParameterType } from '../../../lib';
1717
import { ConnectionOptions } from '../../../lib/contracts/IDBSQLClient';
1818
import { InternalConnectionOptions } from '../../../lib/contracts/InternalConnectionOptions';
19-
import StatusError from '../../../lib/errors/StatusError';
2019

2120
/**
2221
* kernel-execution end-to-end test.
@@ -172,7 +171,7 @@ describe('kernel execution end-to-end', function e2eSuite() {
172171
}
173172
});
174173

175-
it('binds qualified INTERVAL MONTH and INTERVAL DAY values', async () => {
174+
it('preserves INTERVAL MONTH on the SEA wire', async () => {
176175
const client = new DBSQLClient();
177176

178177
await client.connect({
@@ -183,81 +182,27 @@ describe('kernel execution end-to-end', function e2eSuite() {
183182
} as ConnectionOptions & InternalConnectionOptions);
184183

185184
const session = await client.openSession({ initialCatalog: 'main' });
186-
let monthOperation;
187-
let dayOperation;
185+
let operation;
186+
let caught: unknown;
188187
try {
189-
monthOperation = await session.executeStatement("SELECT ? = INTERVAL '13' MONTH AS matches", {
188+
operation = await session.executeStatement('SELECT ?', {
190189
ordinalParameters: [
191190
new DBSQLParameter({
192191
type: DBSQLParameterType.INTERVALMONTH,
193-
value: '13',
194-
}),
195-
],
196-
});
197-
expect(await monthOperation.fetchAll()).to.deep.equal([{ matches: true }]);
198-
await monthOperation.close();
199-
monthOperation = undefined;
200-
201-
dayOperation = await session.executeStatement("SELECT ? = INTERVAL '3' DAY AS matches", {
202-
ordinalParameters: [
203-
new DBSQLParameter({
204-
type: DBSQLParameterType.INTERVALDAY,
205-
value: '3',
192+
value: '2-6',
206193
}),
207194
],
208195
});
209-
expect(await dayOperation.fetchAll()).to.deep.equal([{ matches: true }]);
196+
await operation.fetchAll();
197+
} catch (error) {
198+
caught = error;
210199
} finally {
211-
await monthOperation?.close();
212-
await dayOperation?.close();
200+
await operation?.close();
213201
await session.close();
214202
await client.close();
215203
}
216-
});
217-
218-
it('maps malformed qualified INTERVAL request failures to StatusError', async () => {
219-
const client = new DBSQLClient();
220-
221-
await client.connect({
222-
host: hostName as string,
223-
path: httpPath as string,
224-
token: token as string,
225-
useKernel: true,
226-
} as ConnectionOptions & InternalConnectionOptions);
227-
228-
const session = await client.openSession({ initialCatalog: 'main' });
229-
const expectCastFailure = async (
230-
parameterType: DBSQLParameterType,
231-
value: string,
232-
targetType: string,
233-
expectedWireType: string,
234-
) => {
235-
let operation;
236-
let caught: unknown;
237-
try {
238-
operation = await session.executeStatement(`SELECT CAST(? AS ${targetType}) AS value`, {
239-
ordinalParameters: [new DBSQLParameter({ type: parameterType, value })],
240-
});
241-
await operation.fetchAll();
242-
} catch (error) {
243-
caught = error;
244-
} finally {
245-
await operation?.close();
246-
}
247204

248-
expect(caught).to.be.instanceOf(StatusError);
249-
expect((caught as StatusError).sqlState).to.equal('22023');
250-
expect((caught as StatusError).message).to.include(expectedWireType);
251-
};
252-
253-
try {
254-
// Each value is valid for the wider CAST target but malformed for the
255-
// qualified parameter type sent on the wire.
256-
await expectCastFailure(DBSQLParameterType.INTERVALMONTH, '2-6', 'INTERVAL YEAR TO MONTH', 'INTERVAL MONTH');
257-
await expectCastFailure(DBSQLParameterType.INTERVALDAY, '3 04:05:06', 'INTERVAL DAY TO SECOND', 'INTERVAL DAY');
258-
} finally {
259-
await session.close();
260-
await client.close();
261-
}
205+
// "2-6" is valid YEAR TO MONTH syntax, but invalid for INTERVAL MONTH.
206+
expect(caught).to.be.instanceOf(Error);
262207
});
263208
});

‎tests/unit/DBSQLSession.test.ts‎

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@ import DBSQLSession, { numberToInt64 } from '../../lib/DBSQLSession';
55
import InfoValue from '../../lib/dto/InfoValue';
66
import Status from '../../lib/dto/Status';
77
import DBSQLOperation from '../../lib/DBSQLOperation';
8-
import StatusError from '../../lib/errors/StatusError';
9-
import OperationStateError from '../../lib/errors/OperationStateError';
10-
import { TSessionHandle, TProtocolVersion, TStatusCode } from '../../thrift/TCLIService_types';
8+
import { TSessionHandle, TProtocolVersion } from '../../thrift/TCLIService_types';
119
import ClientContextStub from './.stubs/ClientContextStub';
1210
import { createSessionForTest } from './.stubs/createSessionForTest';
1311

@@ -78,32 +76,6 @@ describe('DBSQLSession', () => {
7876
expect(result).instanceOf(DBSQLOperation);
7977
});
8078

81-
it('should surface an immediate execution failure as a StatusError with SQLSTATE', async () => {
82-
const context = new ClientContextStub();
83-
context.driver.executeStatementResp = {
84-
status: {
85-
statusCode: TStatusCode.ERROR_STATUS,
86-
sqlState: '22023',
87-
errorCode: 123,
88-
errorMessage: 'Invalid interval value',
89-
},
90-
};
91-
const session = createSessionForTest({ handle: sessionHandleStub, context });
92-
93-
let caught: unknown;
94-
try {
95-
await session.executeStatement('SELECT CAST(? AS INTERVAL YEAR TO MONTH)');
96-
} catch (error) {
97-
caught = error;
98-
}
99-
100-
expect(caught).to.be.instanceOf(StatusError);
101-
expect(caught).to.be.instanceOf(Error);
102-
expect(caught).to.not.be.instanceOf(OperationStateError);
103-
expect((caught as StatusError).code).to.equal(123);
104-
expect((caught as StatusError).sqlState).to.equal('22023');
105-
});
106-
10779
describe('Arrow support', () => {
10880
it('should not use Arrow if disabled in options', async () => {
10981
const session = createSessionForTest({

‎tests/unit/dto/Status.test.ts‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import { expect } from 'chai';
22
import { TStatusCode } from '../../../thrift/TCLIService_types';
33
import Status from '../../../lib/dto/Status';
4-
import StatusError from '../../../lib/errors/StatusError';
5-
import HiveDriverError from '../../../lib/errors/HiveDriverError';
64

75
describe('StatusFactory', () => {
86
it('should be success', () => {
@@ -74,28 +72,17 @@ describe('StatusFactory', () => {
7472

7573
describe('assert', () => {
7674
it('should throw exception on error status', () => {
77-
let caught: unknown;
78-
try {
75+
const error = expect(() => {
7976
Status.assert({
8077
statusCode: TStatusCode.ERROR_STATUS,
8178
errorMessage: 'error',
82-
errorCode: 0,
83-
sqlState: '22023',
79+
errorCode: 1,
8480
infoMessages: ['line1', 'line2'],
8581
});
86-
} catch (error) {
87-
caught = error;
88-
}
89-
90-
expect(caught).to.be.instanceOf(StatusError);
91-
expect(caught).to.be.instanceOf(HiveDriverError);
92-
expect(caught).to.be.instanceOf(Error);
93-
const statusError = caught as StatusError;
94-
expect(statusError.message).to.equal('error');
95-
expect(statusError.stack).to.equal('line1\nline2');
96-
expect(statusError.code).to.equal(0);
97-
expect(statusError.sqlState).to.equal('22023');
98-
expect(statusError.name).to.equal('Status Error');
82+
}).to.throw('error');
83+
error.with.property('stack', 'line1\nline2');
84+
error.with.property('code', 1);
85+
error.with.property('name', 'Status Error');
9986
});
10087

10188
it('should throw exception on invalid handle status', () => {

0 commit comments

Comments
 (0)