Skip to content

Commit

Permalink
add test, fix comment
Browse files Browse the repository at this point in the history
  • Loading branch information
zhukaihan committed Aug 9, 2024
1 parent d8740c7 commit e5c6e53
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
10 changes: 4 additions & 6 deletions packages/node/src/local/cohort/cohort-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,9 @@ export interface CohortApi {
getCohort(options?: GetCohortOptions): Promise<Cohort>;
}

export class CohortMaxSizeExceededError extends Error {}

export class CohortClientRequestError extends Error {}

export class CohortDownloadError extends Error {}
export class CohortClientRequestError extends Error {} // 4xx errors except 429
export class CohortMaxSizeExceededError extends CohortClientRequestError {} // 413 error
export class CohortDownloadError extends Error {} // All other errors

export class SdkCohortApi implements CohortApi {
private readonly cohortApiKey;
Expand Down Expand Up @@ -86,7 +84,7 @@ export class SdkCohortApi implements CohortApi {
response.status < 500 &&
response.status != 429
) {
// Any 400 other than 429.
// Any 4xx other than 429.
throw new CohortClientRequestError(
`Cohort client error response status ${response.status}, body ${response.body}`,
);
Expand Down
24 changes: 23 additions & 1 deletion packages/node/test/local/cohort/cohortFetcher.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {
CohortClientRequestError,
CohortMaxSizeExceededError,
SdkCohortApi,
} from 'src/local/cohort/cohort-api';
Expand Down Expand Up @@ -38,7 +39,7 @@ const COHORTS = {
},
};

beforeEach(() => {
afterEach(() => {
jest.clearAllMocks();
});

Expand Down Expand Up @@ -155,6 +156,27 @@ test('cohort fetch maxSize exceeded, no retry', async () => {
});
});

test('cohort fetch client error, no retry', async () => {
const cohortApiGetCohortSpy = jest.spyOn(SdkCohortApi.prototype, 'getCohort');
cohortApiGetCohortSpy.mockImplementation(async () => {
throw new CohortClientRequestError();
});

const cohortFetcher = new CohortFetcher('', '', null, 'someurl', 10, 100);
const cohortPromise = cohortFetcher.fetch('c1', 10);

await expect(cohortPromise).rejects.toThrowError();
expect(cohortApiGetCohortSpy).toHaveBeenCalledTimes(1);
expect(cohortApiGetCohortSpy).toHaveBeenCalledWith({
cohortId: 'c1',
lastModified: 10,
libraryName: 'experiment-node-server',
libraryVersion: PACKAGE_VERSION,
maxCohortSize: 10,
timeoutMillis: COHORT_CONFIG_TIMEOUT,
});
});

test('cohort fetch twice on same cohortId uses same promise and make only one request', async () => {
const cohortApiGetCohortSpy = jest.spyOn(SdkCohortApi.prototype, 'getCohort');
cohortApiGetCohortSpy.mockImplementation(async (options) => {
Expand Down

0 comments on commit e5c6e53

Please sign in to comment.