Skip to content

Commit

Permalink
Fix fetch options and improve error reporting (#305)
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcospassos
marcospassos authored Jan 6, 2023
1 parent 2f8336a commit e03dfd9
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 25 deletions.
31 changes: 19 additions & 12 deletions src/contentFetcher.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ export class ContentFetcher {
} else {
reject(new ContentError(body));
}
})
.catch(error => {
if (!response.ok) {
throw new Error(`Error ${response.status} - ${response.statusText}`);
}

throw error;
}),
)
.catch(error => {
Expand All @@ -153,10 +160,6 @@ export class ContentFetcher {
}

private load(slotId: string, signal: AbortSignal, options: FetchOptions): Promise<Response> {
const payload: FetchPayload = {
slotId: slotId,
};

const {apiKey, appId} = this.configuration;

const headers: Record<string, string> = {
Expand All @@ -173,6 +176,18 @@ export class ContentFetcher {
headers['X-Api-Key'] = apiKey;
}

const payload: FetchPayload = {
slotId: slotId,
};

if (options.version !== undefined) {
payload.version = `${options.version}`;
}

if (options.preferredLocale !== undefined) {
payload.preferredLocale = options.preferredLocale;
}

const dynamic = ContentFetcher.isDynamicContent(options);

if (dynamic) {
Expand All @@ -192,14 +207,6 @@ export class ContentFetcher {
headers['User-Agent'] = options.userAgent;
}

if (options.version !== undefined) {
payload.version = `${options.version}`;
}

if (options.preferredLocale !== undefined) {
payload.preferredLocale = options.preferredLocale;
}

if (options.context !== undefined) {
payload.context = options.context;
}
Expand Down
7 changes: 7 additions & 0 deletions src/evaluator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,13 @@ export class Evaluator {

break;
}
})
.catch(error => {
if (!response.ok) {
throw new Error(`Error ${response.status} - ${response.statusText}`);
}

throw error;
}),
)
.catch(
Expand Down
77 changes: 66 additions & 11 deletions test/contentFetcher.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe('A content fetcher', () => {
await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should requite an API key to fetch static content', async () => {
it('should require an API key to fetch static content', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand All @@ -117,7 +117,61 @@ describe('A content fetcher', () => {
await expect(fetcher.fetch(contentId)).resolves.toEqual(content);
});

it('should fetch content using the provided client ID', async () => {
it('should fetch static content for the specified slot version', async () => {
const fetcher = new ContentFetcher({
apiKey: apiKey,
});

const options: FetchOptions = {
static: true,
version: 2,
};

fetchMock.mock({
...requestMatcher,
matcher: `${BASE_ENDPOINT_URL}/external/web/static-content`,
headers: {
...requestMatcher.headers,
'X-Api-Key': apiKey,
},
body: {
...requestMatcher.body,
version: `${options.version}`,
},
response: content,
});

await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should fetch static content for the specified preferred locale', async () => {
const fetcher = new ContentFetcher({
apiKey: apiKey,
});

const options: FetchOptions = {
static: true,
preferredLocale: 'en-US',
};

fetchMock.mock({
...requestMatcher,
matcher: `${BASE_ENDPOINT_URL}/external/web/static-content`,
headers: {
...requestMatcher.headers,
'X-Api-Key': apiKey,
},
body: {
...requestMatcher.body,
preferredLocale: options.preferredLocale,
},
response: content,
});

await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should fetch dynamic content using the provided client ID', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand All @@ -140,7 +194,7 @@ describe('A content fetcher', () => {
await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should fetch content passing the provided client IP', async () => {
it('should fetch dynamic content passing the provided client IP', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand All @@ -163,7 +217,7 @@ describe('A content fetcher', () => {
await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should fetch content passing the provided user agent', async () => {
it('should fetch dynamic content passing the provided user agent', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand All @@ -186,7 +240,7 @@ describe('A content fetcher', () => {
await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should fetch content using the provided user token', async () => {
it('should fetch dynamic content using the provided user token', async () => {
const token = Token.issue(appId, 'foo', Date.now());

const fetcher = new ContentFetcher({
Expand All @@ -209,7 +263,7 @@ describe('A content fetcher', () => {
await expect(fetcher.fetch(contentId, options)).resolves.toEqual(content);
});

it('should fetch content using the provided preview token', async () => {
it('should fetch dynamic content using the provided preview token', async () => {
const token = Token.issue(appId, 'foo', Date.now());

const fetcher = new ContentFetcher({
Expand Down Expand Up @@ -303,7 +357,7 @@ describe('A content fetcher', () => {
expect(fetchOptions?.signal.aborted).toBe(true);
});

it('should fetch content using the provided context', async () => {
it('should fetch dynamic content using the provided context', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand Down Expand Up @@ -341,7 +395,7 @@ describe('A content fetcher', () => {
await expect(promise).resolves.toEqual(content);
});

it('should fetch content for the specified slot version', async () => {
it('should fetch dynamic content for the specified slot version', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand All @@ -362,7 +416,7 @@ describe('A content fetcher', () => {
await expect(promise).resolves.toEqual(content);
});

it('should fetch content for the specified preferred locale', async () => {
it('should fetch dynamic content for the specified preferred locale', async () => {
const fetcher = new ContentFetcher({
appId: appId,
});
Expand Down Expand Up @@ -414,7 +468,7 @@ describe('A content fetcher', () => {
});

const response: ErrorResponse = {
title: expect.stringMatching('Invalid json response body'),
title: 'Error 500 - Internal Server Error',
type: ContentErrorType.UNEXPECTED_ERROR,
detail: 'Please try again or contact Croct support if the error persists.',
status: 500,
Expand All @@ -423,7 +477,8 @@ describe('A content fetcher', () => {
fetchMock.mock({
...requestMatcher,
response: {
body: 'non-json',
status: 500,
body: 'Invalid JSON payload',
},
});

Expand Down
5 changes: 3 additions & 2 deletions test/evaluator.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ describe('An evaluator', () => {
});

const response: ErrorResponse = {
title: expect.stringMatching('Invalid json response body'),
title: 'Error 500 - Internal Server Error',
type: EvaluationErrorType.UNEXPECTED_ERROR,
detail: 'Please try again or contact Croct support if the error persists.',
status: 500,
Expand All @@ -439,7 +439,8 @@ describe('An evaluator', () => {
fetchMock.mock({
...requestMatcher,
response: {
body: 'non-json',
status: 500,
body: 'Invalid JSON payload',
},
});

Expand Down

0 comments on commit e03dfd9

Please sign in to comment.