From 30680e62a760378a5b11f2798931f86cf73afefa Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Tue, 15 Sep 2026 19:58:25 +0000 Subject: [PATCH 1/5] fix(gax): map REST fallback transport errors to gRPC status codes Errors raised by the fallback (REST) transport were forwarded to callers untranslated, so they carried either a Node system error string (e.g. `code: 'ECONNRESET'` for a socket hang up) or a raw HTTP status number instead of a numeric gRPC status code. Retry logic in `normalCalls/retries.ts` matches `err.code` against the numeric retry codes from the service config, so such errors never matched and were treated as permanent: a single transient socket hang up failed the call with zero retries. Client libraries that branch on `err.code` were similarly affected. Two things changed: - `validateStatus` is now passed to `auth.fetch()` so that HTTP error responses resolve and flow through the existing `decodeResponse` path, which produces a `GoogleError` via `GoogleError.parseHttpError()`. 401 and 403 continue to reject so that the auth client can refresh credentials. Previously nothing set `validateStatus`, gaxios rejected on every non-2xx response, and the `!response.ok` branch was unreachable. - Errors that still reject (connection failures, timeouts, aborts) are translated by a new `_toGoogleError()` helper into a `GoogleError` with a numeric `code`, preserving the original error on `cause`. No gaxios-level `retryConfig` is introduced, so retries remain governed by the per-method retry codes in each service config. --- core/packages/gax/src/fallbackServiceStub.ts | 88 +++++++++++- core/packages/gax/test/unit/grpc-fallback.ts | 142 ++++++++++++++++++- core/packages/gax/test/unit/utils.ts | 28 ++++ 3 files changed, 252 insertions(+), 6 deletions(-) diff --git a/core/packages/gax/src/fallbackServiceStub.ts b/core/packages/gax/src/fallbackServiceStub.ts index 119e3b3aab4e..f58aa3bb3028 100644 --- a/core/packages/gax/src/fallbackServiceStub.ts +++ b/core/packages/gax/src/fallbackServiceStub.ts @@ -22,6 +22,8 @@ import * as serializer from 'proto3-json-serializer'; import {isNodeJS} from './featureDetection'; import {StreamArrayParser} from './streamArrayParser'; import {defaultToObjectOptions} from './fallback'; +import {GoogleError} from './googleError'; +import {rpcCodeFromHttpStatusCode, Status} from './status'; import {pipeline, PipelineSource} from 'stream'; import type {Agent as HttpAgent} from 'http'; import type {Agent as HttpsAgent} from 'https'; @@ -82,6 +84,79 @@ function _formatEmptyResponse(rpc: protobuf.Method) { return resp; } +/** + * Translates an error thrown by the underlying fetch implementation into a + * {@link GoogleError} carrying a numeric gRPC status code. + * + * Retry logic (see `normalCalls/retries.ts`) and user code both match on + * numeric gRPC status codes. An untranslated error from `auth.fetch()` carries + * either a system error string (e.g. `'ECONNRESET'` for a socket hang up) or an + * HTTP status number, so it never matches a retry code and is silently treated + * as a permanent failure. + * + * @param err The error thrown by `auth.fetch()`. + * @returns A GoogleError with a numeric `code`, or the original value if it is + * not an Error. + */ +function _toGoogleError(err: unknown): unknown { + if (err instanceof GoogleError) { + return err; + } + if (!(err instanceof Error)) { + return err; + } + + const error = new GoogleError(err.message); + error.cause = err; + + // `GaxiosError` shape, described structurally to avoid depending on the + // error instance originating from any particular copy of gaxios. + const fetchError = err as Partial<{ + status: number; + response: {status?: number}; + code: string | number; + }>; + + // Errors that carry an HTTP status (e.g. the 401 and 403 responses that we + // deliberately let the fetch implementation reject with, so that the auth + // client can refresh credentials and retry) map through the standard + // HTTP-to-gRPC table. + const httpStatus = + typeof fetchError.status === 'number' + ? fetchError.status + : fetchError.response?.status; + if (typeof httpStatus === 'number') { + error.code = rpcCodeFromHttpStatusCode(httpStatus); + return error; + } + + // Otherwise this is a connection-level failure identified by a system error + // code. gRPC reports these conditions as UNAVAILABLE. + switch (fetchError.code) { + case 'ECONNRESET': + case 'ECONNREFUSED': + case 'ECONNABORTED': + case 'EPIPE': + case 'ENOTFOUND': + case 'EAI_AGAIN': + case 'ENETUNREACH': + case 'EHOSTUNREACH': + error.code = Status.UNAVAILABLE; + break; + case 'ETIMEDOUT': + case 'TimeoutError': + error.code = Status.DEADLINE_EXCEEDED; + break; + case 'AbortError': + error.code = Status.CANCELLED; + break; + default: + error.code = Status.UNKNOWN; + break; + } + return error; +} + export function generateServiceStub( rpcs: {[name: string]: protobuf.Method}, protocol: string, @@ -164,6 +239,10 @@ export function generateServiceStub( method: fetchParameters.method, signal: cancelSignal, responseType: 'stream', // ensure gaxios returns the data directly so that it handle data/streams itself + // Error responses must resolve so that they are decoded below into a + // GoogleError carrying a gRPC status code. 401 and 403 keep rejecting + // so that the auth client can refresh credentials and retry. + validateStatus: (status: number) => status !== 401 && status !== 403, agent: agentOption || undefined, }; @@ -245,15 +324,16 @@ export function generateServiceStub( } }) .catch((err: unknown) => { + const translated = _toGoogleError(err); if (rpc.responseStream) { if (callback) { - callback(err); + callback(translated); } - streamArrayParser.emit('error', err); + streamArrayParser.emit('error', translated); } else if (callback) { - callback(err); + callback(translated); } else { - throw err; + throw translated; } }); diff --git a/core/packages/gax/test/unit/grpc-fallback.ts b/core/packages/gax/test/unit/grpc-fallback.ts index 732c52e298d4..a80bcf56da0a 100644 --- a/core/packages/gax/test/unit/grpc-fallback.ts +++ b/core/packages/gax/test/unit/grpc-fallback.ts @@ -23,9 +23,10 @@ import * as protobuf from 'protobufjs'; import * as sinon from 'sinon'; import echoProtoJson = require('../fixtures/echo.json'); import {GrpcClient} from '../../src/fallback'; -import {ClientStubOptions, GoogleAuth, GoogleError} from '../../src'; +import {ClientStubOptions, GoogleAuth, GoogleError, Status} from '../../src'; +import {StreamArrayParser} from '../../src/streamArrayParser'; import {PassThroughClient} from 'google-auth-library'; -import {setMockFallbackResponse} from './utils'; +import {setMockFallbackError, setMockFallbackResponse} from './utils'; let authClient = new PassThroughClient(); let opts = { @@ -539,4 +540,141 @@ describe('grpc-fallback', () => { const stub = await gaxGrpc.createStub(echoService, stubOptions); stub.close({}, {}, {}, () => {}); }); + + describe('transport error translation', () => { + // Errors surfaced by the transport must carry a numeric gRPC status code: + // retry logic in normalCalls/retries.ts matches `err.code` against the + // numeric retry codes from the service config, and client libraries branch + // on the same codes. + function callEcho(): Promise { + return gaxGrpc.createStub(echoService, stubOptions).then( + echoStub => + new Promise(resolve => { + echoStub.echo({content: 'test-content'}, {}, {}, (err?: Error) => + resolve(err as GoogleError), + ); + }), + ); + } + + it('should translate a connection failure into UNAVAILABLE', async () => { + // e.g. a "socket hang up" when the server closes a keep-alive socket. + const fetchError = Object.assign( + new Error( + 'request to https://foo.example.com failed, reason: socket hang up', + ), + {code: 'ECONNRESET'}, + ); + setMockFallbackError(gaxGrpc, fetchError); + + const err = await callEcho(); + + assert(err instanceof GoogleError); + assert.strictEqual(err.code, Status.UNAVAILABLE); + assert.strictEqual(err.cause, fetchError); + }); + + it('should translate a timeout into DEADLINE_EXCEEDED', async () => { + setMockFallbackError( + gaxGrpc, + Object.assign(new Error('The operation was aborted due to timeout'), { + code: 'TimeoutError', + }), + ); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.DEADLINE_EXCEEDED); + }); + + it('should translate an aborted request into CANCELLED', async () => { + setMockFallbackError( + gaxGrpc, + Object.assign(new Error('This operation was aborted'), { + code: 'AbortError', + }), + ); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.CANCELLED); + }); + + it('should use UNKNOWN for an unrecognized transport error', async () => { + setMockFallbackError(gaxGrpc, new Error('something unexpected')); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.UNKNOWN); + assert.strictEqual(err.message, 'something unexpected'); + }); + + it('should map a rejection carrying an HTTP status onto a gRPC status', async () => { + // 401 and 403 responses are rejected by the transport on purpose, so that + // the auth client can refresh credentials and retry. + setMockFallbackError( + gaxGrpc, + Object.assign( + new Error('Request had invalid authentication credentials'), + { + status: 401, + }, + ), + ); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.UNAUTHENTICATED); + }); + + it('should not rewrap an error that is already a GoogleError', async () => { + const googleError = new GoogleError('already translated'); + googleError.code = Status.FAILED_PRECONDITION; + setMockFallbackError(gaxGrpc, googleError); + + const err = await callEcho(); + + assert.strictEqual(err, googleError); + assert.strictEqual(err.code, Status.FAILED_PRECONDITION); + }); + + it('should translate transport errors on server streaming calls', async () => { + setMockFallbackError( + gaxGrpc, + Object.assign(new Error('socket hang up'), {code: 'ECONNRESET'}), + ); + + const echoStub = await gaxGrpc.createStub(echoService, stubOptions); + const stream = echoStub.expand( + {content: 'test content'}, + {}, + {}, + () => {}, + ); + + const err = await new Promise(resolve => { + (stream as StreamArrayParser).on('error', resolve); + }); + + assert.strictEqual(err.code, Status.UNAVAILABLE); + }); + + it('should let the auth client handle 401 and 403, and decode every other status', async () => { + const requestOptions = setMockFallbackError(gaxGrpc, new Error('unused')); + + await callEcho(); + + const validateStatus = requestOptions[0].validateStatus; + assert(validateStatus, 'the transport must set validateStatus'); + // Rejected, so that AuthClient can refresh credentials and retry. + assert.strictEqual(validateStatus(401), false); + assert.strictEqual(validateStatus(403), false); + // Resolved, so that the response is decoded into a GoogleError with a + // gRPC status code rather than surfacing as a raw transport error. + assert.strictEqual(validateStatus(404), true); + assert.strictEqual(validateStatus(429), true); + assert.strictEqual(validateStatus(503), true); + assert.strictEqual(validateStatus(200), true); + }); + }); }); diff --git a/core/packages/gax/test/unit/utils.ts b/core/packages/gax/test/unit/utils.ts index 81440541a1fb..2da5041740d3 100644 --- a/core/packages/gax/test/unit/utils.ts +++ b/core/packages/gax/test/unit/utils.ts @@ -143,3 +143,31 @@ export function setMockFallbackResponse( const authClient = new MockedResponseAuthClient(); gaxGrpc.auth = new GoogleAuth({authClient}); } + +/** + * Makes a Fallback request fail the way the real transport does: by rejecting + * from the auth client rather than resolving with a failed response. This is + * what gaxios does for network-level failures, and for HTTP statuses that do + * not pass `validateStatus`. + * + * @param gaxGrpc The gRPC Client to use + * @param error The error the transport should reject with + * @returns The request options the transport was called with + */ +export function setMockFallbackError(gaxGrpc: GrpcClient, error: Error) { + const requestOptions: gaxios.GaxiosOptions[] = []; + + class MockedErrorAuthClient extends PassThroughClient { + async request( + opts: gaxios.GaxiosOptions, + ): Promise> { + requestOptions.push(opts); + throw error; + } + } + + const authClient = new MockedErrorAuthClient(); + gaxGrpc.auth = new GoogleAuth({authClient}); + + return requestOptions; +} From 5c2285deaff1e917d22aed9acd263c926cd2732e Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Tue, 15 Sep 2026 21:37:00 +0000 Subject: [PATCH 2/5] test(gax): add transport parity tests and a gaxios-faithful response mock --- core/packages/gax/src/fallbackServiceStub.ts | 6 +- core/packages/gax/test/unit/grpc-fallback.ts | 112 ++++++++++++++++++- core/packages/gax/test/unit/utils.ts | 49 ++++++++ 3 files changed, 162 insertions(+), 5 deletions(-) diff --git a/core/packages/gax/src/fallbackServiceStub.ts b/core/packages/gax/src/fallbackServiceStub.ts index f58aa3bb3028..5e9b9895a5cb 100644 --- a/core/packages/gax/src/fallbackServiceStub.ts +++ b/core/packages/gax/src/fallbackServiceStub.ts @@ -117,10 +117,8 @@ function _toGoogleError(err: unknown): unknown { code: string | number; }>; - // Errors that carry an HTTP status (e.g. the 401 and 403 responses that we - // deliberately let the fetch implementation reject with, so that the auth - // client can refresh credentials and retry) map through the standard - // HTTP-to-gRPC table. + // Errors that carry an HTTP status map through the standard HTTP-to-gRPC + // table. const httpStatus = typeof fetchError.status === 'number' ? fetchError.status diff --git a/core/packages/gax/test/unit/grpc-fallback.ts b/core/packages/gax/test/unit/grpc-fallback.ts index a80bcf56da0a..b745aa2b9d02 100644 --- a/core/packages/gax/test/unit/grpc-fallback.ts +++ b/core/packages/gax/test/unit/grpc-fallback.ts @@ -25,8 +25,13 @@ import echoProtoJson = require('../fixtures/echo.json'); import {GrpcClient} from '../../src/fallback'; import {ClientStubOptions, GoogleAuth, GoogleError, Status} from '../../src'; import {StreamArrayParser} from '../../src/streamArrayParser'; +import {rpcCodeFromHttpStatusCode} from '../../src/status'; import {PassThroughClient} from 'google-auth-library'; -import {setMockFallbackError, setMockFallbackResponse} from './utils'; +import { + setMockFallbackError, + setMockFallbackHttpResponse, + setMockFallbackResponse, +} from './utils'; let authClient = new PassThroughClient(); let opts = { @@ -677,4 +682,109 @@ describe('grpc-fallback', () => { assert.strictEqual(validateStatus(200), true); }); }); + + describe('transport parity', () => { + // A client must observe the same canonical error code regardless of which + // transport carried the call: retry configuration is expressed in gRPC + // status codes, and user code branches on them. The gRPC transport reports + // the server's canonical code directly, so the fallback transport has to + // arrive at the same value from the HTTP response. + interface ParityCase { + name: string; + httpStatus: number; + status?: string; + expected: Status; + } + + const cases: ParityCase[] = [ + { + name: 'contention', + httpStatus: 409, + status: 'ABORTED', + expected: Status.ABORTED, + }, + { + name: 'bad request', + httpStatus: 400, + status: 'INVALID_ARGUMENT', + expected: Status.INVALID_ARGUMENT, + }, + { + name: 'missing resource', + httpStatus: 404, + status: 'NOT_FOUND', + expected: Status.NOT_FOUND, + }, + { + name: 'quota', + httpStatus: 429, + status: 'RESOURCE_EXHAUSTED', + expected: Status.RESOURCE_EXHAUSTED, + }, + { + name: 'backend unavailable', + httpStatus: 503, + status: 'UNAVAILABLE', + expected: Status.UNAVAILABLE, + }, + // No `status` field: the code must still be derived from the HTTP status + // rather than passed through as an HTTP number. + { + name: 'unavailable without a status field', + httpStatus: 503, + expected: Status.UNAVAILABLE, + }, + { + name: 'conflict without a status field', + httpStatus: 409, + expected: Status.ABORTED, + }, + ]; + + for (const testCase of cases) { + it(`should surface ${Status[testCase.expected]} for ${testCase.name}`, async () => { + const body: {error: {code: number; message: string; status?: string}} = + { + error: { + code: testCase.httpStatus, + message: `${testCase.name} (test)`, + }, + }; + if (testCase.status) { + body.error.status = testCase.status; + } + + setMockFallbackHttpResponse( + gaxGrpc, + new Response(Buffer.from(JSON.stringify(body)), { + status: testCase.httpStatus, + }), + ); + + const echoStub = await gaxGrpc.createStub(echoService, stubOptions); + const err = await new Promise(resolve => { + echoStub.echo({content: 'test'}, {}, {}, (e?: Error) => + resolve(e as GoogleError), + ); + }); + + assert(err instanceof GoogleError); + assert.strictEqual(err.code, testCase.expected); + // The HTTP status must not leak through as the error code. + assert.notStrictEqual(err.code as number, testCase.httpStatus); + }); + } + + it('should produce codes consistent with the shared HTTP-to-gRPC mapping', () => { + // The fallback transport and the mapping table used elsewhere in gax must + // not drift apart. + for (const testCase of cases) { + assert.strictEqual( + rpcCodeFromHttpStatusCode(testCase.httpStatus), + testCase.expected, + `HTTP ${testCase.httpStatus} should map to ${Status[testCase.expected]}`, + ); + } + }); + }); }); diff --git a/core/packages/gax/test/unit/utils.ts b/core/packages/gax/test/unit/utils.ts index 2da5041740d3..017d94b440bb 100644 --- a/core/packages/gax/test/unit/utils.ts +++ b/core/packages/gax/test/unit/utils.ts @@ -171,3 +171,52 @@ export function setMockFallbackError(gaxGrpc: GrpcClient, error: Error) { return requestOptions; } + +/** + * Sets a response for a Fallback request, reproducing how gaxios decides + * between resolving and rejecting. + * + * Unlike {@link setMockFallbackResponse}, which always resolves, this applies + * the request's `validateStatus` predicate (defaulting to gaxios' own 2xx-only + * rule when the caller does not supply one) and rejects with a GaxiosError-shaped + * error when it fails. Transports that do not opt in to receiving error + * responses therefore see a rejection here, exactly as they would in production. + * + * @param gaxGrpc The gRPC Client to use + * @param response The Response object to use + */ +export function setMockFallbackHttpResponse( + gaxGrpc: GrpcClient, + response: Response, +) { + class MockedHttpAuthClient extends PassThroughClient { + async request( + opts: gaxios.GaxiosOptions, + ): Promise> { + const validateStatus = + opts.validateStatus ?? + ((status: number) => status >= 200 && status < 300); + + if (!validateStatus(response.status)) { + throw Object.assign( + new Error(`Request failed with status code ${response.status}`), + { + status: response.status, + response: {status: response.status}, + }, + ); + } + + return Object.assign(response, { + config: { + headers: response.headers, + url: new URL(opts.url || 'https://example.com'), + }, + data: response.body as T, + }); + } + } + + const authClient = new MockedHttpAuthClient(); + gaxGrpc.auth = new GoogleAuth({authClient}); +} From 8191274e0cc3d9bf9eac49a74c30241a8d401688 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:01:19 +0000 Subject: [PATCH 3/5] fix(gax): detect aborts and timeouts by error name `_toGoogleError()` matched `'AbortError'` and `'TimeoutError'` against `err.code`, but no fetch implementation reports them that way. Native fetch rejects with a `DOMException` whose `code` is the numeric DOMException value (20 for abort, 23 for timeout) and whose `name` carries the string; node-fetch sets `name` and no `code`. Both therefore fell through to `UNKNOWN`, so a cancelled call did not surface as `CANCELLED` and a timeout did not surface as `DEADLINE_EXCEEDED`. Match on `err.name` instead, consistent with how the rest of this file detects cancellation, while still accepting a string `code` because gaxios matches errors in that form. The existing tests passed only because they constructed the error by hand with `{code: 'AbortError'}`. They now use the `DOMException` that fetch actually throws, with the string-`code` form kept as a separate case. --- core/packages/gax/src/fallbackServiceStub.ts | 18 ++++++++++--- core/packages/gax/test/unit/grpc-fallback.ts | 28 ++++++++++++++++++++ 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/core/packages/gax/src/fallbackServiceStub.ts b/core/packages/gax/src/fallbackServiceStub.ts index 5e9b9895a5cb..770bd259ae59 100644 --- a/core/packages/gax/src/fallbackServiceStub.ts +++ b/core/packages/gax/src/fallbackServiceStub.ts @@ -128,6 +128,20 @@ function _toGoogleError(err: unknown): unknown { return error; } + // Cancellations and timeouts are reported as a DOMException by native fetch, + // where `code` is a numeric DOMException value (20 and 23) rather than a + // string, and by name alone under node-fetch. Match on `name`, as the rest of + // this file does when it detects cancellation, and accept a string `code` too + // because gaxios matches errors in that form. + if (err.name === 'AbortError' || fetchError.code === 'AbortError') { + error.code = Status.CANCELLED; + return error; + } + if (err.name === 'TimeoutError' || fetchError.code === 'TimeoutError') { + error.code = Status.DEADLINE_EXCEEDED; + return error; + } + // Otherwise this is a connection-level failure identified by a system error // code. gRPC reports these conditions as UNAVAILABLE. switch (fetchError.code) { @@ -142,12 +156,8 @@ function _toGoogleError(err: unknown): unknown { error.code = Status.UNAVAILABLE; break; case 'ETIMEDOUT': - case 'TimeoutError': error.code = Status.DEADLINE_EXCEEDED; break; - case 'AbortError': - error.code = Status.CANCELLED; - break; default: error.code = Status.UNKNOWN; break; diff --git a/core/packages/gax/test/unit/grpc-fallback.ts b/core/packages/gax/test/unit/grpc-fallback.ts index b745aa2b9d02..494b0b9d41d2 100644 --- a/core/packages/gax/test/unit/grpc-fallback.ts +++ b/core/packages/gax/test/unit/grpc-fallback.ts @@ -580,6 +580,22 @@ describe('grpc-fallback', () => { }); it('should translate a timeout into DEADLINE_EXCEEDED', async () => { + // Native fetch rejects with a DOMException whose `code` is the numeric + // DOMException value (23), not a string, so the name is what identifies it. + setMockFallbackError( + gaxGrpc, + new DOMException( + 'The operation was aborted due to timeout', + 'TimeoutError', + ), + ); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.DEADLINE_EXCEEDED); + }); + + it('should translate a timeout reported only by code into DEADLINE_EXCEEDED', async () => { setMockFallbackError( gaxGrpc, Object.assign(new Error('The operation was aborted due to timeout'), { @@ -593,6 +609,18 @@ describe('grpc-fallback', () => { }); it('should translate an aborted request into CANCELLED', async () => { + // As above: native fetch reports abort as DOMException code 20. + setMockFallbackError( + gaxGrpc, + new DOMException('This operation was aborted', 'AbortError'), + ); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.CANCELLED); + }); + + it('should translate an abort reported only by code into CANCELLED', async () => { setMockFallbackError( gaxGrpc, Object.assign(new Error('This operation was aborted'), { From be66023bdfff07255548fee2becefd203ab19cf7 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:12:35 +0000 Subject: [PATCH 4/5] test(gax): replace the transport parity suite with two decode tests The suite was named for a guarantee it did not provide: it never exercised the gRPC transport, so it compared the fallback transport against a hardcoded table rather than against gRPC. Its closing test pinned that table to rpcCodeFromHttpStatusCode, the same function the transport calls, so it could only fail if one of the two were edited in isolation. Five of its seven cases also duplicated test/unit/status.ts, which already covers the HTTP-to-gRPC map directly. What the suite did uniquely cover is the resolve path: every test in 'transport error translation' uses setMockFallbackError, so none of them put a non-2xx Response through validateStatus and decodeResponse. Two tests are kept for that, folded into the existing suite. One asserts that the code denotes the canonical status the server named; the other that a body without a status field still derives its code from the HTTP status. Both still fail against the pre-fix transport. --- core/packages/gax/test/unit/grpc-fallback.ts | 144 ++++++------------- 1 file changed, 45 insertions(+), 99 deletions(-) diff --git a/core/packages/gax/test/unit/grpc-fallback.ts b/core/packages/gax/test/unit/grpc-fallback.ts index 494b0b9d41d2..3626d922eacd 100644 --- a/core/packages/gax/test/unit/grpc-fallback.ts +++ b/core/packages/gax/test/unit/grpc-fallback.ts @@ -25,7 +25,6 @@ import echoProtoJson = require('../fixtures/echo.json'); import {GrpcClient} from '../../src/fallback'; import {ClientStubOptions, GoogleAuth, GoogleError, Status} from '../../src'; import {StreamArrayParser} from '../../src/streamArrayParser'; -import {rpcCodeFromHttpStatusCode} from '../../src/status'; import {PassThroughClient} from 'google-auth-library'; import { setMockFallbackError, @@ -709,110 +708,57 @@ describe('grpc-fallback', () => { assert.strictEqual(validateStatus(503), true); assert.strictEqual(validateStatus(200), true); }); - }); - - describe('transport parity', () => { - // A client must observe the same canonical error code regardless of which - // transport carried the call: retry configuration is expressed in gRPC - // status codes, and user code branches on them. The gRPC transport reports - // the server's canonical code directly, so the fallback transport has to - // arrive at the same value from the HTTP response. - interface ParityCase { - name: string; - httpStatus: number; - status?: string; - expected: Status; - } - const cases: ParityCase[] = [ - { - name: 'contention', - httpStatus: 409, - status: 'ABORTED', - expected: Status.ABORTED, - }, - { - name: 'bad request', - httpStatus: 400, - status: 'INVALID_ARGUMENT', - expected: Status.INVALID_ARGUMENT, - }, - { - name: 'missing resource', - httpStatus: 404, - status: 'NOT_FOUND', - expected: Status.NOT_FOUND, - }, - { - name: 'quota', - httpStatus: 429, - status: 'RESOURCE_EXHAUSTED', - expected: Status.RESOURCE_EXHAUSTED, - }, - { - name: 'backend unavailable', - httpStatus: 503, - status: 'UNAVAILABLE', - expected: Status.UNAVAILABLE, - }, - // No `status` field: the code must still be derived from the HTTP status - // rather than passed through as an HTTP number. - { - name: 'unavailable without a status field', - httpStatus: 503, - expected: Status.UNAVAILABLE, - }, - { - name: 'conflict without a status field', - httpStatus: 409, - expected: Status.ABORTED, - }, - ]; + it('should decode a non-2xx response into its canonical gRPC status', async () => { + // The primary half of the fix: validateStatus lets this response resolve, + // so it reaches decodeResponse and is parsed rather than surfacing as a + // raw transport rejection. parseHttpError prefers the canonical status + // name in the body. + setMockFallbackHttpResponse( + gaxGrpc, + new Response( + Buffer.from( + JSON.stringify({ + error: { + code: 409, + message: 'Too much contention on these documents.', + status: 'ABORTED', + }, + }), + ), + {status: 409}, + ), + ); - for (const testCase of cases) { - it(`should surface ${Status[testCase.expected]} for ${testCase.name}`, async () => { - const body: {error: {code: number; message: string; status?: string}} = - { - error: { - code: testCase.httpStatus, - message: `${testCase.name} (test)`, - }, - }; - if (testCase.status) { - body.error.status = testCase.status; - } + const err = await callEcho(); - setMockFallbackHttpResponse( - gaxGrpc, - new Response(Buffer.from(JSON.stringify(body)), { - status: testCase.httpStatus, - }), - ); + assert(err instanceof GoogleError); + assert.strictEqual(err.code, Status.ABORTED); + // The code must denote the status the server named, which is the same + // value the gRPC transport reports for this condition. + assert.strictEqual(Status[err.code!], 'ABORTED'); + // The HTTP status must not leak through as the error code. + assert.notStrictEqual(err.code as number, 409); + }); - const echoStub = await gaxGrpc.createStub(echoService, stubOptions); - const err = await new Promise(resolve => { - echoStub.echo({content: 'test'}, {}, {}, (e?: Error) => - resolve(e as GoogleError), - ); - }); + it('should derive a code from the HTTP status when the body has none', async () => { + setMockFallbackHttpResponse( + gaxGrpc, + new Response( + Buffer.from( + JSON.stringify({ + error: {code: 503, message: 'The service is currently down.'}, + }), + ), + {status: 503}, + ), + ); - assert(err instanceof GoogleError); - assert.strictEqual(err.code, testCase.expected); - // The HTTP status must not leak through as the error code. - assert.notStrictEqual(err.code as number, testCase.httpStatus); - }); - } + const err = await callEcho(); - it('should produce codes consistent with the shared HTTP-to-gRPC mapping', () => { - // The fallback transport and the mapping table used elsewhere in gax must - // not drift apart. - for (const testCase of cases) { - assert.strictEqual( - rpcCodeFromHttpStatusCode(testCase.httpStatus), - testCase.expected, - `HTTP ${testCase.httpStatus} should map to ${Status[testCase.expected]}`, - ); - } + assert(err instanceof GoogleError); + assert.strictEqual(err.code, Status.UNAVAILABLE); + assert.notStrictEqual(err.code as number, 503); }); }); }); From 8f92a6e56f190c250b944030a8d1154abbc7d38e Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:38:33 +0000 Subject: [PATCH 5/5] fix(gax): report transport failures as UNAVAILABLE, as gRPC does The previous commit enumerated system error codes and mapped anything it did not recognize to UNKNOWN. That table was invented rather than derived, and it diverged from the transport it is meant to stand in for. @grpc/grpc-js does not enumerate errnos. It defaults transport failures to UNAVAILABLE (subchannel-call.js, the write and stream error paths; picker.js for 'No connection established'; resolver-dns.js for resolution failures) and inspects errno only to refine an HTTP/2 INTERNAL_ERROR, where it treats ECONNRESET and ETIMEDOUT as UNAVAILABLE. Two consequences of the table were wrong: - ETIMEDOUT became DEADLINE_EXCEEDED, where gRPC reports UNAVAILABLE. DEADLINE_EXCEEDED is for an elapsed call deadline, not a socket timeout. Since Firestore's Commit retries only RESOURCE_EXHAUSTED and UNAVAILABLE, a socket timeout would still not have been retried. - Any errno absent from the list became UNKNOWN and so non-retryable, and such a list can only ever be incomplete. Cancellation and an elapsed deadline stay separate, since gRPC distinguishes them. Everything else that rejects before a response is produced is now UNAVAILABLE. Decode errors are handled nearer the decoder and do not reach this path, so they are not affected. --- core/packages/gax/src/fallbackServiceStub.ts | 44 +++++++++----------- core/packages/gax/test/unit/grpc-fallback.ts | 20 ++++++++- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/core/packages/gax/src/fallbackServiceStub.ts b/core/packages/gax/src/fallbackServiceStub.ts index 770bd259ae59..b1dc131d734f 100644 --- a/core/packages/gax/src/fallbackServiceStub.ts +++ b/core/packages/gax/src/fallbackServiceStub.ts @@ -94,6 +94,11 @@ function _formatEmptyResponse(rpc: protobuf.Method) { * HTTP status number, so it never matches a retry code and is silently treated * as a permanent failure. * + * An error carrying an HTTP status maps through `rpcCodeFromHttpStatusCode`. + * Otherwise the call failed before producing a response, which is reported as + * UNAVAILABLE, except for an explicit cancellation (CANCELLED) or an elapsed + * deadline (DEADLINE_EXCEEDED). + * * @param err The error thrown by `auth.fetch()`. * @returns A GoogleError with a numeric `code`, or the original value if it is * not an Error. @@ -128,11 +133,12 @@ function _toGoogleError(err: unknown): unknown { return error; } - // Cancellations and timeouts are reported as a DOMException by native fetch, - // where `code` is a numeric DOMException value (20 and 23) rather than a - // string, and by name alone under node-fetch. Match on `name`, as the rest of - // this file does when it detects cancellation, and accept a string `code` too - // because gaxios matches errors in that form. + // An explicit cancellation and an elapsed deadline are distinct conditions in + // gRPC, so they are separated out before the general case below. Native fetch + // reports both as a DOMException, where `code` is a numeric DOMException + // value (20 and 23) rather than a string, so match on `name` as the rest of + // this file does when it detects cancellation. A string `code` is also + // accepted, because gaxios normalizes a DOMException's name onto `code`. if (err.name === 'AbortError' || fetchError.code === 'AbortError') { error.code = Status.CANCELLED; return error; @@ -142,26 +148,14 @@ function _toGoogleError(err: unknown): unknown { return error; } - // Otherwise this is a connection-level failure identified by a system error - // code. gRPC reports these conditions as UNAVAILABLE. - switch (fetchError.code) { - case 'ECONNRESET': - case 'ECONNREFUSED': - case 'ECONNABORTED': - case 'EPIPE': - case 'ENOTFOUND': - case 'EAI_AGAIN': - case 'ENETUNREACH': - case 'EHOSTUNREACH': - error.code = Status.UNAVAILABLE; - break; - case 'ETIMEDOUT': - error.code = Status.DEADLINE_EXCEEDED; - break; - default: - error.code = Status.UNKNOWN; - break; - } + // Anything else that rejects here failed before producing a response, which + // gRPC reports as UNAVAILABLE irrespective of the underlying system error: + // @grpc/grpc-js defaults transport failures to UNAVAILABLE and only inspects + // errno to refine an HTTP/2 INTERNAL_ERROR. Enumerating errnos here would + // classify anything left off the list as non-retryable, so follow gRPC and + // treat the whole category uniformly. Errors raised while decoding a response + // are handled nearer the decoder and do not reach this point. + error.code = Status.UNAVAILABLE; return error; } diff --git a/core/packages/gax/test/unit/grpc-fallback.ts b/core/packages/gax/test/unit/grpc-fallback.ts index 3626d922eacd..42f745c04fbe 100644 --- a/core/packages/gax/test/unit/grpc-fallback.ts +++ b/core/packages/gax/test/unit/grpc-fallback.ts @@ -632,15 +632,31 @@ describe('grpc-fallback', () => { assert.strictEqual(err.code, Status.CANCELLED); }); - it('should use UNKNOWN for an unrecognized transport error', async () => { + it('should use UNAVAILABLE for an unrecognized transport error', async () => { + // An error that rejected before a response was produced is a transport + // failure, which gRPC reports as UNAVAILABLE whatever the cause. Codes + // left off a list would otherwise be classified as non-retryable. setMockFallbackError(gaxGrpc, new Error('something unexpected')); const err = await callEcho(); - assert.strictEqual(err.code, Status.UNKNOWN); + assert.strictEqual(err.code, Status.UNAVAILABLE); assert.strictEqual(err.message, 'something unexpected'); }); + it('should translate a socket timeout into UNAVAILABLE', async () => { + // @grpc/grpc-js maps ETIMEDOUT to UNAVAILABLE, not DEADLINE_EXCEEDED, + // which is reserved for an elapsed call deadline. + setMockFallbackError( + gaxGrpc, + Object.assign(new Error('socket timeout'), {code: 'ETIMEDOUT'}), + ); + + const err = await callEcho(); + + assert.strictEqual(err.code, Status.UNAVAILABLE); + }); + it('should map a rejection carrying an HTTP status onto a gRPC status', async () => { // 401 and 403 responses are rejected by the transport on purpose, so that // the auth client can refresh credentials and retry.