Skip to content

Commit

Permalink
Merge pull request #431 from podium-lib/cherry_pick_timeout_fix
Browse files Browse the repository at this point in the history
Cherry pick timeout fix
  • Loading branch information
digitalsadhu authored Oct 17, 2024
2 parents 1045c21 + ebdfc74 commit cc7e71e
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 8 deletions.
30 changes: 23 additions & 7 deletions lib/http.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { request } from 'undici';
import { request as undiciRequest } from 'undici';

/**
* @typedef {object} PodiumHttpClientRequestOptions
Expand All @@ -7,22 +7,38 @@ import { request } from 'undici';
* @property {boolean} [rejectUnauthorized]
* @property {boolean} [follow]
* @property {number} [timeout]
* @property {number} [bodyTimeout]
* @property {object} [query]
* @property {import('http').IncomingHttpHeaders} [headers]
*/

export default class HTTP {
constructor(requestFn = undiciRequest) {
this.requestFn = requestFn;
}

/**
* @param {string} url
* @param {PodiumHttpClientRequestOptions} options
* @returns {Promise<Pick<import('undici').Dispatcher.ResponseData, 'statusCode' | 'headers' | 'body'>>}
*/
async request(url, options) {
const { statusCode, headers, body } = await request(
new URL(url),
options,
);
return { statusCode, headers, body };
const abortController = new AbortController();

const timeoutId = setTimeout(() => {
abortController.abort();
}, options.timeout || 1000);

try {
const { statusCode, headers, body } = await this.requestFn(
new URL(url),
{
...options,
signal: abortController.signal,
},
);
return { statusCode, headers, body };
} finally {
clearTimeout(timeoutId);
}
}
}
3 changes: 2 additions & 1 deletion lib/resolver.content.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export default class PodletClientContentResolver {
/** @type {import('./http.js').PodiumHttpClientRequestOptions} */
const reqOptions = {
rejectUnauthorized: outgoing.rejectUnauthorized,
bodyTimeout: outgoing.timeout,
timeout: outgoing.timeout,
method: 'GET',
query: outgoing.reqOptions.query,
headers,
Expand Down Expand Up @@ -267,6 +267,7 @@ export default class PodletClientContentResolver {
}),
);

// @ts-ignore
pipeline([body, outgoing], (err) => {
if (err) {
this.#log.warn('error while piping content stream', err);
Expand Down
44 changes: 44 additions & 0 deletions tests/http.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { test } from 'node:test';
import { rejects } from 'node:assert';
import HTTP from '../lib/http.js';

test('should abort the request if it takes longer than the timeout', async () => {
// Mock the undici.Client's request method
const mockRequestFn = async (url, { signal }) => {
return new Promise((resolve, reject) => {
// Simulate a delay longer than the timeout
setTimeout(() => {
if (signal.aborted) {
const abortError = new Error(
'Request aborted due to timeout',
);
abortError.name = 'AbortError';
reject(abortError);
} else {
resolve({
statusCode: 200,
headers: {},
body: 'OK',
});
}
}, 2000); // 2 seconds delay
});
};

// @ts-ignore
const http = new HTTP(mockRequestFn);
const url = 'https://example.com/test';
const options = {
method: /** @type {'GET'} */ ('GET'),
timeout: 1000, // 1 second timeout
};

// Assert that the request is rejected with an AbortError
await rejects(
http.request(url, options),
(/** @type {Error} */ err) =>
err.name === 'AbortError' &&
err.message === 'Request aborted due to timeout',
'Expected request to be aborted due to timeout',
);
});

0 comments on commit cc7e71e

Please sign in to comment.