diff --git a/lib/errors/OperationStateError.ts b/lib/errors/OperationStateError.ts index 794464d7..46f5024a 100644 --- a/lib/errors/OperationStateError.ts +++ b/lib/errors/OperationStateError.ts @@ -22,10 +22,28 @@ export default class OperationStateError extends HiveDriverError { public response?: TGetOperationStatusResp; + // SQLSTATE reported by the server for the failed operation (if any) + public sqlState?: string; + constructor(errorCode: OperationStateErrorCode, response?: TGetOperationStatusResp) { super(response?.displayMessage ?? errorMessages[errorCode]); this.errorCode = errorCode; this.response = response; + + // Only the top-level field describes the failed operation: `status` is + // always a successful GetOperationStatus RPC status by the time this is thrown. + const sqlState = response?.sqlState; + if (sqlState) { + // Non-enumerable, matching how the kernel backend attaches `sqlState` + // (see `defineErrorMetadata` in lib/kernel/KernelErrorMapping.ts), so the + // same error class looks identical regardless of backend. + Object.defineProperty(this, 'sqlState', { + value: sqlState, + writable: true, + enumerable: false, + configurable: true, + }); + } } } diff --git a/tests/e2e/operation_error_codes.test.ts b/tests/e2e/operation_error_codes.test.ts new file mode 100644 index 00000000..d1a2eb72 --- /dev/null +++ b/tests/e2e/operation_error_codes.test.ts @@ -0,0 +1,59 @@ +import { expect, AssertionError } from 'chai'; +import { DBSQLClient } from '../../lib'; +import IDBSQLSession from '../../lib/contracts/IDBSQLSession'; +import OperationStateError, { OperationStateErrorCode } from '../../lib/errors/OperationStateError'; + +import config from './utils/config'; + +// Regression for https://github.com/databricks/databricks-sql-nodejs/issues/33 — +// errors thrown for failed operations should carry a machine-readable code +// (SQLSTATE) at the top level, not only a message buried in `response`. +describe('Operation error codes', () => { + let client: DBSQLClient; + let session: IDBSQLSession; + + before(async () => { + client = new DBSQLClient(); + const connection = await client.connect({ + host: config.host, + path: config.path, + token: config.token, + }); + session = await connection.openSession({ + initialCatalog: config.catalog, + initialSchema: config.schema, + }); + }); + + after(async () => { + await session?.close(); + await client?.close(); + }); + + async function expectOperationStateError(query: string) { + try { + const operation = await session.executeStatement(query); + await operation.fetchAll(); + expect.fail('It should throw an error'); + } catch (error) { + if (error instanceof AssertionError) { + throw error; + } + expect(error).to.be.instanceOf(OperationStateError); + return error as OperationStateError & { sqlState?: string }; + } + throw new Error('unreachable'); + } + + it('should expose server SQLSTATE on OperationStateError for runtime failures', async () => { + const error = await expectOperationStateError("SELECT raise_error('boom')"); + + expect(error.errorCode).to.be.eq(OperationStateErrorCode.Error); + + const wireSqlState = error.response?.sqlState; + expect(wireSqlState, 'server should send a SQLSTATE on the wire').to.be.a('string'); + expect(wireSqlState).to.have.length(5); + + expect(error.sqlState).to.be.eq(wireSqlState); + }); +}); diff --git a/tests/unit/errors/OperationStateError.test.ts b/tests/unit/errors/OperationStateError.test.ts new file mode 100644 index 00000000..ac38c371 --- /dev/null +++ b/tests/unit/errors/OperationStateError.test.ts @@ -0,0 +1,53 @@ +import { expect } from 'chai'; +import OperationStateError, { OperationStateErrorCode } from '../../../lib/errors/OperationStateError'; +import { mapKernelErrorToJsError } from '../../../lib/kernel/KernelErrorMapping'; +import { TOperationState, TStatusCode } from '../../../thrift/TCLIService_types'; + +describe('OperationStateError', () => { + it('should expose response.sqlState at top level', () => { + const error = new OperationStateError(OperationStateErrorCode.Error, { + status: { statusCode: TStatusCode.SUCCESS_STATUS, sqlState: '00000' }, + operationState: TOperationState.ERROR_STATE, + sqlState: '22012', + displayMessage: 'Division by zero', + }) as OperationStateError & { sqlState?: string }; + + expect(error.errorCode).to.be.eq(OperationStateErrorCode.Error); + expect(error.message).to.be.eq('Division by zero'); + expect(error.sqlState).to.be.eq('22012'); + }); + + it('should not take sqlState from the RPC status', () => { + const error = new OperationStateError(OperationStateErrorCode.Error, { + status: { statusCode: TStatusCode.SUCCESS_STATUS, sqlState: '00000' }, + operationState: TOperationState.ERROR_STATE, + }); + + expect(Object.prototype.hasOwnProperty.call(error, 'sqlState')).to.be.false; + }); + + it('should attach sqlState with the same descriptor as the kernel backend', () => { + const thriftError = new OperationStateError(OperationStateErrorCode.Error, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + operationState: TOperationState.ERROR_STATE, + sqlState: '22012', + }); + const kernelError = mapKernelErrorToJsError({ code: 'SqlError', message: 'x', sqlstate: '22012' }); + + expect(kernelError).to.be.instanceOf(OperationStateError); + expect(Object.getOwnPropertyDescriptor(thriftError, 'sqlState')).to.deep.eq( + Object.getOwnPropertyDescriptor(kernelError, 'sqlState'), + ); + expect(Object.keys(thriftError)).to.not.include('sqlState'); + expect(JSON.parse(JSON.stringify(thriftError))).to.not.have.property('sqlState'); + }); + + it('should not define sqlState when the server reports none', () => { + const error = new OperationStateError(OperationStateErrorCode.Error, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + operationState: TOperationState.ERROR_STATE, + }); + + expect(Object.prototype.hasOwnProperty.call(error, 'sqlState')).to.be.false; + }); +});