diff --git a/CHANGELOG.md b/CHANGELOG.md index 957853dc..6e70b188 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- CloudFetch retry-exhaustion errors now identify the download and the last failure: the `RetryError` message reads `CloudFetch result download failed: (last error: )`. `` is `HTTP : ` (or `HTTP ` when the status text is empty) for HTTP failures, and `network error `, `network error ()` (e.g. `aborted`, when there is no `code`) or a bare `network error` for network failures (the raw network-error message, which can contain the presigned URL, is not copied). If no cause can be determined, the ` (last error: …)` suffix is omitted. The `RetryError` instance, `errorCode` and `payload` (including the node-fetch `Request` / `Response`) are unchanged. (#471) - Report untyped NULL columns as STRING in kernel result metadata, matching Thrift. (#470) ## 3.0.0 diff --git a/lib/result/CloudFetchResultHandler.ts b/lib/result/CloudFetchResultHandler.ts index 755f2a24..a64bb4bb 100644 --- a/lib/result/CloudFetchResultHandler.ts +++ b/lib/result/CloudFetchResultHandler.ts @@ -1,6 +1,7 @@ import fetch, { RequestInfo, RequestInit, Request, Response } from 'node-fetch'; import { TGetResultSetMetadataResp, TRowSet, TSparkArrowResultLink } from '../../thrift/TCLIService_types'; import HiveDriverError from '../errors/HiveDriverError'; +import RetryError from '../errors/RetryError'; import IClientContext from '../contracts/IClientContext'; import IResultsProvider, { ResultsProviderFetchNextOptions } from './IResultsProvider'; import { ArrowBatch } from './utils'; @@ -157,15 +158,20 @@ export default class CloudFetchResultHandler implements IResultsProvider { - downloaded = undefined; - const request = new Request(url, requestConfig); - const response = await fetch(request); - if (response.ok) { - downloaded = await response.arrayBuffer(); - } - return { request, response }; - }); + let result: { request: Request; response: Response }; + try { + result = await retryPolicy.invokeWithRetry(async () => { + downloaded = undefined; + const request = new Request(url, requestConfig); + const response = await fetch(request); + if (response.ok) { + downloaded = await response.arrayBuffer(); + } + return { request, response }; + }); + } catch (error) { + throw CloudFetchResultHandler.describeDownloadError(error); + } // Fall back to reading the body here if the retry policy returned a // response without consuming it via our operation (e.g. unit-test stubs // that hand back a pre-baked Response without invoking the operation @@ -177,6 +183,58 @@ export default class CloudFetchResultHandler implements IResultsProvider { const chunk = await operation.fetchChunk({ maxRows: 100000, disableBuffering: true }); expect(chunk.length).to.be.gt(0); }); + + it('should surface HTTP status and download context when CloudFetch retries are exhausted', async () => { + const retryMaxAttempts = 2; + const session = await openSession({ + cloudFetchConcurrentDownloads: 1, + useLZ4Compression: false, + retryMaxAttempts, + retryDelayMin: 1, + retryDelayMax: 10, + }); + + let serverHits = 0; + const server = http.createServer((req, res) => { + serverHits += 1; + res.writeHead(500, 'Internal Server Error', { 'Content-Type': 'text/plain' }); + res.end('Internal Server Error'); + }); + await new Promise((resolve) => { + server.listen(0, '127.0.0.1', () => resolve()); + }); + const { port } = server.address() as AddressInfo; + + try { + const queriedRowsCount = 10000000; // result has to be quite big to enable CloudFetch + const operation = await session.executeStatement( + ` + SELECT * + FROM range(0, ${queriedRowsCount}) AS t1 + LEFT JOIN (SELECT 1) AS t2 + `, + { + maxRows: null, // disable DirectResults + useCloudFetch: true, + }, + ); + + await operation.finished(); + + // @ts-expect-error TS2339: Property getResultHandler does not exist on type IOperation + const resultHandler = await operation.getResultHandler(); + expect(resultHandler.source.source).to.be.instanceOf(CloudFetchResultHandler); + const cfResultHandler = resultHandler.source.source; + + // Plain-HTTP local server: let node-fetch pick its default agent for the download. + // The stub is scoped to the `fetchChunk` call below and restored right after it, so + // later requests on this connection provider (e.g. `session.close()`) keep the real agent + // (proxy / custom CA / keep-alive config) + const connectionProvider = await cfResultHandler.context.getConnectionProvider(); + const getAgentStub = sinon.stub(connectionProvider, 'getAgent').resolves(undefined); + + // Redirect every presigned link to the local server that always responds with HTTP 500, + // keeping the real expiry time and row count returned by the warehouse + const rowSetProvider = cfResultHandler.source; + const originalFetchNext = rowSetProvider.fetchNext.bind(rowSetProvider); + const originalFileLinks: Array = []; + sinon.stub(rowSetProvider, 'fetchNext').callsFake(async (...args: Array) => { + const rowSet = await originalFetchNext(...args); + rowSet?.resultLinks?.forEach((link: any) => { + originalFileLinks.push(link.fileLink); + link.fileLink = `http://127.0.0.1:${port}/result`; + link.httpHeaders = {}; + }); + return rowSet; + }); + + let thrown: unknown; + try { + await operation.fetchChunk({ maxRows: 100000, disableBuffering: true }); + } catch (error) { + thrown = error; + } finally { + getAgentStub.restore(); + } + + // Pin the retry-exhaustion path (not the plain `CloudFetch HTTP error ...` thrown by `downloadLink`) + expect(thrown).to.be.instanceOf(RetryError); + expect((thrown as RetryError).errorCode).to.be.equal(RetryErrorCode.AttemptsExceeded); + expect(serverHits).to.be.equal(retryMaxAttempts); + const { message } = thrown as RetryError; + expect(message).to.contain('CloudFetch result download failed'); + expect(message).to.contain('Max retry count exceeded'); + expect(message).to.contain('HTTP 500: Internal Server Error'); + + // The real presigned link (host and credentialed query string) must not leak into the message + expect(originalFileLinks.length).to.be.gt(0); + originalFileLinks.forEach((fileLink) => { + const { host, search } = new URL(fileLink); + expect(message).to.not.contain(host); + if (search) { + expect(message).to.not.contain(search); + } + }); + } finally { + await new Promise((resolve) => { + server.close(() => resolve()); + }); + await session.close(); + } + }); }); diff --git a/tests/unit/result/CloudFetchResultHandler.test.ts b/tests/unit/result/CloudFetchResultHandler.test.ts index 93ab712a..030e4775 100644 --- a/tests/unit/result/CloudFetchResultHandler.test.ts +++ b/tests/unit/result/CloudFetchResultHandler.test.ts @@ -10,6 +10,7 @@ import CloudFetchResultHandler from '../../../lib/result/CloudFetchResultHandler import ResultsProviderStub from '../.stubs/ResultsProviderStub'; import { TRowSet, TSparkArrowResultLink, TStatusCode } from '../../../thrift/TCLIService_types'; import { ArrowBatch } from '../../../lib/result/utils'; +import RetryError, { RetryErrorCode } from '../../../lib/errors/RetryError'; import BaseClientContextStub from '../.stubs/ClientContextStub'; import { ClientConfig } from '../../../lib/contracts/IClientContext'; @@ -414,6 +415,261 @@ describe('CloudFetchResultHandler', () => { } }); + it('should add download context and HTTP status to RetryError when retries are exhausted', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + context.invokeWithRetryStub.callsFake(async () => { + throw new RetryError(RetryErrorCode.AttemptsExceeded, { + request: new Request('http://example.com/result/1'), + response: new Response('', { status: 500, statusText: 'Internal Server Error' }), + }); + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.instanceOf(RetryError); + expect(thrown.errorCode).to.be.equal(RetryErrorCode.AttemptsExceeded); + expect(thrown.message).to.contain('CloudFetch'); + expect(thrown.message).to.contain('HTTP 500'); + expect(thrown.message).to.contain('Internal Server Error'); + expect(thrown.message).to.contain('Max retry count exceeded'); + expect(thrown.stack.split('\n')[0]).to.be.equal(`Error: ${thrown.message}`); + expect(thrown.stack).to.not.contain('describeDownloadError'); + }); + + it('should add download context and HTTP status to RetryError when retry timeout is exceeded', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + context.invokeWithRetryStub.callsFake(async () => { + throw new RetryError(RetryErrorCode.TimeoutExceeded, { + request: new Request('http://example.com/result/1'), + response: new Response('', { status: 503, statusText: 'Service Unavailable' }), + }); + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.instanceOf(RetryError); + expect(thrown.errorCode).to.be.equal(RetryErrorCode.TimeoutExceeded); + expect(thrown.message).to.be.equal( + 'CloudFetch result download failed: Retry timeout exceeded (last error: HTTP 503: Service Unavailable)', + ); + expect(thrown.stack.split('\n')[0]).to.be.equal(`Error: ${thrown.message}`); + }); + + it('should add download context and network error cause to RetryError when retries are exhausted', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + const originalError = new RetryError( + RetryErrorCode.AttemptsExceeded, + Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), + ); + context.invokeWithRetryStub.callsFake(async () => { + throw originalError; + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.equal(originalError); + expect(thrown.stack.split('\n')[0]).to.be.equal(`Error: ${thrown.message}`); + expect(thrown).to.be.instanceOf(RetryError); + expect(thrown.errorCode).to.be.equal(RetryErrorCode.AttemptsExceeded); + expect(thrown.message).to.be.equal( + 'CloudFetch result download failed: Max retry count exceeded (last error: network error ECONNRESET)', + ); + }); + + it('should not prefix the same RetryError instance twice when it is re-thrown', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + + const originalError = new RetryError( + RetryErrorCode.AttemptsExceeded, + Object.assign(new Error('socket hang up'), { code: 'ECONNRESET' }), + ); + context.invokeWithRetryStub.callsFake(async () => { + throw originalError; + }); + + const expectedMessage = + 'CloudFetch result download failed: Max retry count exceeded (last error: network error ECONNRESET)'; + + for (let i = 0; i < 2; i += 1) { + const result = new CloudFetchResultHandler(context, new ResultsProviderStub([sampleRowSet1], undefined), { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + let thrown: any; + try { + // eslint-disable-next-line no-await-in-loop + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.equal(originalError); + expect(thrown.message).to.be.equal(expectedMessage); + expect(thrown.stack.split('\n')[0]).to.be.equal(`Error: ${expectedMessage}`); + } + }); + + it('should not leak presigned URLs from network errors into the RetryError message', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + const presignedUrl = 'https://storage.example.com/results/chunk-0?sig=SECRET_SIGNATURE&se=2026-11-06'; + const originalError = new RetryError( + RetryErrorCode.AttemptsExceeded, + Object.assign(new Error(`request to ${presignedUrl} failed, reason: socket hang up`), { + code: 'ECONNRESET', + type: 'system', + }), + ); + context.invokeWithRetryStub.callsFake(async () => { + throw originalError; + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.equal(originalError); + expect(thrown.message).to.contain('ECONNRESET'); + expect(thrown.message).to.not.contain('SECRET_SIGNATURE'); + expect(thrown.message).to.not.contain('storage.example.com'); + expect(thrown.stack.split('\n')[0]).to.not.contain('SECRET_SIGNATURE'); + }); + + it('should use the network error type when no code is available', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + const originalError = new RetryError( + RetryErrorCode.AttemptsExceeded, + Object.assign(new Error('request to https://storage.example.com/x?sig=SECRET failed'), { type: 'aborted' }), + ); + context.invokeWithRetryStub.callsFake(async () => { + throw originalError; + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.equal(originalError); + expect(thrown.message).to.be.equal( + 'CloudFetch result download failed: Max retry count exceeded (last error: network error (aborted))', + ); + }); + + it('should preserve the original node-fetch request/response payload of a RetryError', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + const presignedUrl = 'https://storage.example.com/results/chunk-0?sig=SECRET_SIGNATURE&se=2026-11-06'; + const request = new Request(presignedUrl); + const response = new Response('', { status: 503, statusText: 'Service Unavailable', url: presignedUrl } as any); + const payload = { request, response }; + const originalError = new RetryError(RetryErrorCode.AttemptsExceeded, payload); + context.invokeWithRetryStub.callsFake(async () => { + throw originalError; + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.equal(originalError); + expect(thrown).to.be.instanceOf(RetryError); + expect(thrown.errorCode).to.be.equal(RetryErrorCode.AttemptsExceeded); + expect(thrown.message).to.contain('HTTP 503: Service Unavailable'); + expect(thrown.message).to.not.contain('SECRET_SIGNATURE'); + expect(thrown.payload).to.be.equal(payload); + expect(thrown.payload.request).to.be.equal(request); + expect(thrown.payload.response).to.be.equal(response); + expect(thrown.payload.response).to.be.instanceOf(Response); + }); + + it('should re-throw errors other than RetryError unchanged', async () => { + const context = new ClientContextStub({ cloudFetchConcurrentDownloads: 1 }); + const rowSetProvider = new ResultsProviderStub([sampleRowSet1], undefined); + + const result = new CloudFetchResultHandler(context, rowSetProvider, { + status: { statusCode: TStatusCode.SUCCESS_STATUS }, + }); + + const originalError = Object.assign(new Error('unable to verify the first certificate'), { + code: 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + }); + const originalMessage = originalError.message; + const originalStack = originalError.stack; + context.invokeWithRetryStub.callsFake(async () => { + throw originalError; + }); + + let thrown: any; + try { + await result.fetchNext({ limit: 10000 }); + } catch (e) { + thrown = e; + } + + expect(thrown).to.be.equal(originalError); + expect(thrown.code).to.be.equal('UNABLE_TO_VERIFY_LEAF_SIGNATURE'); + expect(thrown.message).to.be.equal(originalMessage); + expect(thrown.stack).to.be.equal(originalStack); + }); + it('should handle expired links', async () => { const context = new ClientContextStub(); const rowSetProvider = new ResultsProviderStub([sampleExpiredRowSet], undefined);