Merge branch 'master' into preserve-token

croct-tech · Oct 7, 2024 · 5233dd8 · 5233dd8
2 parents 39e536e + 6ceb408
commit 5233dd8
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 71 deletions.
diff --git a/src/contentFetcher.ts b/src/contentFetcher.ts
@@ -128,15 +128,15 @@ export class ContentFetcher {
             throw new Error('The API key must be provided to fetch static content.');
         }
 
-        // 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;
 
+            let timer: number | NodeJS.Timeout | undefined;
+
             if (timeout !== undefined) {
-                timeoutReference = setTimeout(
+                timer = setTimeout(
                     () => {
                         const response: ErrorResponse = {
                             title: `Content could not be loaded in time for slot '${slotId}'.`,
@@ -156,6 +156,7 @@ export class ContentFetcher {
             }
 
             this.load(slotId, abortController.signal, options)
+                .finally(() => clearTimeout(timer))
                 .then(response => {
                     const region = response.headers.get('X-Croct-Region');
                     const timing = response.headers.get('X-Croct-Timing');
@@ -192,12 +193,7 @@ 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> {

diff --git a/src/evaluator.ts b/src/evaluator.ts
@@ -167,6 +167,8 @@ export class Evaluator {
             const abortController = new AbortController();
             const timeout = options.timeout ?? this.configuration.defaultTimeout;
 
+            let timer: number | NodeJS.Timeout | undefined;
+
             if (timeout !== undefined) {
                 setTimeout(
                     () => {
@@ -187,63 +189,64 @@ export class Evaluator {
                 );
             }
 
-            const promise = this.fetch(payload, abortController.signal, options);
-
-            promise.then(
-                response => {
-                    const region = response.headers.get('X-Croct-Region');
-                    const timing = response.headers.get('X-Croct-Timing');
-
-                    this.logger.debug(
-                        `Evaluation of the query "${reference}" processed by region ${region} in ${timing}.`,
-                    );
-
-                    return response.json()
-                        .then(body => {
-                            if (response.ok) {
-                                return resolve(body);
-                            }
-
-                            this.logHelp(response.status);
-
-                            const problem: ErrorResponse = body;
-
-                            switch (problem.type) {
-                                case EvaluationErrorType.INVALID_QUERY:
-                                case EvaluationErrorType.EVALUATION_FAILED:
-                                case EvaluationErrorType.TOO_COMPLEX_QUERY:
-                                    reject(new QueryError(problem as QueryErrorResponse));
-
-                                    break;
-
-                                default:
-                                    reject(new EvaluationError(problem));
-
-                                    break;
-                            }
-                        })
-                        .catch(error => {
-                            if (!response.ok) {
-                                throw new Error(`Error ${response.status} - ${response.statusText}`);
-                            }
-
-                            throw error;
-                        });
-                },
-            ).catch(
-                error => {
-                    if (!abortController.signal.aborted) {
-                        reject(
-                            new EvaluationError({
-                                title: formatMessage(error),
-                                type: EvaluationErrorType.UNEXPECTED_ERROR,
-                                detail: 'Please try again or contact Croct support if the error persists.',
-                                status: 500, // Internal Server Error
-                            }),
+            this.fetch(payload, abortController.signal, options)
+                .finally(() => clearTimeout(timer))
+                .then(
+                    response => {
+                        const region = response.headers.get('X-Croct-Region');
+                        const timing = response.headers.get('X-Croct-Timing');
+
+                        this.logger.debug(
+                            `Evaluation of the query "${reference}" processed by region ${region} in ${timing}.`,
                         );
-                    }
-                },
-            );
+
+                        return response.json()
+                            .then(body => {
+                                if (response.ok) {
+                                    return resolve(body);
+                                }
+
+                                this.logHelp(response.status);
+
+                                const problem: ErrorResponse = body;
+
+                                switch (problem.type) {
+                                    case EvaluationErrorType.INVALID_QUERY:
+                                    case EvaluationErrorType.EVALUATION_FAILED:
+                                    case EvaluationErrorType.TOO_COMPLEX_QUERY:
+                                        reject(new QueryError(problem as QueryErrorResponse));
+
+                                        break;
+
+                                    default:
+                                        reject(new EvaluationError(problem));
+
+                                        break;
+                                }
+                            })
+                            .catch(error => {
+                                if (!response.ok) {
+                                    throw new Error(`Error ${response.status} - ${response.statusText}`);
+                                }
+
+                                throw error;
+                            });
+                    },
+                )
+                .catch(
+                    error => {
+                        if (!abortController.signal.aborted) {
+                            reject(
+                                new EvaluationError({
+                                    title: formatMessage(error),
+                                    type: EvaluationErrorType.UNEXPECTED_ERROR,
+                                    detail: 'Please try again or contact Croct support if the error persists.',
+                                    status: 500, // Internal Server Error
+                                }),
+                            );
+                        }
+                    },
+                );
         });
     }
 

diff --git a/test/evaluator.test.ts b/test/evaluator.test.ts
@@ -277,9 +277,7 @@ describe('An evaluator', () => {
         fetchMock.mock({
             ...requestMatcher,
             delay: 20,
-            response: {
-                result: 'Carol',
-            },
+            response: JSON.stringify('Carol'),
         });
 
         const promise = evaluator.evaluate(query, {
@@ -315,9 +313,7 @@ describe('An evaluator', () => {
         fetchMock.mock({
             ...requestMatcher,
             delay: 20,
-            response: {
-                result: 'Carol',
-            },
+            response: JSON.stringify('Carol'),
         });
 
         const promise = evaluator.evaluate(query);
@@ -333,6 +329,31 @@ describe('An evaluator', () => {
         });
     });
 
+    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 evaluator = new Evaluator({
+            appId: appId,
+            logger: logger,
+        });
+
+        fetchMock.mock({
+            ...requestMatcher,
+            response: JSON.stringify('Carol'),
+        });
+
+        await expect(evaluator.evaluate(query, {timeout: 10})).resolves.toBe('Carol');
+
+        expect(logger.error).not.toHaveBeenCalled();
+    });
+
     it('should evaluate queries using the provided context', async () => {
         const evaluator = new Evaluator({
             appId: appId,