[SEA-NodeJS] (3/8) Kernel ErrorCode → JS error-class mapping (M0 minimum) by msrathore-db · Pull Request #377 · databricks/databricks-sql-nodejs

msrathore-db · 2026-05-16T12:16:02Z

Stack

Linear stack of 8 PRs landing the M0 + M1 Phase 1 SEA NodeJS work. Merge in order from base ↑ to tip. The tip branch (msrathore/sea-auth-u2m, PR #383) is the single snapshot containing everything in flight — point your test or benchmark harness at it for an end-to-end check.

Pos	PR	Branch	Scope
1/8	#378	`sea-abstraction`	IBackend / ISessionBackend / IOperationBackend interfaces
2/8	#380	`sea-napi-binding`	TS loader + build script for the kernel-provided `.node` artifact
3/8	#377	`sea-errors-logging`	Kernel `ErrorCode` → JS error-class mapping (M0 minimum)
4/8	#379	`sea-auth`	PAT auth via `useSEA: true`
5/8	#382	`sea-execution`	`executeStatement` + `openSession` (sessionConfig, initialCatalog/Schema)
6/8	#381	`sea-results`	CloudFetch + Inline Arrow result fetching
7/8	#384	`sea-operation`	cancel / close / finished lifecycle + INTERVAL parity + napi-relocation acceptance (absorbed sea-integration content)
8/8	#383	`sea-auth-u2m` ← TIP	M1 Phase 1 OAuth M2M + U2M (5 review rounds, ZERO HIGH at close)

Companion kernel stack (databricks/databricks-sql-kernel): 8 PRs — root #26 (async-public-api) → #27 → #25 → #29 → #28 → #30 → #24 → #23 (tip).

Policy: new PRs always stack on the current tip. No sibling/parallel topology. No force-pushes on existing PRs unless absolutely necessary; if a PR's content is wrong, add a fix-up commit on top of the stack tip rather than rewriting history.

This PR is position 3/8.

Summary

SeaErrorMapping.ts decodes the kernel's __databricks_error__:<json> napi error envelope into typed JS errors (AuthenticationError, NetworkError, OperationStateError, etc.). M0-minimum mapping covers 13 kernel ErrorCode variants.

The decoder gets significantly hardened later in #383 (M1 OAuth) — namespace under error.kernelMetadata.*, sentinel-strip on corrupted envelopes, type-guards — but the M0 baseline is here.

Draft until you give the go for review.

Downstream fixes / reviewer note

This PR is the M0 baseline mapping. The OAuth-era hardening called out above lands in [SEA-NodeJS] (8/8) M1 Phase 1 — OAuth M2M + OAuth U2M (5 review rounds, ZERO HIGH at close) #383, and SQL execution failures are reclassified to Thrift-compatible OperationStateError(ERROR) downstream in feat(sea): Thrift-parity — params, intervals, getInfo, SQL-error class, input validation + queryTags #403.
If reviewing the lower stack and noticing missing SQL-error parity or envelope hardening, those fixes are already stacked later.

Tracking

PECOBLR-2878 — Error class hierarchy expansion
Parent epic: PECOBLR-2666

Co-authored-by: Isaac

…kend Refactors DBSQLClient/Session/Operation to dispatch through three backend interfaces. ThriftBackend (lib/thrift-backend/) contains the relocated existing thrift logic. SeaBackend (lib/sea/) is a stub for M0; the sea-napi-binding feature wires the real impl. Public surface (lib/index.ts) unchanged. No new dependencies. All existing tests pass. Files: - lib/contracts/IBackend.ts (new) - lib/contracts/ISessionBackend.ts (new) - lib/contracts/IOperationBackend.ts (new) - lib/contracts/IDBSQLClient.ts (adds useSEA?: boolean to ConnectionOptions) - lib/thrift-backend/ThriftBackend.ts (new) - lib/thrift-backend/ThriftSessionBackend.ts (new) - lib/thrift-backend/ThriftOperationBackend.ts (new) - lib/sea/SeaBackend.ts (new, M0 stub) - lib/DBSQLClient.ts (dispatch through IBackend; useSEA picks SeaBackend) - lib/DBSQLSession.ts (facade over ISessionBackend; staging stays here) - lib/DBSQLOperation.ts (facade over IOperationBackend; iterators/fetchAll stay here) - tests/unit/DBSQLClient.test.ts (retarget internal state lookup through backend; pre-seed client.backend in tests that bypass connect()) - tests/unit/DBSQLOperation.test.ts (retarget internal state lookup through backend)

…nline type Addresses code-bloat-watchdog findings from commit 0085928: - Restores public-API JSDoc on DBSQLSession + DBSQLOperation methods (was deleted as scope creep; contracts unchanged so docs still apply) - Adds makeStubbedClient() helper to tests/unit/DBSQLClient.test.ts; replaces 14× duplicated ThriftBackend pre-seed - Imports WaitUntilReadyOptions instead of inline option types in IOperationBackend + DBSQLOperation.waitUntilReady

github-actions · 2026-05-16T12:17:49Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

github-actions · 2026-05-17T13:23:46Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Round-N fixes from the 9-reviewer pre-review. Public IOperation/DBSQLOperation surface preserved byte-identical; backend interfaces (IBackend / ISessionBackend / IOperationBackend) made fully neutral so both Thrift and SEA can implement the same contract. F1 — neutral DTOs at IOperationBackend with Thrift-shape preservation on the public facade (adapter pattern): - lib/contracts/OperationStatus.ts (new) — neutral OperationStatus + OperationState enum mirroring databricks-sql-python's CommandState and kernel pyo3's StatementStatus taxonomy. - lib/contracts/ResultMetadata.ts (new) — neutral ResultMetadata + ResultFormat enum mirroring the three TSparkRowSetType cases. - IOperationBackend.status()/getResultMetadata() return the neutral DTOs. - ThriftOperationBackend.status() adapts at the boundary via adaptOperationStatus / adaptResultMetadata; module-level helpers thriftStateToOperationState and thriftRowSetTypeToResultFormat do the enum maps. - ThriftOperationBackend exposes thriftStatusResponse() and thriftResultMetadataResponse() as public Thrift-only accessors used by the facade's zero-loss fast path (kept for internal state-machine + result-handler dispatch as well). - lib/utils/thriftWireSynthesis.ts (new) — synthesizeThriftStatus and synthesizeThriftResultSetMetadata: convert neutral DTOs back to Thrift wire shape for the non-Thrift backend path. Lossy on Thrift-only fields (taskStatus, numModifiedRows, cacheLookupResult, etc.). - DBSQLOperation.status() and getMetadata() preserved Thrift return shape: Thrift backend path returns the real wire response (zero loss); non-Thrift backend path synthesizes via the new helpers. - DBSQLOperation.getResultMetadata() — new additive neutral accessor on IOperation; DBSQLSession.handleStagingOperation uses it instead of the deprecated Thrift-shaped getMetadata(). F2 — IBackend.connect() is now zero-arg. Backend reads everything it needs from IClientContext / constructor; matches Python connector's pattern of passing session_configuration via constructor not method-arg. F3 — Restore the 'Server protocol version' debug log dropped by the original PR-378 refactor. Re-added to ThriftSessionBackend.constructor with the LogLevel.debug + IClientContext.getLogger() pattern; matches the pre-refactor log site at main:lib/DBSQLSession.ts:175. F4 + F11 + F14 — SeaBackend stub safety: - close() is a no-op so DBSQLClient.close()'s state-clearing block can finish even after a useSEA: true connect() failure. - connect() and openSession() throw HiveDriverError instead of generic Error, matching the rest of the codebase. - connect(options: ConnectionOptions) and openSession(request: OpenSessionRequest) declare their parameters (with @typescript-eslint/no-unused-vars disable) so IDE autocomplete prompts the M1 SEA implementer. F6 + F7 + F9 + F10 — JSDoc on backend interfaces: - IBackend: connect/openSession/close docstrings; close() doc explicitly states transport-layer cleanup is owned by DBSQLClient. - ISessionBackend: copy IDBSQLSession's per-method one-liner JSDoc. - IOperationBackend: doc hasResultSet (readonly external; mutates internally), waitUntilReady (MUST throw OperationStateError on terminal non-success). F8 — tests/unit/sea/SeaBackend.test.ts (new) locks in the stub contract: connect() rejects HiveDriverError, openSession() rejects HiveDriverError, close() resolves no-op. ~30 LOC. F12 — Drop legacy { handle, ... } ctor branch from DBSQLOperation and DBSQLSession: - Facades accept only { backend, context }. - DBSQLSession no longer imports ThriftSessionBackend at all. - DBSQLOperation imports ThriftOperationBackend solely for the F1 typed downcast (zero-loss Thrift fast path); this is a deliberate, scoped coupling tied to the back-compat decision. - tests/unit/.stubs/createSessionForTest.ts and createOperationForTest.ts (new) wrap the legacy shape; all 48 + 54 test sites mechanically migrated. F15 — ThriftOperationBackend.waitUntilReady uses imported WaitUntilReadyOptions type instead of an anonymous inline shape. F16 — useSEA flag moved out of public ConnectionOptions: - Removed useSEA?: boolean from the exported lib/contracts/IDBSQLClient.ts ConnectionOptions; no longer ships in the public .d.ts. - lib/contracts/InternalConnectionOptions.ts (new) declares the flag as a non-exported internal extension; DBSQLClient.connect() reads via a typed cast. Mirrors Python's kwargs.get('use_sea', False) pattern at databricks-sql-python/src/databricks/sql/session.py:111. F17 — Missing return; after case 'timeout' in forwardConnectionEvent so a future fifth case doesn't silently fall through. The trailing return; in the last case triggers no-useless-return — quieted with a localized eslint-disable-next-line + intent comment. F5 — deferred per owner instruction (test-only as any cast tightening). Verification: - yarn lint clean (3 pre-existing warnings in tests/e2e/protocol_versions.test.ts). - yarn build clean. - tsc --noEmit -p tsconfig.json clean (apart from pre-existing examples/tokenFederation/* import errors that exist on main). - Runtime smoke test of SeaBackend stub + Thrift-wire synthesis round-trip passes 5/5 assertions. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <[email protected]>

github-actions · 2026-05-24T21:58:44Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Creates the napi-rs binding skeleton: Cargo.toml + lib.rs + module stubs for database/connection/statement/result/error/logger. Captures napi-rs tokio Handle via OnceCell in runtime.rs. Single working #[napi] fn version() proves the binding loads + executes end-to-end in Node. Depends on krn-async-public-api branch (path dep on kernel). Round 2 will add open/execute/fetch methods. Signed-off-by: Madhavendra Rathore <[email protected]>

…wired Adds real async methods on the opaque wrappers backing M0: - openSession (free function) with PAT → kernel Session - Connection::execute_statement → kernel ExecutedStatement - Statement::fetch_next_batch / schema / cancel / close → kernel ResultStream - Arrow batches returned as IPC bytes (per Layer 2 design) - Error mapping preserves kernel ErrorCode + SQLSTATE for TS layer - All entry points wrapped in catch_unwind End-to-end smoke test against pecotesting passes. No new dependencies beyond arrow-{ipc,array,schema} + futures. Uses kernel async public API (no block_on). Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <[email protected]>

…indings Round 1 scaffold declared tracing + tracing-subscriber as deps but never used them. Removed. Logger bridge will re-add in round 3. Other findings from 6b3affd-2026-05-15.md reviewed: - Finding 2 (Database::Drop unreachable in Round 1b) — obsoleted by Round 2 (40d0b57): database.rs no longer declares a Database struct or Drop impl; it is now an `open_session` free function. - Finding 3 (empty Connection::Drop) — obsoleted by Round 2: the Drop impl now spawns a real fire-and-forget close on the captured tokio handle. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <[email protected]>

Per D-006 architectural decision (Python team's workspace pattern): all language bindings (PyO3, napi-rs) now live as workspace siblings in the kernel repo at databricks-sql-kernel/{pyo3,napi}/. What this commit removes from the nodejs repo: - native/sea/Cargo.toml (path dep relocated; package now at databricks-sql-kernel/napi/Cargo.toml with path = "..") - native/sea/build.rs - native/sea/src/* (lib, runtime, database, connection, statement, result, error, logger, util — all 9 files) - native/sea/package.json (the @databricks/sea-native-linux-x64-gnu sub-package moves to the kernel workspace too) - native/sea/index.js (regenerated artifact) What stays in nodejs: - native/sea/index.d.ts — TS declarations consumed by lib/sea/ adapter - native/sea/README.md (new) — explains the move; points readers at databricks-sql-kernel/napi/ What's updated: - package.json: `build:native` and `build:native:debug` scripts now delegate to the kernel workspace via $DATABRICKS_SQL_KERNEL_REPO (defaults to ../../databricks-sql-kernel-sea-WT/napi-binding for the local dev worktree layout). Build copies index.node + index.d.ts back into native/sea/ for the loader to find. Why workspace co-location: - Arrow version pinning lockstep — no silent IPC version drift - path = ".." (clean) vs ../../../../databricks-sql-kernel-sea-WT/... - Single CI: cargo build --workspace covers kernel + pyo3 + napi - Kernel API changes that break either binding caught at PR-review time - Future cgo binding for Go SEA slots in as another workspace member This branch (sea-napi-binding) is now a thin consumer of the kernel napi crate. The actual Rust code lives at krn-napi-binding HEAD on the kernel repo (commit debe3d7). Signed-off-by: Madhavendra Rathore <[email protected]>

…generated Signed-off-by: Madhavendra Rathore <[email protected]>

Addresses PR #380 review findings C1, C2 (partial), C3, H1, H2, H3, H4, H5, H6, H7, H8 (runtime guard), H9, M1, M2, M3, M4 (linguist), M5, M6, M7, M8, L1, L2, L4. - C1 lint: drop `.js` ext from native/sea require so eslint import/extensions passes; gitignore the auto-generated index.js and *.node artifacts; prettier-ignore the napi-rs auto-generated index.d.ts / index.js. - C2 publish: whitelist native/sea/index.{js,d.ts} in .npmignore; declare @databricks/sea-native-linux-x64-gnu in optionalDependencies. The kernel-side package-name rename (drop the linux-x64-gnu prefix) is tracked separately as a cross-PR ask. - C3 test wiring: move tests/native/version.test.ts -> tests/unit/sea/, tests/native/e2e-smoke.test.ts -> tests/e2e/sea/; both now picked up by the existing mocharc globs and run via the existing `npm test` / `npm run e2e` jobs. - H1 noise drop: version.test.ts now asserts one meaningful semver check; three tautological assertions removed. - H2 + H3 + H7 lazy load: rewrite SeaNativeLoader.ts on the lib/utils/lz4.ts pattern. Lazy require behind getSeaNative(); capability-detection helper tryGetSeaNative(); structured error messages that classify MODULE_NOT_FOUND vs ERR_DLOPEN_FAILED and include platform/arch/Node-version + install hint. - H4 supply chain: pin @napi-rs/cli to 2.18.4 in devDependencies; build:native switches to `npx --no-install` so the pinned local install is used (no per-build network fetch). - H5 path: switch build:native default kernel path to the canonical `../../databricks-sql-kernel/napi-binding` (the worktree-specific `-sea-WT` suffix is gone). - H6 CI safety: e2e suite hard-fails when CI=true and any required env var is missing; dev machines still skip. - H8 runtime guard: loader throws a structured error on Node <18 instead of attempting a dlopen that would fail mysteriously. Driver's engines.node stays >=14 — Thrift consumers on older Node continue to work. - H9 + M1 + M2 types: tsconfig adds a `@sea-native` path alias to native/sea/index.d.ts; loader imports the real Connection / Statement / ConnectionOptions / ExecuteOptions / ArrowBatch / ArrowSchema types and re-exports them. Tests use the re-exported types — the inline-shape duplication across three files collapses. - M3 README: rewrite native/sea/README.md to match kernel reality. The napi crate is a standalone Cargo workspace (not a pyo3 sibling); explain the tls-rustls choice that the standalone workspace exists to enable. - M4 drift detection: mark native/sea/index.d.ts as linguist-generated in .gitattributes so GitHub collapses it in diffs and excludes from blame/language stats. - M5 artifacts: gitignore native/sea/index.js, index.node, index.*.node. - M6 + M7 e2e coverage: decode the IPC payload via apache-arrow's tableFromIPC and assert numRows + cell value (not just shape); add drain-past-null idempotence and schema-before-fetch coverage. - L1 build scripts: collapse build:native + build:native:debug into one script taking BUILD_PROFILE (defaults to --release). - L2 / L4 / M8: drop the version() alias and the "Round 2+ will…" comment debt; the loader now actually delivers the value the prior JSDoc was only promising. Verified on this branch: npm run lint clean (0 errors); npm run prettier clean for PR-owned files (3 unrelated pre-existing warnings on PR #378 territory); tsc --project tsconfig.build.json clean; mocha on tests/unit/sea passes (4/4); mocha on tests/e2e/sea passes against a live pecotesting warehouse (2/2, IPC decode confirms SELECT 1 returns 1). Signed-off-by: Madhavendra Rathore <[email protected]>

@deprecated

… IOperationBackend (#378) * sea-abstraction: introduce IBackend / ISessionBackend / IOperationBackend Refactors DBSQLClient/Session/Operation to dispatch through three backend interfaces. ThriftBackend (lib/thrift-backend/) contains the relocated existing thrift logic. SeaBackend (lib/sea/) is a stub for M0; the sea-napi-binding feature wires the real impl. Public surface (lib/index.ts) unchanged. No new dependencies. All existing tests pass. Files: - lib/contracts/IBackend.ts (new) - lib/contracts/ISessionBackend.ts (new) - lib/contracts/IOperationBackend.ts (new) - lib/contracts/IDBSQLClient.ts (adds useSEA?: boolean to ConnectionOptions) - lib/thrift-backend/ThriftBackend.ts (new) - lib/thrift-backend/ThriftSessionBackend.ts (new) - lib/thrift-backend/ThriftOperationBackend.ts (new) - lib/sea/SeaBackend.ts (new, M0 stub) - lib/DBSQLClient.ts (dispatch through IBackend; useSEA picks SeaBackend) - lib/DBSQLSession.ts (facade over ISessionBackend; staging stays here) - lib/DBSQLOperation.ts (facade over IOperationBackend; iterators/fetchAll stay here) - tests/unit/DBSQLClient.test.ts (retarget internal state lookup through backend; pre-seed client.backend in tests that bypass connect()) - tests/unit/DBSQLOperation.test.ts (retarget internal state lookup through backend) * sea-abstraction: cleanup — restore JSDoc, dedupe test pre-seed, fix inline type Addresses code-bloat-watchdog findings from commit 0085928: - Restores public-API JSDoc on DBSQLSession + DBSQLOperation methods (was deleted as scope creep; contracts unchanged so docs still apply) - Adds makeStubbedClient() helper to tests/unit/DBSQLClient.test.ts; replaces 14× duplicated ThriftBackend pre-seed - Imports WaitUntilReadyOptions instead of inline option types in IOperationBackend + DBSQLOperation.waitUntilReady * sea-abstraction: address full-review findings (F1-F17 except F5) Round-N fixes from the 9-reviewer pre-review. Public IOperation/DBSQLOperation surface preserved byte-identical; backend interfaces (IBackend / ISessionBackend / IOperationBackend) made fully neutral so both Thrift and SEA can implement the same contract. F1 — neutral DTOs at IOperationBackend with Thrift-shape preservation on the public facade (adapter pattern): - lib/contracts/OperationStatus.ts (new) — neutral OperationStatus + OperationState enum mirroring databricks-sql-python's CommandState and kernel pyo3's StatementStatus taxonomy. - lib/contracts/ResultMetadata.ts (new) — neutral ResultMetadata + ResultFormat enum mirroring the three TSparkRowSetType cases. - IOperationBackend.status()/getResultMetadata() return the neutral DTOs. - ThriftOperationBackend.status() adapts at the boundary via adaptOperationStatus / adaptResultMetadata; module-level helpers thriftStateToOperationState and thriftRowSetTypeToResultFormat do the enum maps. - ThriftOperationBackend exposes thriftStatusResponse() and thriftResultMetadataResponse() as public Thrift-only accessors used by the facade's zero-loss fast path (kept for internal state-machine + result-handler dispatch as well). - lib/utils/thriftWireSynthesis.ts (new) — synthesizeThriftStatus and synthesizeThriftResultSetMetadata: convert neutral DTOs back to Thrift wire shape for the non-Thrift backend path. Lossy on Thrift-only fields (taskStatus, numModifiedRows, cacheLookupResult, etc.). - DBSQLOperation.status() and getMetadata() preserved Thrift return shape: Thrift backend path returns the real wire response (zero loss); non-Thrift backend path synthesizes via the new helpers. - DBSQLOperation.getResultMetadata() — new additive neutral accessor on IOperation; DBSQLSession.handleStagingOperation uses it instead of the deprecated Thrift-shaped getMetadata(). F2 — IBackend.connect() is now zero-arg. Backend reads everything it needs from IClientContext / constructor; matches Python connector's pattern of passing session_configuration via constructor not method-arg. F3 — Restore the 'Server protocol version' debug log dropped by the original PR-378 refactor. Re-added to ThriftSessionBackend.constructor with the LogLevel.debug + IClientContext.getLogger() pattern; matches the pre-refactor log site at main:lib/DBSQLSession.ts:175. F4 + F11 + F14 — SeaBackend stub safety: - close() is a no-op so DBSQLClient.close()'s state-clearing block can finish even after a useSEA: true connect() failure. - connect() and openSession() throw HiveDriverError instead of generic Error, matching the rest of the codebase. - connect(options: ConnectionOptions) and openSession(request: OpenSessionRequest) declare their parameters (with @typescript-eslint/no-unused-vars disable) so IDE autocomplete prompts the M1 SEA implementer. F6 + F7 + F9 + F10 — JSDoc on backend interfaces: - IBackend: connect/openSession/close docstrings; close() doc explicitly states transport-layer cleanup is owned by DBSQLClient. - ISessionBackend: copy IDBSQLSession's per-method one-liner JSDoc. - IOperationBackend: doc hasResultSet (readonly external; mutates internally), waitUntilReady (MUST throw OperationStateError on terminal non-success). F8 — tests/unit/sea/SeaBackend.test.ts (new) locks in the stub contract: connect() rejects HiveDriverError, openSession() rejects HiveDriverError, close() resolves no-op. ~30 LOC. F12 — Drop legacy { handle, ... } ctor branch from DBSQLOperation and DBSQLSession: - Facades accept only { backend, context }. - DBSQLSession no longer imports ThriftSessionBackend at all. - DBSQLOperation imports ThriftOperationBackend solely for the F1 typed downcast (zero-loss Thrift fast path); this is a deliberate, scoped coupling tied to the back-compat decision. - tests/unit/.stubs/createSessionForTest.ts and createOperationForTest.ts (new) wrap the legacy shape; all 48 + 54 test sites mechanically migrated. F15 — ThriftOperationBackend.waitUntilReady uses imported WaitUntilReadyOptions type instead of an anonymous inline shape. F16 — useSEA flag moved out of public ConnectionOptions: - Removed useSEA?: boolean from the exported lib/contracts/IDBSQLClient.ts ConnectionOptions; no longer ships in the public .d.ts. - lib/contracts/InternalConnectionOptions.ts (new) declares the flag as a non-exported internal extension; DBSQLClient.connect() reads via a typed cast. Mirrors Python's kwargs.get('use_sea', False) pattern at databricks-sql-python/src/databricks/sql/session.py:111. F17 — Missing return; after case 'timeout' in forwardConnectionEvent so a future fifth case doesn't silently fall through. The trailing return; in the last case triggers no-useless-return — quieted with a localized eslint-disable-next-line + intent comment. F5 — deferred per owner instruction (test-only as any cast tightening). Verification: - yarn lint clean (3 pre-existing warnings in tests/e2e/protocol_versions.test.ts). - yarn build clean. - tsc --noEmit -p tsconfig.json clean (apart from pre-existing examples/tokenFederation/* import errors that exist on main). - Runtime smoke test of SeaBackend stub + Thrift-wire synthesis round-trip passes 5/5 assertions. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <[email protected]> * sea-abstraction: address PR #378 review-comment fixes (H1 / M1-M4 / L1-L10) Addresses 15 review findings from the code-review-squad pass on PR #378. L11 (backend kind field on the three interfaces) is deliberately deferred to avoid a cross-stack cascade ripple while the downstream PRs are still in flight. H1 — fetchChunk lost mid-flight failIfClosed regression. Add optional `isClosed?: () => boolean` to IOperationBackend.fetchChunk's options bag. ThriftOperationBackend.fetchChunk probes it after the setTimeout(0) macrotask yield and returns [] when set; the facade's post-fetch failIfClosed then raises the user-visible OperationStateError. Restores the guard that the refactor split across the facade/backend boundary so a cancel/close arriving during the yield window no longer runs the data RPC to completion needlessly. M1 — neutralize WaitUntilReadyOptions callback shape. Introduce IOperationBackendWaitOptions { callback?: (status: OperationStatus) => unknown } on the backend interface. Facade keeps the public Thrift-typed OperationStatusCallback and adapts at the boundary by wrapping the user's callback with synthesizeThriftStatus. ThriftOperationBackend.waitUntilReady consumes the neutral options and passes adaptOperationStatus(response) to the callback. M2 — synthesizeOkStatus maps OperationState to TStatusCode. Add synthesizeStatusFromOperation that returns ERROR_STATUS for Failed/Cancelled/Closed (carrying errorMessage + sqlState) and SUCCESS_STATUS otherwise. Wire it into synthesizeThriftStatus so legacy Status.assert(resp.status) sees the right code on non-Thrift backends. M3 — TelemetryEvent + DriverConfiguration carry a backend tag. Add optional backend?: 'thrift' | 'sea' | 'kernel' on both interfaces so dashboards can slice latency/error rate by backend without a metrics-schema migration once non-Thrift emission goes live. M4 — test coverage for the synthesize helpers + useSEA failure path. New tests/unit/thrift-backend/wireSynthesis.test.ts covering all OperationState/ResultFormat mappings, ERROR_STATUS carries errorMessage/sqlState, hasResultSet round-trip, schema/arrowSchema/ lz4Compressed/isStagingOperation preservation, and the L3 throw on unknown ResultFormat. New test in DBSQLClient.test.ts asserts that a useSEA:true connect failure leaves this.backend === undefined and the next openSession() surfaces "not connected" rather than the SeaBackend's "not implemented" error. L1 — forwardConnectionEvent normalizes payload to Error. Replace `payload as Error` with `payload instanceof Error ? payload : new Error(String(payload))` so a backend that emits a non-Error through the cross-backend onConnectionEvent doesn't crash the logger.log call. L2 — DBSQLClient.connect publishes this.backend only on success. Construct the backend locally, await connect() in a try/catch, run a best-effort backend.close() (per IBackend.close()'s safe-on-partial-init contract) and rethrow on failure. Only assign this.backend after a clean connect so a failed connect surfaces "DBSQLClient: not connected" on the next openSession. L3 — resultFormatToThrift throws on unknown ResultFormat. Replace the silent default fallback to COLUMN_BASED_SET with a HiveDriverError. Prevents a future ResultFormat enum extension from silently routing results through JsonResultHandler and surfacing garbled rows. L4 — DBSQLOperation.getMetadata carries @deprecated. Adds the canonical TypeScript JSDoc tag so IDEs (strikethrough), tsc, ESLint plugins, and agentic codegen pick up the soft deprecation in favour of getResultMetadata. L5 — numberToInt64 re-export carries @deprecated. Re-export through a named const with a JSDoc block (rather than a bare `export { ... } from`) so the @deprecated tag attaches to the symbol consumers see in their IDE / .d.ts. L6 — DBSQLSession.runBackend helper. Collapse 11 duplicated `failIfClosed → backend.X → failIfClosed` brackets into a single private runBackend<T>(fn) so the open-flag-before-and-after contract has a name and can't be forgotten in a new delegation method. L7 — restore three why-comments deleted from DBSQLSession. Staging-detection invariant in executeStatement, AWS-vs-Azure 404 difference on staging-remove, and the Content-Length-required note on staging-upload. Verbatim from main; these document non-obvious intentional behaviour the refactor inadvertently dropped. L8 — hasResultSet becomes a method on IOperationBackend. The value is state-dependent (the Thrift impl mutates the underlying operation handle inside processOperationStatusResponse), so the property+readonly+disclaimer-JSDoc pattern was misleading. Method form makes the live-read semantics obvious to a fresh implementer. 3 facade call sites updated. L9 — wireSynthesis moves under thrift-backend. The file imports Thrift IDL types and produces Thrift-typed values; it belongs next to ThriftOperationBackend, not in the neutral lib/utils/ tree where it would creep into the dependency cone of future backend-neutral helpers. Same reasoning that placed numberToInt64 and getDirectResultsOptions under thrift-backend/. L10 — interface-level downcast policy. Add a JSDoc paragraph on IOperationBackend grandfathering the two existing `instanceof ThriftOperationBackend` downcasts in DBSQLOperation.status/getMetadata and prohibiting new ones. Future zero-loss back-compat needs should extend the interface (or add an optional method) rather than spawn a per-backend branch matrix. Gates: yarn build (exit 0), yarn lint (0 errors, 3 pre-existing warnings in tests/e2e/protocol_versions.test.ts), yarn test on touched files (163 passing, +12 net new tests from M4 work; 2 failures pre- existing on PR head unchanged: getSchema-directResults and the LZ4-cloud-fetch flag — both flagged in the team-lead playbook as known prior regressions). Cascade implications for downstream PRs (#380 #377 #379 #382 #381 #384 #383): L8 converts hasResultSet from a property to a method, M1 swaps WaitUntilReadyOptions for IOperationBackendWaitOptions on the backend interface. Both are mechanical renames at downstream backend impls when they rebase. Co-authored-by: Isaac Signed-off-by: Madhavendra Rathore <[email protected]> * Fix SEA abstraction merge fallout Restore Thrift compatibility paths needed by existing schema and result-handler tests after merging main telemetry changes. Signed-off-by: Madhavendra Rathore <[email protected]> * Restore Thrift result-handler compatibility hooks Keep existing e2e-only inspection hooks available through the facade while the new backend abstraction owns result handling. Signed-off-by: Madhavendra Rathore <[email protected]> --------- Signed-off-by: Madhavendra Rathore <[email protected]>

…DI seam, packaging, tests Rebuilds native/sea against the merged kernel main (binding renamed @databricks/sea-native -> @databricks/sql-kernel via kernel #82) and addresses the /full-review findings on PR #380: - C1: commit the napi-rs router (native/sea/index.js, un-ignored) + a prepack assertion so the publish tarball can never ship without it. Companion kernel fix (databricks-sql-kernel#93) corrects the base package name so the router require paths resolve. - C2: drop the @sea-native tsconfig path alias; use a relative import so no unresolvable specifier leaks into the emitted .d.ts. - C3/C11: error hints no longer name a 404-ing package; dlopen hint now includes the underlying dlerror string + concrete remediation. - C4: document M0 = linux-x64-gnu-only scope + npm scope-lock note. - C5: SeaNativeBinding = typeof import('../../native/sea') (no drift). - C6: SeaNativeLoader is now a class with an injectable load seam; getSeaNative/tryGetSeaNative are thin process-global wrappers. - C7: re-exports renamed Sea* to avoid colliding with Thrift types. - C8: version.test.ts fails loud on the linux-x64 CI runner + shape checks; new loader.test.ts covers the hint branches, Node gate, shape check, and caching via the injected seam. - C9: Node-version guard fails closed (NaN or < floor). - C13: e2e smoke uses the shared tests/e2e/utils/config.ts creds. - C10/C12 are resolved upstream in merged kernel main / deferred to M1. index.d.ts regenerated from merged main: ExecuteOptions dropped (catalog/schema/sessionConf are session-level on openSession), close() awaits DeleteSession, schema() is sync, sessionId/statementId getters added. Verified: 11 unit tests; e2e SELECT 1 against a live warehouse, direct and through mitmproxy (SEA REST: POST /sql/sessions -> POST /sql/statements -> DELETE /sql/sessions/<id>). Co-authored-by: Isaac

Single mapping function in lib/sea/SeaErrorMapping.ts converts the napi-binding's surfaced kernel error (code+message+sqlstate) to the appropriate existing JS error class. M0 minimum: PAT auth errors land as AuthenticationError; cancel/timeout as OperationStateError; network/internal as HiveDriverError. SQLSTATE preserved on the error object via .sqlState property. No new error classes. M1 may add nuance. Signed-off-by: Madhavendra Rathore <[email protected]>

msrathore-db · 2026-06-01T08:32:03Z

Superseded — this layer is folded into the consolidated SEA foundation, now split into #409 → #410 → #411 (stacked on main).

msrathore-db added 2 commits May 15, 2026 00:50

msrathore-db temporarily deployed to azure-prod May 16, 2026 12:16 — with GitHub Actions Inactive

msrathore-db force-pushed the msrathore/sea-errors-logging branch from f7cdb80 to 8ccc8e2 Compare May 17, 2026 13:22

msrathore-db had a problem deploying to azure-prod May 17, 2026 13:22 — with GitHub Actions Failure

msrathore-db changed the base branch from main to msrathore/sea-abstraction May 17, 2026 13:22

msrathore-db changed the base branch from msrathore/sea-abstraction to msrathore/sea-napi-binding May 17, 2026 13:22

msrathore-db mentioned this pull request May 17, 2026

[SEA-NodeJS] (1/8) Backend abstraction — IBackend / ISessionBackend / IOperationBackend #378

Merged

msrathore-db changed the title ~~[SEA-NodeJS] (3/9) Kernel ErrorCode → JS error-class mapping (M0 minimum)~~ [SEA-NodeJS] (3/8) Kernel ErrorCode → JS error-class mapping (M0 minimum) May 17, 2026

msrathore-db force-pushed the msrathore/sea-napi-binding branch from 20231c8 to 2f1865b Compare May 24, 2026 21:58

msrathore-db force-pushed the msrathore/sea-errors-logging branch from 8ccc8e2 to c9a8811 Compare May 24, 2026 21:58

msrathore-db added 6 commits May 24, 2026 23:23

sea-napi-binding: build:native uses --platform so index.js router is …

01f31cd

…generated Signed-off-by: Madhavendra Rathore <[email protected]>

msrathore-db force-pushed the msrathore/sea-napi-binding branch from 6b70ce4 to 548a14b Compare May 24, 2026 23:23

msrathore-db force-pushed the msrathore/sea-errors-logging branch from c9a8811 to 3dc4b3c Compare May 24, 2026 23:31

msrathore-db added 2 commits May 30, 2026 23:52

msrathore-db force-pushed the msrathore/sea-errors-logging branch from 3dc4b3c to 83015ac Compare May 31, 2026 00:02

msrathore-db mentioned this pull request May 31, 2026

feat(sea): metadata methods — catalogs/schemas/tables/columns/functions/keys/typeInfo #404

Open

msrathore-db marked this pull request as ready for review June 1, 2026 00:54

msrathore-db force-pushed the msrathore/sea-napi-binding branch 2 times, most recently from f95e3d4 to d6101aa Compare June 1, 2026 05:46

msrathore-db closed this Jun 1, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SEA-NodeJS] (3/8) Kernel ErrorCode → JS error-class mapping (M0 minimum)#377

[SEA-NodeJS] (3/8) Kernel ErrorCode → JS error-class mapping (M0 minimum)#377
msrathore-db wants to merge 11 commits into
msrathore/sea-napi-bindingfrom
msrathore/sea-errors-logging

msrathore-db commented May 16, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented May 16, 2026

Uh oh!

github-actions Bot commented May 17, 2026

Uh oh!

github-actions Bot commented May 24, 2026

Uh oh!

msrathore-db commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

msrathore-db commented May 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack

Summary

Downstream fixes / reviewer note

Tracking

Uh oh!

github-actions Bot commented May 16, 2026

Uh oh!

github-actions Bot commented May 17, 2026

Uh oh!

github-actions Bot commented May 24, 2026

Uh oh!

msrathore-db commented Jun 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

msrathore-db commented May 16, 2026 •

edited

Loading