Skip to content

Commit

Permalink
[tweek-client] add client error handling (#147)
Browse files Browse the repository at this point in the history
* add tweek client with fallback

* add deprecated decorator to fetch

use getValue instead

* add TweekClientWithFallback tests

* use $includeErrors

* add option to throw on key errors

* bump version

* fix repo tests

* call onKeyError for each failed key

* cr
  • Loading branch information
nataly87s authored Apr 16, 2019
1 parent 7055440 commit 4fe6be3
Show file tree
Hide file tree
Showing 20 changed files with 403 additions and 60 deletions.
2 changes: 1 addition & 1 deletion js/tweek-client/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "tweek-client",
"version": "1.0.0-rc7",
"version": "1.0.0",
"description": "Tweek client for JavaScript",
"author": "Soluto",
"license": "MIT",
Expand Down
14 changes: 14 additions & 0 deletions js/tweek-client/src/TweekClient/KeyValuesError.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { KeyValuesErrors } from './types';

export class KeyValuesError extends Error {
constructor(keyValuesErrors: KeyValuesErrors, message: string);
constructor(public keyValuesErrors: KeyValuesErrors, ...args: any[]) {
super(...args);

Object.setPrototypeOf(this, KeyValuesError.prototype);

if (Error.captureStackTrace) {
Error.captureStackTrace(this, KeyValuesError);
}
}
}
56 changes: 48 additions & 8 deletions js/tweek-client/src/TweekClient/TweekClient.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,38 @@
import { InputParams } from 'query-string';
import chunk from 'lodash.chunk';
import { normalizeBaseUrl, optimizeInclude, toQueryString } from '../utils';
import { deprecated, normalizeBaseUrl, optimizeInclude, toQueryString } from '../utils';
import { TweekInitConfig } from '../types';
import { FetchError } from '../FetchError';
import { Context, GetValuesConfig, ITweekClient, TweekClientConfig } from './types';
import {
Context,
GetValuesConfig,
ITweekClient,
KeyValuesErrorHandler,
KeyValuesErrors,
TweekClientConfig,
} from './types';
import { KeyValuesError } from './KeyValuesError';

type TweekResult<T> = {
data: T;
errors: KeyValuesErrors;
};

function extractData<T>(
{ data, errors }: TweekResult<T>,
throwOnError: boolean | undefined,
onKeyValueError: KeyValuesErrorHandler | undefined,
) {
if (errors && Object.keys(errors).length > 0) {
if (onKeyValueError) {
Object.entries(errors as KeyValuesErrors).forEach(([k, e]) => onKeyValueError(k, e));
}
if (throwOnError) {
throw new KeyValuesError(errors, 'Tweek values had errors');
}
}
return data;
}

export default class TweekClient implements ITweekClient {
config: TweekClientConfig;
Expand All @@ -25,26 +54,37 @@ export default class TweekClient implements ITweekClient {
..._config,
};

const { include, maxChunkSize = 100 } = cfg;
const { include, maxChunkSize = 100, throwOnError, onKeyValueError } = cfg;

if (!include) {
return this._fetchChunk(path, cfg);
return this._fetchChunk<T>(path, cfg).then(res => extractData(res, throwOnError, onKeyValueError));
}

const optimizedInclude = optimizeInclude(include);
const includeChunks = chunk(optimizedInclude, maxChunkSize);
const fetchConfigChunks = includeChunks.map(ic => ({ ...cfg, include: ic }));
const fetchPromises = fetchConfigChunks.map(cc => this._fetchChunk(path, cc));
return <Promise<T>>Promise.all(fetchPromises).then(chunks => chunks.reduce((res, ch) => ({ ...res, ...ch }), {}));
const fetchPromises = fetchConfigChunks.map(cc => this._fetchChunk<T>(path, cc));
return Promise.all(fetchPromises).then(chunks => {
const res = chunks.reduce((res, ch) => ({
data: { ...res.data, ...ch.data },
errors: { ...res.errors, ...ch.errors },
}));
return extractData(res, throwOnError, onKeyValueError);
});
}

fetch = this.getValues;
@deprecated('getValues')
fetch<T>(path: string, config?: GetValuesConfig): Promise<T> {
return this.getValues(path, config);
}

private _fetchChunk<T>(path: string, _config: TweekInitConfig & GetValuesConfig): Promise<T> {
private _fetchChunk<T>(path: string, _config: TweekInitConfig & GetValuesConfig): Promise<TweekResult<T>> {
const { flatten, baseServiceUrl, context, include, ignoreKeyTypes } = _config;

const queryParamsObject = this._contextToQueryParams(context);

queryParamsObject['$includeErrors'] = true;

if (flatten) {
queryParamsObject['$flatten'] = true;
}
Expand Down
19 changes: 19 additions & 0 deletions js/tweek-client/src/TweekClient/TweekClientWithFallback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { GetValuesConfig, ITweekClient } from './types';

export default class TweekClientWithFallback implements ITweekClient {
constructor(private readonly _clients: ITweekClient[]) {}

getValues<T>(path: string, config: GetValuesConfig): Promise<T> {
return this._execute(client => client.getValues(path, config));
}

fetch<T>(path: string, config?: GetValuesConfig): Promise<T> {
return this._execute(client => client.fetch(path, config));
}

private _execute<T>(fn: (client: ITweekClient) => Promise<T>): Promise<T> {
return this._clients.reduce((acc, client) => acc.catch(() => fn(client)), Promise.reject(
new Error('TweekClientWithFallback has no clients'),
) as Promise<T>);
}
}
8 changes: 3 additions & 5 deletions js/tweek-client/src/TweekClient/createTweekClient.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import { createFetchClient } from '../utils';
import { CreateClientConfig } from '../types';
import { Context } from './types';
import { BaseCreateTweekClientConfig } from './types';
import TweekClient from './TweekClient';

export type CreateTweekClientConfig = CreateClientConfig & {
context?: Context;
useLegacyEndpoint?: boolean;
export type CreateTweekClientConfig = BaseCreateTweekClientConfig & {
baseServiceUrl: string;
};

export function createTweekClient({
Expand Down
12 changes: 12 additions & 0 deletions js/tweek-client/src/TweekClient/createTweekClientWithFallback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { createTweekClient } from './createTweekClient';
import TweekClientWithFallback from './TweekClientWithFallback';
import { BaseCreateTweekClientConfig } from './types';

export type CreateTweekClientConfigWithFallback = BaseCreateTweekClientConfig & {
urls: string[];
};

export function createTweekClientWithFallback({ urls, ...config }: CreateTweekClientConfigWithFallback) {
const clients = urls.map(baseServiceUrl => createTweekClient({ baseServiceUrl, ...config }));
return new TweekClientWithFallback(clients);
}
3 changes: 3 additions & 0 deletions js/tweek-client/src/TweekClient/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
export * from './types';
export { default as TweekClient } from './TweekClient';
export { default as TweekClientWithFallback } from './TweekClientWithFallback';
export * from './createTweekClient';
export * from './createTweekClientWithFallback';
export * from './KeyValuesError';
13 changes: 12 additions & 1 deletion js/tweek-client/src/TweekClient/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IdentityContext, TweekInitConfig } from '../types';
import { FetchClientConfig, IdentityContext, TweekInitConfig } from '../types';

export type Context = {
[identityType: string]: string | ({ id?: string } & IdentityContext);
Expand All @@ -8,17 +8,28 @@ type RequestConfig = {
include?: string[];
};

export type KeyValuesErrors = { [keyPath: string]: string };

export type KeyValuesErrorHandler = (keyPath: string, error: string) => void;

type ClientConfig = {
context?: Context;
flatten?: boolean;
ignoreKeyTypes?: boolean;
maxChunkSize?: number;
onKeyValueError?: KeyValuesErrorHandler;
throwOnError?: boolean;
};

export type GetValuesConfig = ClientConfig & RequestConfig;

export type TweekClientConfig = TweekInitConfig & ClientConfig;

export type BaseCreateTweekClientConfig = FetchClientConfig & {
context?: Context;
useLegacyEndpoint?: boolean;
};

export interface ITweekClient {
getValues<T>(path: string, config?: GetValuesConfig): Promise<T>;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,15 @@
import { createFetchClient } from '../utils';
import { CreateClientConfig } from '../types';
import { FetchClientConfig } from '../types';
import TweekManagementClient from './TweekManagementClient';

export function createTweekManagementClient({ baseServiceUrl, ...fetchClientConfig }: CreateClientConfig) {
export type CreateTweekManagementClientConfig = FetchClientConfig & {
baseServiceUrl: string;
};

export function createTweekManagementClient({
baseServiceUrl,
...fetchClientConfig
}: CreateTweekManagementClientConfig) {
return new TweekManagementClient({
baseServiceUrl,
fetch: createFetchClient(fetchClientConfig),
Expand Down
4 changes: 0 additions & 4 deletions js/tweek-client/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,3 @@ export type FetchClientConfig = (BearerAuthenticationOptions | ClientCredentials
requestTimeoutInMillis?: number;
onError?(response: Response): void;
};

export type CreateClientConfig = FetchClientConfig & {
baseServiceUrl: string;
};
18 changes: 18 additions & 0 deletions js/tweek-client/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,21 @@ export const toQueryString = (query: InputParams) => {
const queryString = qs.stringify(query);
return queryString ? `?${queryString}` : '';
};

export function deprecated(newMethod: string) {
let notified = false;
return function(target: Object, propertyKey: string, descriptor: PropertyDescriptor) {
const originalValue = descriptor.value;
descriptor.value = function() {
if (!notified) {
if (typeof process !== 'undefined' && process.env.NODE_ENV !== 'production') {
const name = target.constructor.name;
console.warn(`the ${name}.${propertyKey} method is deprecated, please use ${name}.${newMethod} instead`);
}
notified = true;
}

return originalValue.apply(this, arguments);
};
};
}
100 changes: 100 additions & 0 deletions js/tweek-client/test/TweekClient/errorHandling.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import sinon from 'sinon';
import { expect } from 'chai';
import { KeyValuesError, TweekClient } from '../../src';

describe('TweekClient errorHandling', () => {
let fetchStub: sinon.SinonStub;
let onKeyError: sinon.SinonStub;
let tweekClient: TweekClient;

beforeEach(() => {
fetchStub = sinon.stub();
onKeyError = sinon.stub();
});

describe('onKeyValueError', () => {
beforeEach(() => {
tweekClient = new TweekClient({
baseServiceUrl: '',
fetch: fetchStub,
onKeyValueError: onKeyError,
});
});

it('should not call onKeyValueError if errors is not in the response body', async () => {
fetchStub.resolves(new Response(JSON.stringify({})));

await tweekClient.getValues('');

sinon.assert.notCalled(onKeyError);
});

it('should not call onKeyValueError if errors object is empty', async () => {
fetchStub.resolves(new Response(JSON.stringify({ errors: {} })));

await tweekClient.getValues('');

sinon.assert.notCalled(onKeyError);
});

it('should call onKeyValueError if errors object contains errors', async () => {
const errors = {
some_key: 'some error',
other_key: 'other error',
};
fetchStub.resolves(new Response(JSON.stringify({ errors })));

await tweekClient.getValues('');

Object.entries(errors).forEach(([keyPath, error]) => sinon.assert.calledWithExactly(onKeyError, keyPath, error));
});
});

describe('throwOnError', () => {
beforeEach(() => {
tweekClient = new TweekClient({
baseServiceUrl: '',
fetch: fetchStub,
throwOnError: true,
});
});

it('should not throw if errors is not in the response body', async () => {
fetchStub.resolves(new Response(JSON.stringify({})));

await tweekClient.getValues('');
});

it('should not throw if errors object is empty', async () => {
fetchStub.resolves(new Response(JSON.stringify({ errors: {} })));

await tweekClient.getValues('');
});

it('should throw if errors object contains errors', async () => {
const errors = { some_key: 'some error' };
fetchStub.resolves(new Response(JSON.stringify({ errors })));

await expect(tweekClient.getValues('')).to.be.rejectedWith(KeyValuesError);
});
});

it('should throw and call onKeyValueError if both are defined', async () => {
const keyPath = 'some_key';
const error = 'some error';

tweekClient = new TweekClient({
baseServiceUrl: '',
fetch: fetchStub,
onKeyValueError: onKeyError,
throwOnError: true,
});

fetchStub.resolves(new Response(JSON.stringify({ errors: { [keyPath]: error } })));

await expect(tweekClient.getValues('')).to.be.rejectedWith(KeyValuesError);

sinon.assert.calledOnce(onKeyError);
sinon.assert.calledWithExactly(onKeyError, keyPath, error);
});
});
12 changes: 6 additions & 6 deletions js/tweek-client/test/TweekClient/fetchChuncks.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ describe('TweekClient fetchChunks', () => {
expectedUrl: `${defaultUrl}api/v2/values/_`,
stubCalls: [
{
requestUrl: 'http://test/api/v2/values/_?%24include=a1&%24include=a2&%24include=a3',
response: new Response('{ "a1": 1, "a2": 2, "a3": 3 }'),
requestUrl: 'http://test/api/v2/values/_?%24include=a1&%24include=a2&%24include=a3&%24includeErrors=true',
response: new Response('{"data": { "a1": 1, "a2": 2, "a3": 3 } }'),
},
{
requestUrl: 'http://test/api/v2/values/_?%24include=b1&%24include=b2&%24include=b3',
response: new Response('{ "b1": "a", "b2": "b", "b3": "c" }'),
requestUrl: 'http://test/api/v2/values/_?%24include=b1&%24include=b2&%24include=b3&%24includeErrors=true',
response: new Response('{ "data": { "b1": "a", "b2": "b", "b3": "c" } }'),
},
{
requestUrl: 'http://test/api/v2/values/_?%24include=c5',
response: new Response('{ "c5": true }'),
requestUrl: 'http://test/api/v2/values/_?%24include=c5&%24includeErrors=true',
response: new Response('{ "data": { "c5": true } }'),
},
],
config: {
Expand Down
Loading

0 comments on commit 4fe6be3

Please sign in to comment.