Skip to content

Commit 19bcdaa

Browse files
committed
fix: update version to 1.0.4 in package.json and improve error handling in executeStatement
1 parent aed32d4 commit 19bcdaa

3 files changed

Lines changed: 25 additions & 33 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@bitofsky/databricks-sql",
3-
"version": "1.0.3",
3+
"version": "1.0.4",
44
"description": "Databricks SQL client for Node.js - Direct REST API without SDK",
55
"main": "dist/index.cjs",
66
"module": "dist/index.js",

src/api/executeStatement.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,8 @@ async function fetchMetrics(
2727
statementId: string,
2828
signal?: AbortSignal
2929
): Promise<QueryMetrics | undefined> {
30-
try {
31-
const queryInfo = await getQueryMetrics(auth, statementId, signal)
32-
return queryInfo.metrics
33-
} catch {
34-
// Ignore metrics fetch errors - non-critical
35-
return undefined
36-
}
30+
const queryInfo = await getQueryMetrics(auth, statementId, signal)
31+
return queryInfo.metrics
3732
}
3833

3934
/**
@@ -54,7 +49,13 @@ export async function executeStatement(
5449

5550
// Helper to call onProgress with optional metrics
5651
const emitProgress = onProgress
57-
? async (statementId: string) => onProgress(result, enableMetrics ? await fetchMetrics(auth, statementId, signal) : undefined)
52+
? async () => result ? onProgress(
53+
result,
54+
enableMetrics ? await fetchMetrics(auth, result.statement_id, signal).catch(e => {
55+
logger?.error?.(`executeStatement Failed to fetch query metrics for statement ${result?.statement_id}: ${String(e)}`, { statementId: result?.statement_id })
56+
return undefined
57+
}) : undefined
58+
) : undefined
5859
: undefined
5960

6061
// 1. Build request (filter out undefined values)
@@ -100,9 +101,9 @@ export async function executeStatement(
100101
// 3. Poll until terminal state
101102
while (!TERMINAL_STATES.has(result.status.state)) {
102103
logger?.info?.(`executeStatement Statement ${result.statement_id} in state ${result.status.state}; polling for status...`)
103-
await emitProgress?.(result.statement_id)
104104
await delay(POLL_INTERVAL_MS, signal)
105105
result = await getStatement(auth, result.statement_id, signal)
106+
await emitProgress?.()
106107
}
107108
} catch (err) {
108109
if (err instanceof AbortError || signal?.aborted) {
@@ -118,7 +119,7 @@ export async function executeStatement(
118119
}
119120

120121
// 4. Final progress callback
121-
await emitProgress?.(result.statement_id)
122+
await emitProgress?.()
122123

123124
// 5. Handle terminal states
124125
if (result.status.state === 'SUCCEEDED')

test/executeStatement.spec.ts

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ describe('executeStatement', () => {
9090
body: expect.stringContaining('"wait_timeout":"0s"'),
9191
})
9292
)
93-
expect(onProgress).toHaveBeenCalledWith(expect.objectContaining({ status: { state: 'PENDING' } }), undefined)
9493
expect(onProgress).toHaveBeenCalledWith(expect.objectContaining({ status: { state: 'SUCCEEDED' } }), undefined)
9594
})
9695

@@ -116,7 +115,7 @@ describe('executeStatement', () => {
116115
// Only 2 calls: postStatement + getStatement (no metrics calls)
117116
expect(mockFetch).toHaveBeenCalledTimes(2)
118117
// onProgress should be called without metrics
119-
expect(onProgress).toHaveBeenCalledWith(expect.objectContaining({ status: { state: 'PENDING' } }), undefined)
118+
expect(onProgress).toHaveBeenCalledWith(expect.objectContaining({ status: { state: 'SUCCEEDED' } }), undefined)
120119
})
121120

122121
it('should fetch metrics when enableMetrics is true', async () => {
@@ -127,15 +126,15 @@ describe('executeStatement', () => {
127126
ok: true,
128127
json: () => Promise.resolve(mockPendingResult),
129128
})
130-
// getQueryMetrics (during polling)
129+
// getStatement
131130
.mockResolvedValueOnce({
132131
ok: true,
133-
json: () => Promise.resolve(mockQueryInfo),
132+
json: () => Promise.resolve(mockSucceededAfterPolling),
134133
})
135-
// getStatement
134+
// getQueryMetrics (during polling)
136135
.mockResolvedValueOnce({
137136
ok: true,
138-
json: () => Promise.resolve(mockSucceededAfterPolling),
137+
json: () => Promise.resolve(mockQueryInfo),
139138
})
140139
// getQueryMetrics (final)
141140
.mockResolvedValueOnce({
@@ -153,17 +152,9 @@ describe('executeStatement', () => {
153152
await vi.advanceTimersByTimeAsync(5000)
154153
await resultPromise
155154

156-
// 4 calls: postStatement + getQueryMetrics + getStatement + getQueryMetrics
155+
// 4 calls: postStatement + getStatement + getQueryMetrics + getQueryMetrics
157156
expect(mockFetch).toHaveBeenCalledTimes(4)
158157

159-
// Check that onProgress was called with metrics
160-
expect(onProgress).toHaveBeenCalledWith(
161-
expect.objectContaining({ status: { state: 'PENDING' } }),
162-
expect.objectContaining({
163-
total_time_ms: 959,
164-
execution_time_ms: 642,
165-
})
166-
)
167158
expect(onProgress).toHaveBeenCalledWith(
168159
expect.objectContaining({ status: { state: 'SUCCEEDED' } }),
169160
expect.objectContaining({
@@ -180,18 +171,18 @@ describe('executeStatement', () => {
180171
ok: true,
181172
json: () => Promise.resolve(mockPendingResult),
182173
})
183-
// getQueryMetrics fails (401 - no retry)
174+
// getStatement
175+
.mockResolvedValueOnce({
176+
ok: true,
177+
json: () => Promise.resolve(mockSucceededAfterPolling),
178+
})
179+
// getQueryMetrics fails again (401 - no retry)
184180
.mockResolvedValueOnce({
185181
ok: false,
186182
status: 401,
187183
statusText: 'Unauthorized',
188184
text: () => Promise.resolve('Unauthorized'),
189185
})
190-
// getStatement
191-
.mockResolvedValueOnce({
192-
ok: true,
193-
json: () => Promise.resolve(mockSucceededAfterPolling),
194-
})
195186
// getQueryMetrics fails again (401 - no retry)
196187
.mockResolvedValueOnce({
197188
ok: false,
@@ -213,7 +204,7 @@ describe('executeStatement', () => {
213204
// Statement should still succeed even if metrics fail
214205
expect(result.status.state).toBe('SUCCEEDED')
215206
// onProgress should be called without metrics (undefined)
216-
expect(onProgress).toHaveBeenCalledWith(expect.objectContaining({ status: { state: 'PENDING' } }), undefined)
207+
expect(onProgress).toHaveBeenCalledWith(expect.objectContaining({ status: { state: 'SUCCEEDED' } }), undefined)
217208
})
218209

219210
it('should throw DatabricksSqlError when statement fails', async () => {

0 commit comments

Comments
 (0)