From 1daa265ff82b58e54e658b8f5834aa5fe1701a9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Fri, 31 Aug 2018 15:17:24 +0200 Subject: [PATCH] [kfetch] Add support for interceptors (#22128) --- .../error_auto_create_index.test.js | 32 +- src/ui/public/kfetch/index.ts | 2 +- src/ui/public/kfetch/kfetch.test.ts | 468 ++++++++++++++++-- src/ui/public/kfetch/kfetch.ts | 114 +++-- src/ui/public/kfetch/kfetch_abortable.test.ts | 2 +- src/ui/public/kfetch/kfetch_error.ts | 30 ++ 6 files changed, 525 insertions(+), 123 deletions(-) create mode 100644 src/ui/public/kfetch/kfetch_error.ts diff --git a/src/ui/public/error_auto_create_index/error_auto_create_index.test.js b/src/ui/public/error_auto_create_index/error_auto_create_index.test.js index fbc028d2e92199..39c4ac691ff2bc 100644 --- a/src/ui/public/error_auto_create_index/error_auto_create_index.test.js +++ b/src/ui/public/error_auto_create_index/error_auto_create_index.test.js @@ -30,7 +30,7 @@ import fetchMock from 'fetch-mock'; import { kfetch } from 'ui/kfetch'; import { isAutoCreateIndexError } from './error_auto_create_index'; -describe('isAutoCreateIndexError correctly handles FetchError thrown by kfetch', () => { +describe('isAutoCreateIndexError correctly handles KFetchError thrown by kfetch', () => { describe('404', () => { beforeEach(() => { fetchMock.post({ @@ -43,15 +43,12 @@ describe('isAutoCreateIndexError correctly handles FetchError thrown by kfetch', afterEach(() => fetchMock.restore()); test('should return false', async () => { - let gotError = false; + expect.assertions(1); try { await kfetch({ method: 'POST', pathname: 'my/path' }); - } catch (fetchError) { - gotError = true; - expect(isAutoCreateIndexError(fetchError)).toBe(false); + } catch (kfetchError) { + expect(isAutoCreateIndexError(kfetchError)).toBe(false); } - - expect(gotError).toBe(true); }); }); @@ -67,15 +64,12 @@ describe('isAutoCreateIndexError correctly handles FetchError thrown by kfetch', afterEach(() => fetchMock.restore()); test('should return false', async () => { - let gotError = false; + expect.assertions(1); try { await kfetch({ method: 'POST', pathname: 'my/path' }); - } catch (fetchError) { - gotError = true; - expect(isAutoCreateIndexError(fetchError)).toBe(false); + } catch (kfetchError) { + expect(isAutoCreateIndexError(kfetchError)).toBe(false); } - - expect(gotError).toBe(true); }); }); @@ -85,7 +79,7 @@ describe('isAutoCreateIndexError correctly handles FetchError thrown by kfetch', matcher: '*', response: { body: { - code: 'ES_AUTO_CREATE_INDEX_ERROR' + code: 'ES_AUTO_CREATE_INDEX_ERROR', }, status: 503, }, @@ -94,16 +88,12 @@ describe('isAutoCreateIndexError correctly handles FetchError thrown by kfetch', afterEach(() => fetchMock.restore()); test('should return true', async () => { - let gotError = false; + expect.assertions(1); try { await kfetch({ method: 'POST', pathname: 'my/path' }); - } catch (fetchError) { - gotError = true; - expect(isAutoCreateIndexError(fetchError)).toBe(true); + } catch (kfetchError) { + expect(isAutoCreateIndexError(kfetchError)).toBe(true); } - - expect(gotError).toBe(true); }); }); }); - diff --git a/src/ui/public/kfetch/index.ts b/src/ui/public/kfetch/index.ts index ab1cadf0c2371a..1b0861da9fc1bd 100644 --- a/src/ui/public/kfetch/index.ts +++ b/src/ui/public/kfetch/index.ts @@ -17,5 +17,5 @@ * under the License. */ -export { kfetch } from './kfetch'; +export { kfetch, addInterceptor } from './kfetch'; export { kfetchAbortable } from './kfetch_abortable'; diff --git a/src/ui/public/kfetch/kfetch.test.ts b/src/ui/public/kfetch/kfetch.test.ts index a227bdf207139d..c93630f3a898f8 100644 --- a/src/ui/public/kfetch/kfetch.test.ts +++ b/src/ui/public/kfetch/kfetch.test.ts @@ -18,7 +18,7 @@ */ jest.mock('../chrome', () => ({ - addBasePath: (path: string) => `myBase/${path}`, + addBasePath: (path: string) => `http://localhost.com/myBase/${path}`, })); jest.mock('../metadata', () => ({ @@ -28,84 +28,446 @@ jest.mock('../metadata', () => ({ })); import fetchMock from 'fetch-mock'; -import { kfetch } from './kfetch'; +import { + addInterceptor, + Interceptor, + kfetch, + resetInterceptors, + withDefaultOptions, +} from './kfetch'; +import { KFetchError } from './kfetch_error'; describe('kfetch', () => { - const matcherName: any = /my\/path/; + afterEach(() => { + fetchMock.restore(); + resetInterceptors(); + }); - describe('resolves', () => { - beforeEach(() => fetchMock.get(matcherName, new Response(JSON.stringify({ foo: 'bar' })))); - afterEach(() => fetchMock.restore()); + it('should use supplied request method', async () => { + fetchMock.post('*', {}); + await kfetch({ pathname: 'my/path', method: 'POST' }); + expect(fetchMock.lastOptions('*').method).toBe('POST'); + }); - it('should return response', async () => { - expect(await kfetch({ pathname: 'my/path', query: { a: 'b' } })).toEqual({ - foo: 'bar', - }); + it('should use supplied Content-Type', async () => { + fetchMock.get('*', {}); + await kfetch({ pathname: 'my/path', headers: { 'Content-Type': 'CustomContentType' } }); + expect(fetchMock.lastOptions('*').headers).toMatchObject({ + 'Content-Type': 'CustomContentType', + }); + }); + + it('should use supplied pathname and querystring', async () => { + fetchMock.get('*', {}); + await kfetch({ pathname: 'my/path', query: { a: 'b' } }); + expect(fetchMock.lastUrl('*')).toBe('http://localhost.com/myBase/my/path?a=b'); + }); + + it('should use supplied headers', async () => { + fetchMock.get('*', {}); + await kfetch({ + pathname: 'my/path', + headers: { myHeader: 'foo' }, + }); + + expect(fetchMock.lastOptions('*').headers).toEqual({ + 'Content-Type': 'application/json', + 'kbn-version': 'my-version', + myHeader: 'foo', + }); + }); + + it('should return response', async () => { + fetchMock.get('*', { foo: 'bar' }); + expect(await kfetch({ pathname: 'my/path' })).toEqual({ + foo: 'bar', + }); + }); + + it('should prepend url with basepath by default', async () => { + fetchMock.get('*', {}); + await kfetch({ pathname: 'my/path' }); + expect(fetchMock.lastUrl('*')).toBe('http://localhost.com/myBase/my/path'); + }); + + it('should not prepend url with basepath when disabled', async () => { + fetchMock.get('*', {}); + await kfetch({ pathname: 'my/path' }, { prependBasePath: false }); + expect(fetchMock.lastUrl('*')).toBe('my/path'); + }); + + it('should make request with defaults', async () => { + fetchMock.get('*', {}); + await kfetch({ pathname: 'my/path' }); + + expect(fetchMock.lastOptions('*')).toEqual({ + method: 'GET', + credentials: 'same-origin', + headers: { + 'Content-Type': 'application/json', + 'kbn-version': 'my-version', + }, + }); + }); + + it('should reject on network error', async () => { + expect.assertions(1); + fetchMock.get('*', { throws: new Error('Network issue') }); + + try { + await kfetch({ pathname: 'my/path' }); + } catch (e) { + expect(e.message).toBe('Network issue'); + } + }); + + describe('when throwing response error (KFetchError)', async () => { + let error: KFetchError; + beforeEach(async () => { + fetchMock.get('*', { status: 404, body: { foo: 'bar' } }); + try { + await kfetch({ pathname: 'my/path' }); + } catch (e) { + error = e; + } + }); + + it('should contain error message', () => { + expect(error.message).toBe('Not Found'); + }); + + it('should return response body', () => { + expect(error.body).toEqual({ foo: 'bar' }); + }); + + it('should contain response properties', () => { + expect(error.res.status).toBe(404); + expect(error.res.url).toBe('http://localhost.com/myBase/my/path'); + }); + }); + + describe('when all interceptor resolves', () => { + let resp: any; + let interceptorCalls: string[]; + + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + + interceptorCalls = mockInterceptorCalls([{}, {}, {}]); + resp = await kfetch({ pathname: 'my/path' }); + }); + + it('should call interceptors in correct order', () => { + expect(interceptorCalls).toEqual([ + 'Request #3', + 'Request #2', + 'Request #1', + 'Response #1', + 'Response #2', + 'Response #3', + ]); + }); + + it('should make request', () => { + expect(fetchMock.called('*')).toBe(true); + }); + + it('should return response', () => { + expect(resp).toEqual({ foo: 'bar' }); + }); + }); + + describe('when a request interceptor throws; and the next requestError interceptor resolves', () => { + let resp: any; + let interceptorCalls: string[]; + + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + + interceptorCalls = mockInterceptorCalls([ + { requestError: () => ({}) }, + { request: () => Promise.reject(new Error('Error in request')) }, + {}, + ]); + + resp = await kfetch({ pathname: 'my/path' }); + }); + + it('should call interceptors in correct order', () => { + expect(interceptorCalls).toEqual([ + 'Request #3', + 'Request #2', + 'RequestError #1', + 'Response #1', + 'Response #2', + 'Response #3', + ]); + }); + + it('should make request', () => { + expect(fetchMock.called('*')).toBe(true); + }); + + it('should return response', () => { + expect(resp).toEqual({ foo: 'bar' }); + }); + }); + + describe('when a request interceptor throws', () => { + let error: Error; + let interceptorCalls: string[]; + + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + + interceptorCalls = mockInterceptorCalls([ + {}, + { request: () => Promise.reject(new Error('Error in request')) }, + {}, + ]); + + try { + await kfetch({ pathname: 'my/path' }); + } catch (e) { + error = e; + } }); - it('should prepend with basepath by default', async () => { - await kfetch({ pathname: 'my/path', query: { a: 'b' } }); - expect(fetchMock.lastUrl(matcherName)).toBe('myBase/my/path?a=b'); + it('should call interceptors in correct order', () => { + expect(interceptorCalls).toEqual([ + 'Request #3', + 'Request #2', + 'RequestError #1', + 'ResponseError #1', + 'ResponseError #2', + 'ResponseError #3', + ]); }); - it('should not prepend with basepath when disabled', async () => { - await kfetch( + it('should not make request', () => { + expect(fetchMock.called('*')).toBe(false); + }); + + it('should throw error', () => { + expect(error.message).toEqual('Error in request'); + }); + }); + + describe('when a response interceptor throws', () => { + let error: Error; + let interceptorCalls: string[]; + + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + + interceptorCalls = mockInterceptorCalls([ + { response: () => Promise.reject(new Error('Error in response')) }, + {}, + {}, + ]); + + try { + await kfetch({ pathname: 'my/path' }); + } catch (e) { + error = e; + } + }); + + it('should call in correct order', () => { + expect(interceptorCalls).toEqual([ + 'Request #3', + 'Request #2', + 'Request #1', + 'Response #1', + 'ResponseError #2', + 'ResponseError #3', + ]); + }); + + it('should make request', () => { + expect(fetchMock.called('*')).toBe(true); + }); + + it('should throw error', () => { + expect(error.message).toEqual('Error in response'); + }); + }); + + describe('when request interceptor throws; and a responseError interceptor resolves', () => { + let resp: any; + let interceptorCalls: string[]; + + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + + interceptorCalls = mockInterceptorCalls([ + {}, { - pathname: 'my/path', - query: { a: 'b' }, + request: () => { + throw new Error('My request error'); + }, + responseError: () => { + return { custom: 'response' }; + }, }, - { - prependBasePath: false, - } - ); + {}, + ]); - expect(fetchMock.lastUrl(matcherName)).toBe('my/path?a=b'); + resp = await kfetch({ pathname: 'my/path' }); }); - it('should call with default options', async () => { - await kfetch({ pathname: 'my/path', query: { a: 'b' } }); + it('should call in correct order', () => { + expect(interceptorCalls).toEqual([ + 'Request #3', + 'Request #2', + 'RequestError #1', + 'ResponseError #1', + 'ResponseError #2', + 'Response #3', + ]); + }); - expect(fetchMock.lastOptions(matcherName)).toEqual({ - method: 'GET', - credentials: 'same-origin', - headers: { - 'Content-Type': 'application/json', - 'kbn-version': 'my-version', - }, + it('should not make request', () => { + expect(fetchMock.called('*')).toBe(false); + }); + + it('should resolve', () => { + expect(resp).toEqual({ custom: 'response' }); + }); + }); + + describe('when interceptors return synchronously', async () => { + let resp: any; + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + addInterceptor({ + request: config => ({ + ...config, + addedByRequestInterceptor: true, + }), + response: res => ({ + ...res, + addedByResponseInterceptor: true, + }), }); + + resp = await kfetch({ pathname: 'my/path' }); }); - it('should merge headers', async () => { - await kfetch({ - pathname: 'my/path', - query: { a: 'b' }, - headers: { myHeader: 'foo' }, + it('should modify request', () => { + expect(fetchMock.lastOptions('*')).toMatchObject({ + addedByRequestInterceptor: true, + method: 'GET', }); - expect(fetchMock.lastOptions(matcherName).headers).toEqual({ - 'Content-Type': 'application/json', - 'kbn-version': 'my-version', - myHeader: 'foo', + }); + + it('should modify response', () => { + expect(resp).toEqual({ + addedByResponseInterceptor: true, + foo: 'bar', }); }); }); - describe('rejects', () => { - beforeEach(() => { - fetchMock.get(matcherName, { - status: 404, + describe('when interceptors return promise', async () => { + let resp: any; + beforeEach(async () => { + fetchMock.get('*', { foo: 'bar' }); + addInterceptor({ + request: config => + Promise.resolve({ + ...config, + addedByRequestInterceptor: true, + }), + response: res => + Promise.resolve({ + ...res, + addedByResponseInterceptor: true, + }), + }); + + resp = await kfetch({ pathname: 'my/path' }); + }); + + it('should modify request', () => { + expect(fetchMock.lastOptions('*')).toMatchObject({ + addedByRequestInterceptor: true, + method: 'GET', }); }); - afterEach(() => fetchMock.restore()); - it('should throw custom error containing response object', () => { - return kfetch({ - pathname: 'my/path', - query: { a: 'b' }, - }).catch(e => { - expect(e.message).toBe('Not Found'); - expect(e.res.status).toBe(404); - expect(e.res.url).toBe('myBase/my/path?a=b'); + it('should modify response', () => { + expect(resp).toEqual({ + addedByResponseInterceptor: true, + foo: 'bar', }); }); }); }); + +function mockInterceptorCalls(interceptors: Interceptor[]) { + const interceptorCalls: string[] = []; + interceptors.forEach((interceptor, i) => { + addInterceptor({ + request: config => { + interceptorCalls.push(`Request #${i + 1}`); + + if (interceptor.request) { + return interceptor.request(config); + } + + return config; + }, + requestError: e => { + interceptorCalls.push(`RequestError #${i + 1}`); + if (interceptor.requestError) { + return interceptor.requestError(e); + } + + throw e; + }, + response: res => { + interceptorCalls.push(`Response #${i + 1}`); + + if (interceptor.response) { + return interceptor.response(res); + } + + return res; + }, + responseError: e => { + interceptorCalls.push(`ResponseError #${i + 1}`); + + if (interceptor.responseError) { + return interceptor.responseError(e); + } + + throw e; + }, + }); + }); + + return interceptorCalls; +} + +describe('withDefaultOptions', () => { + it('should remove undefined query params', () => { + const { query } = withDefaultOptions({ + query: { + foo: 'bar', + param1: (undefined as any) as string, + param2: (null as any) as string, + param3: '', + }, + }); + expect(query).toEqual({ foo: 'bar', param2: null, param3: '' }); + }); + + it('should add default options', () => { + expect(withDefaultOptions({})).toEqual({ + credentials: 'same-origin', + headers: { 'Content-Type': 'application/json', 'kbn-version': 'my-version' }, + method: 'GET', + }); + }); +}); diff --git a/src/ui/public/kfetch/kfetch.ts b/src/ui/public/kfetch/kfetch.ts index bf0515af1c9187..cf3e41c308c65c 100644 --- a/src/ui/public/kfetch/kfetch.ts +++ b/src/ui/public/kfetch/kfetch.ts @@ -19,36 +19,85 @@ import 'isomorphic-fetch'; import { merge } from 'lodash'; +// @ts-ignore not really worth typing +import { metadata } from 'ui/metadata'; import url from 'url'; import chrome from '../chrome'; +import { KFetchError } from './kfetch_error'; -// @ts-ignore not really worth typing -import { metadata } from '../metadata'; - -class FetchError extends Error { - constructor(public readonly res: Response, public readonly body?: any) { - super(res.statusText); - - // captureStackTrace is only available in the V8 engine, so any browser using - // a different JS engine won't have access to this method. - if (Error.captureStackTrace) { - Error.captureStackTrace(this, FetchError); - } - } +interface KFetchQuery { + [key: string]: string | number | boolean; } export interface KFetchOptions extends RequestInit { pathname?: string; - query?: { [key: string]: string | number | boolean }; + query?: KFetchQuery; } export interface KFetchKibanaOptions { prependBasePath?: boolean; } -export function kfetch(fetchOptions: KFetchOptions, kibanaOptions?: KFetchKibanaOptions) { - // fetch specific options with defaults - const { pathname, query, ...combinedFetchOptions } = merge( +export interface Interceptor { + request?: (config: KFetchOptions) => Promise | KFetchOptions; + requestError?: (e: any) => Promise | KFetchOptions; + response?: (res: any) => any; + responseError?: (e: any) => any; +} + +const interceptors: Interceptor[] = []; +export const resetInterceptors = () => (interceptors.length = 0); +export const addInterceptor = (interceptor: Interceptor) => interceptors.push(interceptor); + +export async function kfetch( + options: KFetchOptions, + { prependBasePath = true }: KFetchKibanaOptions = {} +) { + const combinedOptions = withDefaultOptions(options); + const promise = requestInterceptors(combinedOptions).then( + ({ pathname, query, ...restOptions }) => { + const fullUrl = url.format({ + pathname: prependBasePath ? chrome.addBasePath(pathname) : pathname, + query, + }); + + return fetch(fullUrl, restOptions).then(async res => { + if (res.ok) { + return res.json(); + } + throw new KFetchError(res, await getBodyAsJson(res)); + }); + } + ); + + return responseInterceptors(promise); +} + +// Request/response interceptors are called in opposite orders. +// Request hooks start from the newest interceptor and end with the oldest. +function requestInterceptors(config: KFetchOptions): Promise { + return interceptors.reduceRight((acc, interceptor) => { + return acc.then(interceptor.request, interceptor.requestError); + }, Promise.resolve(config)); +} + +// Response hooks start from the oldest interceptor and end with the newest. +function responseInterceptors(responsePromise: Promise) { + return interceptors.reduce((acc, interceptor) => { + return acc.then(interceptor.response, interceptor.responseError); + }, responsePromise); +} + +async function getBodyAsJson(res: Response) { + try { + return await res.json(); + } catch (e) { + return null; + } +} + +export function withDefaultOptions(options?: KFetchOptions): KFetchOptions { + return merge( { method: 'GET', credentials: 'same-origin', @@ -57,35 +106,6 @@ export function kfetch(fetchOptions: KFetchOptions, kibanaOptions?: KFetchKibana 'kbn-version': metadata.version, }, }, - fetchOptions + options ); - - // kibana specific options with defaults - const combinedKibanaOptions = { - prependBasePath: true, - ...kibanaOptions, - }; - - const fullUrl = url.format({ - pathname: combinedKibanaOptions.prependBasePath ? chrome.addBasePath(pathname) : pathname, - query, - }); - - const fetching = new Promise(async (resolve, reject) => { - const res = await fetch(fullUrl, combinedFetchOptions); - - if (res.ok) { - return resolve(await res.json()); - } - - try { - // attempt to read the body of the response - return reject(new FetchError(res, await res.json())); - } catch (_) { - // send FetchError without the body if we are not be able to read the body for some reason - return reject(new FetchError(res)); - } - }); - - return fetching; } diff --git a/src/ui/public/kfetch/kfetch_abortable.test.ts b/src/ui/public/kfetch/kfetch_abortable.test.ts index 5d2351b3cbb58e..38ce8faed43d52 100644 --- a/src/ui/public/kfetch/kfetch_abortable.test.ts +++ b/src/ui/public/kfetch/kfetch_abortable.test.ts @@ -18,7 +18,7 @@ */ jest.mock('../chrome', () => ({ - addBasePath: (path: string) => `myBase/${path}`, + addBasePath: (path: string) => `http://localhost.com/myBase/${path}`, })); jest.mock('../metadata', () => ({ diff --git a/src/ui/public/kfetch/kfetch_error.ts b/src/ui/public/kfetch/kfetch_error.ts new file mode 100644 index 00000000000000..f351959e624b85 --- /dev/null +++ b/src/ui/public/kfetch/kfetch_error.ts @@ -0,0 +1,30 @@ +/* + * Licensed to Elasticsearch B.V. under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +export class KFetchError extends Error { + constructor(public readonly res: Response, public readonly body?: any) { + super(res.statusText); + + // captureStackTrace is only available in the V8 engine, so any browser using + // a different JS engine won't have access to this method. + if (Error.captureStackTrace) { + Error.captureStackTrace(this, KFetchError); + } + } +}