Skip to content

Commit 286a6b2

Browse files
committed
refactor: Combine duplicate TLS error tests into data-driven suite
Replaces repetitive test cases in `makeAuthenticatedRequest` and `makeRequest` with a single, data-driven test loop. This verifies all conditions (ECONNRESET, ETIMEDOUT, "timed out", "TLS handshake") with reduced code duplication and improved maintenance. ```
1 parent 0d43c77 commit 286a6b2

File tree

2 files changed

+43
-222
lines changed

2 files changed

+43
-222
lines changed

src/nodejs-common/util.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -920,9 +920,10 @@ export class Util {
920920
lowerCaseMessage.includes('etimedout') ||
921921
lowerCaseMessage.includes('econnreset');
922922
if (isTLsTimeoutOrConnReset) {
923-
err = new Error(
923+
const timeOutError = new Error(
924924
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
925925
);
926+
err = timeOutError;
926927
}
927928
}
928929
util.handleResp(err, response as {} as r.Response, body, callback!);

test/nodejs-common/util.ts

Lines changed: 41 additions & 221 deletions
Original file line numberDiff line numberDiff line change
@@ -1190,136 +1190,53 @@ describe('common/util', () => {
11901190
});
11911191

11921192
describe('TLS handshake errors', () => {
1193-
function createMakeRequestStub(
1194-
sandbox: sinon.SinonSandbox,
1195-
networkError: Error,
1196-
util: Util & {[index: string]: Function},
1197-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1198-
authClient: any
1199-
) {
1200-
const authorizedReqOpts = {uri: 'test-uri'} as DecorateRequestOptions;
1201-
sandbox
1202-
.stub(authClient, 'authorizeRequest')
1203-
.resolves(authorizedReqOpts);
1204-
sandbox.stub(authClient, 'getProjectId').resolves('test-project-id');
1205-
1206-
sandbox
1207-
.stub(util, 'makeRequest')
1208-
.callsFake((_authenticatedReqOpts, cfg, callback) => {
1209-
const mockRequestStream = new stream.Duplex({
1210-
write(_chunk, _encoding, callback) {
1211-
callback();
1212-
},
1213-
read() {},
1214-
}) as stream.Duplex & {abort: () => void};
1215-
mockRequestStream.abort = () => {};
1216-
1217-
if (!cfg.stream) {
1218-
const retryCallback = (
1219-
err: Error | null,
1220-
response: {},
1221-
body: unknown
1222-
) => {
1223-
if (err) {
1224-
const lowerCaseMessage = err.message.toLowerCase();
1225-
const isTLsTimeoutOrConnReset =
1226-
lowerCaseMessage.includes('tls handshake') ||
1227-
lowerCaseMessage.includes('timed out') ||
1228-
lowerCaseMessage.includes('etimedout') ||
1229-
lowerCaseMessage.includes('econnreset');
1230-
if (isTLsTimeoutOrConnReset) {
1231-
err = new Error(
1232-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1233-
);
1234-
}
1235-
}
1236-
util.handleResp(err, response as r.Response, body, callback!);
1237-
};
1238-
1239-
retryCallback(networkError, {} as r.Response, null);
1240-
}
1241-
1242-
return mockRequestStream;
1243-
});
1244-
}
12451193
const reqOpts = fakeReqOpts;
12461194
beforeEach(() => {
12471195
authClient.authorizeRequest = async () => reqOpts;
1248-
});
1249-
1250-
it('should transform raw ECONNRESET into TLS ApiError', done => {
1251-
const networkError = new Error('ECONNRESET');
1252-
sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);
1253-
createMakeRequestStub(sandbox, networkError, util, authClient);
1254-
const makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(
1255-
{}
1256-
);
1257-
1258-
makeAuthenticatedRequest({} as DecorateRequestOptions, err => {
1259-
assert.ok(err);
1260-
assert.strictEqual(
1261-
err.message,
1262-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1263-
);
1264-
done();
1265-
});
1266-
});
1267-
1268-
it('should transform raw "TLS handshake" into TLS ApiError', done => {
1269-
const networkError = new Error(
1270-
'Request failed due to TLS handshake timeout.'
1271-
);
12721196
sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);
1273-
createMakeRequestStub(sandbox, networkError, util, authClient);
1274-
const makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(
1275-
{}
1276-
);
1277-
1278-
makeAuthenticatedRequest({} as DecorateRequestOptions, err => {
1279-
assert.ok(err);
1280-
assert.strictEqual(
1281-
err.message,
1282-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1283-
);
1284-
done();
1285-
});
12861197
});
12871198

1288-
it('should transform raw generic "timed out" into TLS ApiError', done => {
1289-
const networkError = new Error('The request timed out.');
1290-
sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);
1291-
createMakeRequestStub(sandbox, networkError, util, authClient);
1292-
const makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(
1293-
{}
1294-
);
1295-
1296-
makeAuthenticatedRequest({} as DecorateRequestOptions, err => {
1297-
assert.ok(err);
1298-
assert.strictEqual(
1299-
err.message,
1300-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1301-
);
1302-
done();
1303-
});
1304-
});
1305-
1306-
it('should transform raw ETIMEDOUT into TLS ApiError', done => {
1307-
const networkError = new Error(
1308-
'Request failed with error: ETIMEDOUT'
1309-
);
1310-
sandbox.stub(fakeGoogleAuth, 'GoogleAuth').returns(authClient);
1311-
createMakeRequestStub(sandbox, networkError, util, authClient);
1312-
const makeAuthenticatedRequest = util.makeAuthenticatedRequestFactory(
1313-
{}
1314-
);
1315-
1316-
makeAuthenticatedRequest({} as DecorateRequestOptions, err => {
1317-
assert.ok(err);
1318-
assert.strictEqual(
1319-
err.message,
1320-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1321-
);
1322-
done();
1199+
const testCases = [
1200+
{name: 'ECONNRESET', error: new Error('ECONNRESET')},
1201+
{
1202+
name: '"TLS handshake"',
1203+
error: new Error('Request failed due to TLS handshake timeout.'),
1204+
},
1205+
{
1206+
name: 'generic "timed out"',
1207+
error: new Error('The request timed out.'),
1208+
},
1209+
{
1210+
name: 'ETIMEDOUT',
1211+
error: new Error('Request failed with error: ETIMEDOUT'),
1212+
},
1213+
];
1214+
1215+
testCases.forEach(({name, error: networkError}) => {
1216+
it(`should transform raw ${name} into specific TLS/CPU starvation Error`, done => {
1217+
// Override `retry-request` to simulate a network error.
1218+
retryRequestOverride = (
1219+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1220+
_reqOpts: any,
1221+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1222+
_opts: any,
1223+
callback: (err: Error, res: {}, body: null) => void
1224+
) => {
1225+
callback(networkError, {}, null);
1226+
return {abort: () => {}}; // Return an abortable request.
1227+
};
1228+
1229+
const makeAuthenticatedRequest =
1230+
util.makeAuthenticatedRequestFactory({});
1231+
1232+
makeAuthenticatedRequest({} as DecorateRequestOptions, err => {
1233+
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+
);
1238+
done();
1239+
});
13231240
});
13241241
});
13251242
});
@@ -1705,22 +1622,6 @@ describe('common/util', () => {
17051622
});
17061623

17071624
describe('callback mode', () => {
1708-
function testTlsErrorTransformation(
1709-
errorToMock: Error | NodeJS.ErrnoException
1710-
) {
1711-
const mockResponse: r.Response = {
1712-
headers: {},
1713-
} as r.Response;
1714-
1715-
return (
1716-
_rOpts: DecorateRequestOptions,
1717-
_opts: MakeRequestConfig,
1718-
callback: r.RequestCallback
1719-
) => {
1720-
callback(errorToMock, mockResponse, null);
1721-
};
1722-
}
1723-
17241625
it('should pass the default options to retryRequest', done => {
17251626
retryRequestOverride = testDefaultRetryRequestConfig(done);
17261627
util.makeRequest(
@@ -1798,87 +1699,6 @@ describe('common/util', () => {
17981699

17991700
util.makeRequest(fakeReqOpts, {}, assert.ifError);
18001701
});
1801-
1802-
it('should transform "tls handshake" into specific TLS/CPU starvation Error', done => {
1803-
const networkError = new Error('Request failed due to TLS handshake.');
1804-
1805-
// Stub handleResp to inspect the transformed error
1806-
stub('handleResp', err => {
1807-
assert.ok(err);
1808-
1809-
// Assert the error transformation occurred
1810-
assert.notStrictEqual(err, networkError);
1811-
assert.strictEqual(
1812-
err.message,
1813-
'Request or TLS handshake timed out. This may be due to CPU starvation or a temporary network issue.'
1814-
);
1815-
done();
1816-
});
1817-
1818-
retryRequestOverride = testTlsErrorTransformation(networkError);
1819-
util.makeRequest(reqOpts, {}, assert.ifError);
1820-
});
1821-
1822-
it('should transform "timed out" into specific TLS/CPU starvation Error', done => {
1823-
const networkError = new Error('Client request timed out.');
1824-
1825-
stub('handleResp', err => {
1826-
assert.ok(err);
1827-
assert.ok(err.message, 'TLS handshake timed out');
1828-
done();
1829-
});
1830-
1831-
retryRequestOverride = testTlsErrorTransformation(networkError);
1832-
util.makeRequest(reqOpts, {}, assert.ifError);
1833-
});
1834-
1835-
it('should transform "etimedout" into specific TLS/CPU starvation Error', done => {
1836-
const networkError: NodeJS.ErrnoException = new Error(
1837-
'connect ETIMEDOUT'
1838-
);
1839-
networkError.code = 'ETIMEDOUT';
1840-
1841-
stub('handleResp', err => {
1842-
assert.ok(err);
1843-
assert.ok(err.message, 'TLS handshake timed out');
1844-
done();
1845-
});
1846-
1847-
retryRequestOverride = testTlsErrorTransformation(networkError);
1848-
util.makeRequest(reqOpts, {}, assert.ifError);
1849-
});
1850-
1851-
it('should transform "econnreset" into specific TLS/CPU starvation Error', done => {
1852-
const networkError: NodeJS.ErrnoException = new Error(
1853-
'socket ECONNRESET'
1854-
);
1855-
networkError.code = 'ECONNRESET';
1856-
1857-
stub('handleResp', err => {
1858-
assert.ok(err);
1859-
assert.ok(err.message, 'TLS handshake timed out');
1860-
done();
1861-
});
1862-
1863-
retryRequestOverride = testTlsErrorTransformation(networkError);
1864-
util.makeRequest(reqOpts, {}, assert.ifError);
1865-
});
1866-
1867-
it('should NOT transform a regular non-TLS error', done => {
1868-
const networkError = new Error('Non-network API error.');
1869-
1870-
// Stub handleResp to check if the error is passed through unchanged
1871-
stub('handleResp', err => {
1872-
assert.ok(err);
1873-
// Assert the error was NOT transformed and is the original object
1874-
assert.strictEqual(err, networkError);
1875-
assert.doesNotMatch(err.message, /TLS handshake timed out/);
1876-
done();
1877-
});
1878-
1879-
retryRequestOverride = testTlsErrorTransformation(networkError);
1880-
util.makeRequest(reqOpts, {}, assert.ifError);
1881-
});
18821702
});
18831703
});
18841704

0 commit comments

Comments
 (0)