diff --git a/src/contentFetcher.ts b/src/contentFetcher.ts index a1377e1..9c37d0d 100644 --- a/src/contentFetcher.ts +++ b/src/contentFetcher.ts @@ -136,7 +136,7 @@ export class ContentFetcher { setTimeout( () => { const response: ErrorResponse = { - title: 'Maximum timeout reached before content could be loaded.', + title: `Content could not be loaded in time for slot '${slotId}'.`, type: ContentErrorType.TIMEOUT, detail: `The content took more than ${timeout}ms to load.`, status: 408, // Request Timeout @@ -157,7 +157,7 @@ export class ContentFetcher { const region = response.headers.get('X-Croct-Region'); const timing = response.headers.get('X-Croct-Timing'); - this.logger.debug(`Request processed by region ${region} in ${timing}`); + this.logger.debug(`Content for slot '${slotId}' processed by region ${region} in ${timing}.`); return response.json() .then(body => { diff --git a/src/evaluator.ts b/src/evaluator.ts index 38db40f..a980de5 100644 --- a/src/evaluator.ts +++ b/src/evaluator.ts @@ -137,13 +137,14 @@ export class Evaluator { public evaluate(query: string, options: EvaluationOptions = {}): Promise { const length = getLength(query); + const reference = query.length > 20 ? `${query.slice(0, 20)}...` : query; if (length > Evaluator.MAX_QUERY_LENGTH) { const response: QueryErrorResponse = { title: 'The query is too complex.', status: 422, // Unprocessable Entity type: EvaluationErrorType.TOO_COMPLEX_QUERY, - detail: `The query must be at most ${Evaluator.MAX_QUERY_LENGTH} characters long, ` + detail: `The query "${reference}" must be at most ${Evaluator.MAX_QUERY_LENGTH} characters long, ` + `but it is ${length} characters long.`, errors: [{ cause: 'The query is longer than expected.', @@ -170,7 +171,7 @@ export class Evaluator { setTimeout( () => { const response: ErrorResponse = { - title: 'Maximum evaluation timeout reached before evaluation could complete.', + title: `Evaluation could not be completed in time for query "${reference}".`, type: EvaluationErrorType.TIMEOUT, detail: `The evaluation took more than ${timeout}ms to complete.`, status: 408, // Request Timeout @@ -193,7 +194,9 @@ export class Evaluator { const region = response.headers.get('X-Croct-Region'); const timing = response.headers.get('X-Croct-Timing'); - this.logger.debug(`Request processed by region ${region} in ${timing}`); + this.logger.debug( + `Evaluation of the query "${reference}" processed by region ${region} in ${timing}.`, + ); return response.json() .then(body => { diff --git a/test/contentFetcher.test.ts b/test/contentFetcher.test.ts index 75960d1..254d293 100644 --- a/test/contentFetcher.test.ts +++ b/test/contentFetcher.test.ts @@ -26,7 +26,7 @@ describe('A content fetcher', () => { ); const plainTextApiKey = parsedApiKey.getIdentifier(); - const contentId = 'hero-banner'; + const slotId = 'hero-banner'; const content = { content: { title: 'Hello World', @@ -41,7 +41,7 @@ describe('A content fetcher', () => { 'X-Client-Library': CLIENT_LIBRARY, }, body: { - slotId: contentId, + slotId: slotId, }, }; @@ -75,7 +75,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId)).resolves.toEqual(content); }); it.each<[string, string|ApiKey]>([ @@ -100,7 +100,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should require an API key to fetch static content', async () => { @@ -108,7 +108,7 @@ describe('A content fetcher', () => { appId: appId, }); - await expect(() => fetcher.fetch(contentId, {static: true})) + await expect(() => fetcher.fetch(slotId, {static: true})) .toThrowWithMessage(Error, 'The API key must be provided to fetch static content.'); }); @@ -127,7 +127,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId)).resolves.toEqual(content); }); it('should fetch static content for the specified slot version', async () => { @@ -154,7 +154,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch static content for the specified preferred locale', async () => { @@ -181,7 +181,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch dynamic content using the provided client ID', async () => { @@ -204,7 +204,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch dynamic content passing the provided client IP', async () => { @@ -227,7 +227,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch dynamic content passing the provided client agent', async () => { @@ -250,7 +250,7 @@ describe('A content fetcher', () => { response: content, }); - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch dynamic content using the provided user token', async () => { @@ -273,7 +273,7 @@ describe('A content fetcher', () => { userToken: token, }; - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch dynamic content using the provided preview token', async () => { @@ -296,7 +296,7 @@ describe('A content fetcher', () => { previewToken: token, }; - await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content); + await expect(fetcher.fetch(slotId, options)).resolves.toEqual(content); }); it('should fetch using the extra options', async () => { @@ -329,7 +329,7 @@ describe('A content fetcher', () => { ...nonOverridableOptions, }; - await fetcher.fetch(contentId, {extra: extraOptions as FetchOptions['extra']}); + await fetcher.fetch(slotId, {extra: extraOptions as FetchOptions['extra']}); expect(fetchMock.lastOptions()).toEqual(expect.objectContaining(overridableOptions)); expect(fetchMock.lastOptions()).not.toEqual(expect.objectContaining(nonOverridableOptions)); @@ -360,7 +360,7 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId, { + const promise = fetcher.fetch(slotId, { timeout: 10, }); @@ -373,7 +373,7 @@ describe('A content fetcher', () => { await expect(promise).rejects.toThrow(ContentError); await expect(promise).rejects.toHaveProperty('response', { - title: 'Maximum timeout reached before content could be loaded.', + title: `Content could not be loaded in time for slot '${slotId}'.`, type: ContentErrorType.TIMEOUT, detail: 'The content took more than 10ms to load.', status: 408, @@ -400,14 +400,14 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); jest.advanceTimersByTime(11); await expect(promise).rejects.toThrow(ContentError); await expect(promise).rejects.toHaveProperty('response', { - title: 'Maximum timeout reached before content could be loaded.', + title: `Content could not be loaded in time for slot '${slotId}'.`, type: ContentErrorType.TIMEOUT, detail: 'The content took more than 10ms to load.', status: 408, @@ -447,7 +447,7 @@ describe('A content fetcher', () => { response: content, }); - const promise = fetcher.fetch(contentId, {context: context}); + const promise = fetcher.fetch(slotId, {context: context}); await expect(promise).resolves.toEqual(content); }); @@ -468,7 +468,7 @@ describe('A content fetcher', () => { response: content, }); - const promise = fetcher.fetch(contentId, {version: version}); + const promise = fetcher.fetch(slotId, {version: version}); await expect(promise).resolves.toEqual(content); }); @@ -488,7 +488,7 @@ describe('A content fetcher', () => { response: content, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); await expect(promise).resolves.toEqual(content); }); @@ -509,7 +509,7 @@ describe('A content fetcher', () => { response: content, }); - const promise = fetcher.fetch(contentId, {preferredLocale: preferredLocale}); + const promise = fetcher.fetch(slotId, {preferredLocale: preferredLocale}); await expect(promise).resolves.toEqual(content); }); @@ -533,7 +533,7 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); await expect(promise).rejects.toThrow(ContentError); await expect(promise).rejects.toHaveProperty('response', response); @@ -559,7 +559,7 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); await expect(promise).rejects.toThrow(ContentError); await expect(promise).rejects.toHaveProperty('response', response); @@ -585,7 +585,7 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); await expect(promise).rejects.toThrow(ContentError); await expect(promise.catch((error: ContentError) => error.response)).resolves.toEqual(response); @@ -610,7 +610,7 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); await expect(promise).rejects.toThrow(ContentError); await expect(promise).rejects.toHaveProperty('response', response); @@ -659,7 +659,7 @@ describe('A content fetcher', () => { }, }); - const promise = fetcher.fetch(contentId); + const promise = fetcher.fetch(slotId); await expect(promise).rejects.toThrowWithMessage(ContentError, scenario.title); @@ -682,21 +682,26 @@ describe('A content fetcher', () => { logger: logger, }); + const region = 'us-central1'; + const processingTime = 120.1234; + fetchMock.mock({ ...requestMatcher, response: { status: 200, body: content, headers: { - 'X-Croct-Region': 'us-central1', - 'X-Croct-Timing': '120.1234ms', + 'X-Croct-Region': region, + 'X-Croct-Timing': processingTime, }, }, }); - await fetcher.fetch(contentId); + await fetcher.fetch(slotId); - expect(logger.debug).toHaveBeenCalledWith('Request processed by region us-central1 in 120.1234ms'); + expect(logger.debug).toHaveBeenCalledWith( + `Content for slot '${slotId}' processed by region ${region} in ${processingTime}.`, + ); }); it('should not be serializable', () => { diff --git a/test/evaluator.test.ts b/test/evaluator.test.ts index b1c12d1..7c33362 100644 --- a/test/evaluator.test.ts +++ b/test/evaluator.test.ts @@ -294,7 +294,7 @@ describe('An evaluator', () => { await expect(promise).rejects.toThrow(EvaluationError); await expect(promise).rejects.toHaveProperty('response', { - title: 'Maximum evaluation timeout reached before evaluation could complete.', + title: 'Evaluation could not be completed in time for query "user\'s name".', type: EvaluationErrorType.TIMEOUT, detail: 'The evaluation took more than 10ms to complete.', status: 408, @@ -326,7 +326,7 @@ describe('An evaluator', () => { await expect(promise).rejects.toThrow(EvaluationError); await expect(promise).rejects.toHaveProperty('response', { - title: 'Maximum evaluation timeout reached before evaluation could complete.', + title: 'Evaluation could not be completed in time for query "user\'s name".', type: EvaluationErrorType.TIMEOUT, detail: 'The evaluation took more than 10ms to complete.', status: 408, @@ -455,7 +455,7 @@ describe('An evaluator', () => { title: 'The query is too complex.', status: 422, type: EvaluationErrorType.TOO_COMPLEX_QUERY, - detail: `The query must be at most ${Evaluator.MAX_QUERY_LENGTH} ` + detail: `The query "____________________..." must be at most ${Evaluator.MAX_QUERY_LENGTH} ` + `characters long, but it is ${length} characters long.`, errors: [{ cause: 'The query is longer than expected.', @@ -627,6 +627,8 @@ describe('An evaluator', () => { }); const result = true; + const region = 'us-central1'; + const timing = 120.1234; fetchMock.mock({ ...requestMatcher, @@ -642,7 +644,9 @@ describe('An evaluator', () => { await evaluator.evaluate(query); - expect(logger.debug).toHaveBeenCalledWith('Request processed by region us-central1 in 120.1234ms'); + expect(logger.debug).toHaveBeenCalledWith( + `Evaluation of the query "${query}" processed by region ${region} in ${timing}ms.`, + ); }); it('should not be serializable', () => { diff --git a/test/sdk.test.ts b/test/sdk.test.ts index 037fcfa..10bda9c 100644 --- a/test/sdk.test.ts +++ b/test/sdk.test.ts @@ -459,7 +459,7 @@ describe('A SDK', () => { it('should configure the evaluator with the specified default timeout', async () => { jest.useFakeTimers(); - const query = '1 + 2'; + const query = '1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10'; const result = 3; fetchMock.mock({ @@ -486,7 +486,7 @@ describe('A SDK', () => { await expect(promise).rejects.toThrowWithMessage( EvaluationError, - 'Maximum evaluation timeout reached before evaluation could complete.', + 'Evaluation could not be completed in time for query "1 + 2 + 3 + 4 + 5 + ...".', ); await expect(promise).rejects.toHaveProperty('response', expect.objectContaining({ @@ -497,7 +497,7 @@ describe('A SDK', () => { it('should configure the evaluator with the default timeout', async () => { jest.useFakeTimers(); - const query = '1 + 2'; + const query = '1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10'; const result = 3; fetchMock.mock({ @@ -523,7 +523,7 @@ describe('A SDK', () => { await expect(promise).rejects.toThrowWithMessage( EvaluationError, - 'Maximum evaluation timeout reached before evaluation could complete.', + 'Evaluation could not be completed in time for query "1 + 2 + 3 + 4 + 5 + ...".', ); await expect(promise).rejects.toHaveProperty('response', expect.objectContaining({ @@ -596,7 +596,7 @@ describe('A SDK', () => { await expect(promise).rejects.toThrowWithMessage( ContentError, - 'Maximum timeout reached before content could be loaded.', + `Content could not be loaded in time for slot '${slotId}'.`, ); await expect(promise).rejects.toHaveProperty('response', expect.objectContaining({ @@ -637,7 +637,7 @@ describe('A SDK', () => { await expect(promise).rejects.toThrowWithMessage( ContentError, - 'Maximum timeout reached before content could be loaded.', + `Content could not be loaded in time for slot '${slotId}'.`, ); await expect(promise).rejects.toHaveProperty('response', expect.objectContaining({