Skip to content

Commit

Permalink
Fix timeout logging on requests that didn't timeout
Browse files Browse the repository at this point in the history
  • Loading branch information
Fryuni committed Sep 11, 2024
1 parent 8c7f610 commit 89871d4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
14 changes: 11 additions & 3 deletions src/contentFetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,15 @@ export class ContentFetcher {
throw new Error('The API key must be provided to fetch static content.');
}

return new Promise((resolve, reject) => {
// Types for Browser and Node environment
let timeoutReference: number | NodeJS.Timeout | undefined;

return new Promise<FetchResponse<P>>((resolve, reject) => {
const abortController = new AbortController();
const timeout = options.timeout ?? this.configuration.defaultTimeout;

if (timeout !== undefined) {
setTimeout(
timeoutReference = setTimeout(
() => {
const response: ErrorResponse = {
title: `Content could not be loaded in time for slot '${slotId}'.`,
Expand Down Expand Up @@ -189,7 +192,12 @@ export class ContentFetcher {
);
}
});
});
})
.finally(() => {
// Once the fetch completes, regardless of outcome, cancel the timeout
// to avoid logging an error that didn't happen.
clearTimeout(timeoutReference);
});
}

private load(slotId: string, signal: AbortSignal, options: FetchOptions): Promise<Response> {
Expand Down
30 changes: 30 additions & 0 deletions test/contentFetcher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,36 @@ describe('A content fetcher', () => {
});
});

it('should not log a timeout error message when request completes before the timeout', async () => {
jest.useFakeTimers();

const logger: Logger = {
debug: jest.fn(),
info: jest.fn(),
warn: jest.fn(),
error: jest.fn(),
};

const fetcher = new ContentFetcher({
appId: appId,
logger: logger,
defaultTimeout: 10,
});

fetchMock.mock({
...requestMatcher,
response: {
result: 'Carol',
},
});

await fetcher.fetch(slotId);

jest.advanceTimersByTime(11);

expect(logger.error).not.toHaveBeenCalled();
});

it('should fetch dynamic content using the provided context', async () => {
const fetcher = new ContentFetcher({
appId: appId,
Expand Down

0 comments on commit 89871d4

Please sign in to comment.