Skip to content

Commit 264beb5

Browse files
committed
Fix: Improve error handling to distinguish network errors from TLS failures
Separates specific network transport errors (`ETIMEDOUT`, `ECONNRESET`) from genuine TLS handshake failures. The previous approach incorrectly categorized lower-level connection issues as "TLS errors," leading to misleading diagnostics for end-users. This change ensures accurate reporting based on the error pattern: - "tls handshake": Protocol/certificate issue. - "etimedout" / "timed out": Network timeout/availability. - "econnreset": Connection forcefully reset by host/intermediary.
1 parent fa8a54d commit 264beb5

File tree

2 files changed

+41
-16
lines changed

2 files changed

+41
-16
lines changed

src/nodejs-common/util.ts

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,12 @@ export interface ParsedHttpResponseBody {
256256
err?: Error;
257257
}
258258

259+
export enum UtilExceptionMessages {
260+
TLS_TIMEOUT_ERROR_MESSAGE = 'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.',
261+
ETIMEDOUT_ERROR_MESSAGE = 'Connection timed out. This suggests a network issue or the server took too long to respond.',
262+
ECONNRESET_ERROR_MESSAGE = 'Connection reset by peer. The connection was closed unexpectedly, possibly by a firewall or network issue.',
263+
}
264+
259265
/**
260266
* Custom error type for API errors.
261267
*
@@ -914,15 +920,29 @@ export class Util {
914920
(err: Error | null, response: {}, body: any) => {
915921
if (err) {
916922
const lowerCaseMessage = err.message.toLowerCase();
917-
const isTLsTimeoutOrConnReset =
918-
/tls handshake|timed out|etimedout|econnreset/.test(
919-
lowerCaseMessage
923+
let newError: Error | undefined;
924+
925+
if (lowerCaseMessage.includes('tls handshake')) {
926+
newError = new Error(
927+
UtilExceptionMessages.TLS_TIMEOUT_ERROR_MESSAGE
928+
);
929+
} else if (
930+
lowerCaseMessage.includes('etimedout') ||
931+
lowerCaseMessage.includes('timed out')
932+
) {
933+
newError = new Error(
934+
UtilExceptionMessages.ETIMEDOUT_ERROR_MESSAGE
935+
);
936+
} else if (lowerCaseMessage.includes('econnreset')) {
937+
newError = new Error(
938+
UtilExceptionMessages.ECONNRESET_ERROR_MESSAGE
920939
);
921-
if (isTLsTimeoutOrConnReset) {
922-
const TLS_TIMEOUT_ERROR_MESSAGE =
923-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.';
924-
const timeOutError = new Error(TLS_TIMEOUT_ERROR_MESSAGE);
925-
err = timeOutError;
940+
}
941+
942+
if (newError) {
943+
// Preserve the original stack trace for better debugging
944+
newError.stack = err.stack;
945+
err = newError;
926946
}
927947
}
928948
util.handleResp(err, response as {} as r.Response, body, callback!);

test/nodejs-common/util.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import {
4646
MakeRequestConfig,
4747
ParsedHttpRespMessage,
4848
Util,
49+
UtilExceptionMessages,
4950
} from '../../src/nodejs-common/util.js';
5051
import {DEFAULT_PROJECT_ID_TOKEN} from '../../src/nodejs-common/service.js';
5152
import duplexify from 'duplexify';
@@ -1189,31 +1190,38 @@ describe('common/util', () => {
11891190
});
11901191
});
11911192

1192-
describe('TLS handshake errors', () => {
1193+
describe('Handling of TLS Handshake, Timeout, and Connection Reset Errors in Authenticated Requests', () => {
11931194
const reqOpts = fakeReqOpts;
11941195
beforeEach(() => {
11951196
authClient.authorizeRequest = async () => reqOpts;
11961197
sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);
11971198
});
11981199

11991200
const testCases = [
1200-
{name: 'ECONNRESET', error: new Error('ECONNRESET')},
1201+
{
1202+
name: 'ECONNRESET',
1203+
error: new Error('ECONNRESET'),
1204+
expectedMessage: UtilExceptionMessages.ECONNRESET_ERROR_MESSAGE,
1205+
},
12011206
{
12021207
name: '"TLS handshake"',
12031208
error: new Error('Request failed due to TLS handshake timeout.'),
1209+
expectedMessage: UtilExceptionMessages.TLS_TIMEOUT_ERROR_MESSAGE,
12041210
},
12051211
{
12061212
name: 'generic "timed out"',
12071213
error: new Error('The request timed out.'),
1214+
expectedMessage: UtilExceptionMessages.ETIMEDOUT_ERROR_MESSAGE,
12081215
},
12091216
{
12101217
name: 'ETIMEDOUT',
12111218
error: new Error('Request failed with error: ETIMEDOUT'),
1219+
expectedMessage: UtilExceptionMessages.ETIMEDOUT_ERROR_MESSAGE,
12121220
},
12131221
];
12141222

1215-
testCases.forEach(({name, error: networkError}) => {
1216-
it(`should transform raw ${name} into specific TLS/CPU starvation Error`, done => {
1223+
testCases.forEach(({name, error: networkError, expectedMessage}) => {
1224+
it(`should transform raw ${name} into specific network error`, done => {
12171225
// Override `retry-request` to simulate a network error.
12181226
retryRequestOverride = (
12191227
// eslint-disable-next-line @typescript-eslint/no-explicit-any
@@ -1231,10 +1239,7 @@ describe('common/util', () => {
12311239

12321240
makeAuthenticatedRequest({} as DecorateRequestOptions, err => {
12331241
assert.ok(err);
1234-
assert.strictEqual(
1235-
err.message,
1236-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1237-
);
1242+
assert.strictEqual(err!.message, expectedMessage);
12381243
done();
12391244
});
12401245
});

0 commit comments

Comments
 (0)