From 32af0ba540b633211e4ed88081d5a21e003eb270 Mon Sep 17 00:00:00 2001 From: Eli Perelman Date: Tue, 23 Apr 2019 11:23:46 -0500 Subject: [PATCH] Expose an HTTP-request browser client --- src/core/public/core_system.ts | 2 +- .../public/http/abortable.ts} | 35 ++- .../public/http/http_fetch_error.ts} | 33 +-- src/core/public/http/http_service.mock.ts | 31 ++- src/core/public/http/http_service.test.ts | 220 ++++++++++++++++-- src/core/public/http/http_service.ts | 82 ++++++- src/core/public/http/index.ts | 1 + src/core/public/http/types.ts | 41 ++++ src/core/public/legacy/legacy_service.ts | 1 + src/legacy/ui/public/kfetch/index.ts | 22 +- src/legacy/ui/public/kfetch/kfetch.test.ts | 104 +++++---- src/legacy/ui/public/kfetch/kfetch.ts | 78 +++---- .../public/search/rollup_search_strategy.js | 11 +- 13 files changed, 478 insertions(+), 183 deletions(-) rename src/{legacy/ui/public/kfetch/kfetch_abortable.test.ts => core/public/http/abortable.ts} (56%) rename src/{legacy/ui/public/kfetch/kfetch_abortable.ts => core/public/http/http_fetch_error.ts} (57%) create mode 100644 src/core/public/http/types.ts diff --git a/src/core/public/core_system.ts b/src/core/public/core_system.ts index 5d301dfd7f99f17..1f3382fa09033d6 100644 --- a/src/core/public/core_system.ts +++ b/src/core/public/core_system.ts @@ -119,8 +119,8 @@ export class CoreSystem { const i18n = this.i18n.setup(); const injectedMetadata = this.injectedMetadata.setup(); const fatalErrors = this.fatalErrors.setup({ i18n }); - const http = this.http.setup({ fatalErrors }); const basePath = this.basePath.setup({ injectedMetadata }); + const http = this.http.setup({ basePath, injectedMetadata, fatalErrors }); const uiSettings = this.uiSettings.setup({ http, injectedMetadata, diff --git a/src/legacy/ui/public/kfetch/kfetch_abortable.test.ts b/src/core/public/http/abortable.ts similarity index 56% rename from src/legacy/ui/public/kfetch/kfetch_abortable.test.ts rename to src/core/public/http/abortable.ts index bb1c5ac07252433..ff728d9280d694d 100644 --- a/src/legacy/ui/public/kfetch/kfetch_abortable.test.ts +++ b/src/core/public/http/abortable.ts @@ -17,23 +17,22 @@ * under the License. */ -jest.mock('../chrome', () => ({ - addBasePath: (path: string) => `http://localhost/myBase/${path}`, -})); +import { HttpHandler, HttpFetchOptions, AbortablePromise } from './types'; -jest.mock('../metadata', () => ({ - metadata: { - version: 'my-version', - }, -})); +export function abortable(handler: HttpHandler) { + return (path: string, options: HttpFetchOptions = {}): AbortablePromise => { + const controller = options.signal + ? new AbortController() + : { signal: options.signal, abort: Function.prototype }; + const promise = new Promise((resolve, reject) => { + handler(path, { ...options, signal: controller.signal }).then(resolve, reject); + }); -import { kfetchAbortable } from './kfetch_abortable'; - -describe('kfetchAbortable', () => { - it('should return an object with a fetching promise and an abort callback', () => { - const { fetching, abort } = kfetchAbortable({ pathname: 'my/path' }); - expect(typeof fetching.then).toBe('function'); - expect(typeof fetching.catch).toBe('function'); - expect(typeof abort).toBe('function'); - }); -}); + return Object.assign(promise, { + abort() { + controller.abort(); + return promise; + }, + }); + }; +} diff --git a/src/legacy/ui/public/kfetch/kfetch_abortable.ts b/src/core/public/http/http_fetch_error.ts similarity index 57% rename from src/legacy/ui/public/kfetch/kfetch_abortable.ts rename to src/core/public/http/http_fetch_error.ts index 11057054f330f31..a73fb7e3ffbd409 100644 --- a/src/legacy/ui/public/kfetch/kfetch_abortable.ts +++ b/src/core/public/http/http_fetch_error.ts @@ -17,29 +17,14 @@ * under the License. */ -import { kfetch, KFetchKibanaOptions, KFetchOptions } from './kfetch'; +export class HttpFetchError extends Error { + constructor(message: string, public readonly response?: Response, public readonly body?: any) { + super(message); -type Omit = Pick>; - -function createAbortable() { - const abortController = new AbortController(); - const { signal, abort } = abortController; - - return { - signal, - abort: abort.bind(abortController), - }; -} - -export function kfetchAbortable( - fetchOptions?: Omit, - kibanaOptions?: KFetchKibanaOptions -) { - const { signal, abort } = createAbortable(); - const fetching = kfetch({ ...fetchOptions, signal }, kibanaOptions); - - return { - fetching, - abort, - }; + // 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, HttpFetchError); + } + } } diff --git a/src/core/public/http/http_service.mock.ts b/src/core/public/http/http_service.mock.ts index 3146ad4d2217f38..6ee298584268423 100644 --- a/src/core/public/http/http_service.mock.ts +++ b/src/core/public/http/http_service.mock.ts @@ -16,25 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -import { HttpService, HttpSetup } from './http_service'; -const createSetupContractMock = () => { - const setupContract: jest.Mocked = { - addLoadingCount: jest.fn(), - getLoadingCount$: jest.fn(), - }; - return setupContract; -}; +import { HttpService, HttpSetup } from './http_service'; -type HttpServiceContract = PublicMethodsOf; -const createMock = () => { - const mocked: jest.Mocked = { - setup: jest.fn(), - stop: jest.fn(), - }; - mocked.setup.mockReturnValue(createSetupContractMock()); - return mocked; -}; +const createSetupContractMock = (): jest.Mocked => ({ + fetch: jest.fn(), + get: jest.fn(), + post: jest.fn(), + put: jest.fn(), + del: jest.fn(), + addLoadingCount: jest.fn(), + getLoadingCount$: jest.fn(), +}); +const createMock = (): jest.Mocked> => ({ + setup: jest.fn().mockReturnValue(createSetupContractMock()), + stop: jest.fn(), +}); export const httpServiceMock = { create: createMock, diff --git a/src/core/public/http/http_service.test.ts b/src/core/public/http/http_service.test.ts index 35b674aca9b9f36..2fd8c14bbe37742 100644 --- a/src/core/public/http/http_service.test.ts +++ b/src/core/public/http/http_service.test.ts @@ -19,35 +19,213 @@ import * as Rx from 'rxjs'; import { toArray } from 'rxjs/operators'; +// @ts-ignore +import fetchMock from 'fetch-mock/es5/client'; +import { BasePathService } from '../base_path/base_path_service'; import { fatalErrorsServiceMock } from '../fatal_errors/fatal_errors_service.mock'; +import { injectedMetadataServiceMock } from '../injected_metadata/injected_metadata_service.mock'; import { HttpService } from './http_service'; function setupService() { - const service = new HttpService(); + const httpService = new HttpService(); const fatalErrors = fatalErrorsServiceMock.createSetupContract(); - const setup = service.setup({ fatalErrors }); + const injectedMetadata = injectedMetadataServiceMock.createSetupContract(); - return { service, fatalErrors, setup }; + injectedMetadata.getBasePath.mockReturnValueOnce('http://localhost/myBase'); + + const basePath = new BasePathService().setup({ injectedMetadata }); + const http = httpService.setup({ basePath, fatalErrors, injectedMetadata }); + + return { httpService, fatalErrors, http }; } +describe('http requests', async () => { + afterEach(() => { + fetchMock.restore(); + }); + + it('should use supplied request method', async () => { + const { http } = setupService(); + + fetchMock.post('*', {}); + await http.fetch('/my/path', { method: 'POST' }); + + expect(fetchMock.lastOptions()!.method).toBe('POST'); + }); + + it('should use supplied Content-Type', async () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + await http.fetch('/my/path', { headers: { 'Content-Type': 'CustomContentType' } }); + + expect(fetchMock.lastOptions()!.headers).toMatchObject({ + 'Content-Type': 'CustomContentType', + }); + }); + + it('should use supplied pathname and querystring', async () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + await http.fetch('/my/path', { query: { a: 'b' } }); + + expect(fetchMock.lastUrl()).toBe('http://localhost/myBase/my/path?a=b'); + }); + + it('should use supplied headers', async () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + await http.fetch('/my/path', { + headers: { myHeader: 'foo' }, + }); + + expect(fetchMock.lastOptions()!.headers).toEqual({ + 'Content-Type': 'application/json', + 'kbn-version': 'kibanaVersion', + myHeader: 'foo', + }); + }); + + it('should return response', async () => { + const { http } = setupService(); + + fetchMock.get('*', { foo: 'bar' }); + + const json = await http.fetch('/my/path'); + + expect(json).toEqual({ foo: 'bar' }); + }); + + it('should prepend url with basepath by default', async () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + await http.fetch('/my/path'); + + expect(fetchMock.lastUrl()).toBe('http://localhost/myBase/my/path'); + }); + + it('should not prepend url with basepath when disabled', async () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + await http.fetch('my/path', { prependBasePath: false }); + + expect(fetchMock.lastUrl()).toBe('/my/path'); + }); + + it('should make request with defaults', async () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + await http.fetch('/my/path'); + + expect(fetchMock.lastOptions()!).toMatchObject({ + method: 'GET', + credentials: 'same-origin', + headers: { + 'Content-Type': 'application/json', + 'kbn-version': 'kibanaVersion', + }, + }); + }); + + it('should reject on network error', async () => { + const { http } = setupService(); + + expect.assertions(1); + fetchMock.get('*', { status: 500 }); + + await expect(http.fetch('/my/path')).rejects.toThrow(/Internal Server Error/); + }); + + it('should contain error message when throwing response', async () => { + const { http } = setupService(); + + fetchMock.get('*', { status: 404, body: { foo: 'bar' } }); + + await expect(http.fetch('/my/path')).rejects.toMatchObject({ + message: 'Not Found', + body: { + foo: 'bar', + }, + response: { + status: 404, + url: 'http://localhost/myBase/my/path', + }, + }); + }); + + it('should support get() helper', async () => { + const { http } = setupService(); + + fetchMock.get('*', { method: 'POST' }); + await http.get('/my/path'); + + expect(fetchMock.lastOptions()!.method).toBe('GET'); + }); + + it('should support post() helper', async () => { + const { http } = setupService(); + + fetchMock.post('*', {}); + await http.post('/my/path', { method: 'GET', body: '{}' }); + + expect(fetchMock.lastOptions()!.method).toBe('POST'); + }); + + it('should support put() helper', async () => { + const { http } = setupService(); + + fetchMock.put('*', {}); + await http.put('/my/path', { method: 'GET', body: '{}' }); + + expect(fetchMock.lastOptions()!.method).toBe('PUT'); + }); + + it('should support del() helper', async () => { + const { http } = setupService(); + + fetchMock.delete('*', {}); + await http.del('/my/path', { method: 'GET' }); + + expect(fetchMock.lastOptions()!.method).toBe('DELETE'); + }); + + it('should return an abortable promise', () => { + const { http } = setupService(); + + fetchMock.get('*', {}); + + const abortable = http.fetch('/my/path'); + + expect(typeof abortable.then).toBe('function'); + expect(typeof abortable.catch).toBe('function'); + expect(typeof abortable.abort).toBe('function'); + expect(abortable.abort()).toBe(abortable); + }); +}); + describe('addLoadingCount()', async () => { it('subscribes to passed in sources, unsubscribes on stop', () => { - const { service, setup } = setupService(); + const { httpService, http } = setupService(); const unsubA = jest.fn(); const subA = jest.fn().mockReturnValue(unsubA); - setup.addLoadingCount(new Rx.Observable(subA)); + http.addLoadingCount(new Rx.Observable(subA)); expect(subA).toHaveBeenCalledTimes(1); expect(unsubA).not.toHaveBeenCalled(); const unsubB = jest.fn(); const subB = jest.fn().mockReturnValue(unsubB); - setup.addLoadingCount(new Rx.Observable(subB)); + http.addLoadingCount(new Rx.Observable(subB)); expect(subB).toHaveBeenCalledTimes(1); expect(unsubB).not.toHaveBeenCalled(); - service.stop(); + httpService.stop(); expect(subA).toHaveBeenCalledTimes(1); expect(unsubA).toHaveBeenCalledTimes(1); @@ -56,35 +234,35 @@ describe('addLoadingCount()', async () => { }); it('adds a fatal error if source observables emit an error', async () => { - const { setup, fatalErrors } = setupService(); + const { http, fatalErrors } = setupService(); - setup.addLoadingCount(Rx.throwError(new Error('foo bar'))); + http.addLoadingCount(Rx.throwError(new Error('foo bar'))); expect(fatalErrors.add.mock.calls).toMatchSnapshot(); }); it('adds a fatal error if source observable emits a negative number', async () => { - const { setup, fatalErrors } = setupService(); + const { http, fatalErrors } = setupService(); - setup.addLoadingCount(Rx.of(1, 2, 3, 4, -9)); + http.addLoadingCount(Rx.of(1, 2, 3, 4, -9)); expect(fatalErrors.add.mock.calls).toMatchSnapshot(); }); }); describe('getLoadingCount$()', async () => { it('emits 0 initially, the right count when sources emit their own count, and ends with zero', async () => { - const { service, setup } = setupService(); + const { httpService, http } = setupService(); const countA$ = new Rx.Subject(); const countB$ = new Rx.Subject(); const countC$ = new Rx.Subject(); - const promise = setup + const promise = http .getLoadingCount$() .pipe(toArray()) .toPromise(); - setup.addLoadingCount(countA$); - setup.addLoadingCount(countB$); - setup.addLoadingCount(countC$); + http.addLoadingCount(countA$); + http.addLoadingCount(countB$); + http.addLoadingCount(countC$); countA$.next(100); countB$.next(10); @@ -94,20 +272,20 @@ describe('getLoadingCount$()', async () => { countC$.complete(); countB$.next(0); - service.stop(); + httpService.stop(); expect(await promise).toMatchSnapshot(); }); it('only emits when loading count changes', async () => { - const { service, setup } = setupService(); + const { httpService, http } = setupService(); const count$ = new Rx.Subject(); - const promise = setup + const promise = http .getLoadingCount$() .pipe(toArray()) .toPromise(); - setup.addLoadingCount(count$); + http.addLoadingCount(count$); count$.next(0); count$.next(0); count$.next(0); @@ -115,7 +293,7 @@ describe('getLoadingCount$()', async () => { count$.next(0); count$.next(1); count$.next(1); - service.stop(); + httpService.stop(); expect(await promise).toMatchSnapshot(); }); diff --git a/src/core/public/http/http_service.ts b/src/core/public/http/http_service.ts index 4121181cf80ec51..1f10fda153fb9e0 100644 --- a/src/core/public/http/http_service.ts +++ b/src/core/public/http/http_service.ts @@ -28,19 +28,91 @@ import { tap, } from 'rxjs/operators'; -import { FatalErrorsSetup } from '../fatal_errors'; +import { merge } from 'lodash'; +import { format } from 'url'; -interface Deps { - fatalErrors: FatalErrorsSetup; -} +import { Deps, HttpFetchOptions, HttpBody } from './types'; +import { abortable } from './abortable'; +import { HttpFetchError } from './http_fetch_error'; + +const JSON_CONTENT = /^(application\/(json|x-javascript)|text\/(x-)?javascript|x-json)(;.*)?$/; +const NDJSON_CONTENT = /^(application\/ndjson)(;.*)?$/; /** @internal */ export class HttpService { private readonly loadingCount$ = new Rx.BehaviorSubject(0); private readonly stop$ = new Rx.Subject(); - public setup({ fatalErrors }: Deps) { + public setup({ basePath, injectedMetadata, fatalErrors }: Deps) { + const defaults: HttpFetchOptions = { + method: 'GET', + credentials: 'same-origin', + prependBasePath: true, + headers: { + 'Content-Type': 'application/json', + 'kbn-version': injectedMetadata.getKibanaVersion(), + }, + }; + + async function fetch(path: string, options: HttpFetchOptions = {}): Promise { + const { query, prependBasePath, ...fetchOptions } = merge({}, defaults, options); + const url = format({ + pathname: prependBasePath ? basePath.addToPath(path) : path, + query, + }); + + let response; + let body = null; + + try { + response = await window.fetch(url, fetchOptions); + } catch (err) { + throw new HttpFetchError(err.message); + } + + const contentType = response.headers.get('Content-Type') || ''; + + try { + if (NDJSON_CONTENT.test(contentType)) { + body = await response.blob(); + } else if (JSON_CONTENT.test(contentType)) { + body = await response.json(); + } else { + body = await response.text(); + } + } catch (err) { + throw new HttpFetchError(err.message, response, body); + } + + if (!response.ok) { + throw new HttpFetchError(response.statusText, response, body); + } + + return body; + } + + async function get(path: string, options: HttpFetchOptions = {}) { + return fetch(path, { ...options, method: 'GET' }); + } + + async function post(path: string, options: HttpFetchOptions = {}) { + return fetch(path, { ...options, method: 'POST' }); + } + + async function put(path: string, options: HttpFetchOptions = {}) { + return fetch(path, { ...options, method: 'PUT' }); + } + + async function del(path: string, options: HttpFetchOptions = {}) { + return fetch(path, { ...options, method: 'DELETE' }); + } + return { + fetch: abortable(fetch), + get: abortable(get), + post: abortable(post), + put: abortable(put), + del: abortable(del), addLoadingCount: (count$: Rx.Observable) => { count$ .pipe( diff --git a/src/core/public/http/index.ts b/src/core/public/http/index.ts index 24ba49a4dfcac11..ee17c225c711b39 100644 --- a/src/core/public/http/index.ts +++ b/src/core/public/http/index.ts @@ -18,3 +18,4 @@ */ export { HttpService, HttpSetup } from './http_service'; +export { HttpFetchError } from './http_fetch_error'; diff --git a/src/core/public/http/types.ts b/src/core/public/http/types.ts new file mode 100644 index 000000000000000..cdce74498a74a65 --- /dev/null +++ b/src/core/public/http/types.ts @@ -0,0 +1,41 @@ +/* + * 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. + */ + +import { BasePathSetup } from '../base_path'; +import { InjectedMetadataSetup } from '../injected_metadata'; +import { FatalErrorsSetup } from '../fatal_errors'; + +export interface Deps { + basePath: BasePathSetup; + injectedMetadata: InjectedMetadataSetup; + fatalErrors: FatalErrorsSetup; +} +export interface HttpFetchQuery { + [key: string]: string | number | boolean | undefined; +} +export interface HttpFetchOptions extends RequestInit { + query?: HttpFetchQuery; + prependBasePath?: boolean; +} +export interface Abortable { + abort: () => Promise; +} +export type AbortablePromise = Promise & Abortable; +export type HttpHandler = (path: string, options?: HttpFetchOptions) => Promise; +export type HttpBody = BodyInit | null; diff --git a/src/core/public/legacy/legacy_service.ts b/src/core/public/legacy/legacy_service.ts index 3fa1bc5aa7118c6..86c6d373fc9886b 100644 --- a/src/core/public/legacy/legacy_service.ts +++ b/src/core/public/legacy/legacy_service.ts @@ -69,6 +69,7 @@ export class LegacyPlatformService { require('ui/metadata').__newPlatformSetup__(injectedMetadata.getLegacyMetadata()); require('ui/i18n').__newPlatformSetup__(i18n.Context); require('ui/notify/fatal_error').__newPlatformSetup__(fatalErrors); + require('ui/kfetch').__newPlatformSetup__(http); require('ui/notify/toasts').__newPlatformSetup__(notifications.toasts); require('ui/chrome/api/loading_count').__newPlatformSetup__(http); require('ui/chrome/api/base_path').__newPlatformSetup__(basePath); diff --git a/src/legacy/ui/public/kfetch/index.ts b/src/legacy/ui/public/kfetch/index.ts index 234304b5950aa45..011321c83c4cd02 100644 --- a/src/legacy/ui/public/kfetch/index.ts +++ b/src/legacy/ui/public/kfetch/index.ts @@ -17,5 +17,23 @@ * under the License. */ -export { kfetch, addInterceptor, KFetchOptions, KFetchQuery } from './kfetch'; -export { kfetchAbortable } from './kfetch_abortable'; +import { createKfetch, KFetchKibanaOptions, KFetchOptions } from './kfetch'; +export { addInterceptor, KFetchOptions, KFetchQuery } from './kfetch'; + +import { HttpSetup } from '../../../../core/public'; + +let http: HttpSetup; +let kfetchInstance: (options: KFetchOptions, kfetchOptions?: KFetchKibanaOptions) => any; + +export function __newPlatformSetup__(httpSetup: HttpSetup) { + if (http) { + throw new Error('ui/kfetch already initialized with New Platform APIs'); + } + + http = httpSetup; + kfetchInstance = createKfetch(http); +} + +export const kfetch = (options: KFetchOptions, kfetchOptions?: KFetchKibanaOptions) => { + return kfetchInstance(options, kfetchOptions); +}; diff --git a/src/legacy/ui/public/kfetch/kfetch.test.ts b/src/legacy/ui/public/kfetch/kfetch.test.ts index 8f8cc807911a3af..2c4cf1afbd9bf1b 100644 --- a/src/legacy/ui/public/kfetch/kfetch.test.ts +++ b/src/legacy/ui/public/kfetch/kfetch.test.ts @@ -17,28 +17,37 @@ * under the License. */ -jest.mock('../chrome', () => ({ - addBasePath: (path: string) => `http://localhost/myBase/${path}`, -})); - -jest.mock('../metadata', () => ({ - metadata: { - version: 'my-version', - }, -})); - // @ts-ignore import fetchMock from 'fetch-mock/es5/client'; -import { - addInterceptor, - Interceptor, - kfetch, - resetInterceptors, - withDefaultOptions, -} from './kfetch'; +import { __newPlatformSetup__, addInterceptor, kfetch, KFetchOptions } from '.'; +import { Interceptor, resetInterceptors, withDefaultOptions } from './kfetch'; import { KFetchError } from './kfetch_error'; +/* eslint-disable @kbn/eslint/no-restricted-paths */ +import { HttpService } from '../../../../core/public/http'; +import { fatalErrorsServiceMock } from '../../../../core/public/fatal_errors/fatal_errors_service.mock'; +import { injectedMetadataServiceMock } from '../../../../core/public/injected_metadata/injected_metadata_service.mock'; +import { BasePathService } from '../../../../core/public/base_path'; +/* eslint-enable @kbn/eslint/no-restricted-paths */ + +function setupService() { + const httpService = new HttpService(); + const fatalErrors = fatalErrorsServiceMock.createSetupContract(); + const injectedMetadata = injectedMetadataServiceMock.createSetupContract(); + + injectedMetadata.getBasePath.mockReturnValue('http://localhost/myBase'); + + const basePath = new BasePathService().setup({ injectedMetadata }); + const http = httpService.setup({ basePath, fatalErrors, injectedMetadata }); + + return { httpService, fatalErrors, http }; +} + describe('kfetch', () => { + beforeAll(() => { + __newPlatformSetup__(setupService().http); + }); + afterEach(() => { fetchMock.restore(); resetInterceptors(); @@ -46,13 +55,13 @@ describe('kfetch', () => { it('should use supplied request method', async () => { fetchMock.post('*', {}); - await kfetch({ pathname: 'my/path', method: 'POST' }); + await kfetch({ pathname: '/my/path', method: 'POST' }); expect(fetchMock.lastOptions()!.method).toBe('POST'); }); it('should use supplied Content-Type', async () => { fetchMock.get('*', {}); - await kfetch({ pathname: 'my/path', headers: { 'Content-Type': 'CustomContentType' } }); + await kfetch({ pathname: '/my/path', headers: { 'Content-Type': 'CustomContentType' } }); expect(fetchMock.lastOptions()!.headers).toMatchObject({ 'Content-Type': 'CustomContentType', }); @@ -60,73 +69,82 @@ describe('kfetch', () => { it('should use supplied pathname and querystring', async () => { fetchMock.get('*', {}); - await kfetch({ pathname: 'my/path', query: { a: 'b' } }); + await kfetch({ pathname: '/my/path', query: { a: 'b' } }); expect(fetchMock.lastUrl()).toBe('http://localhost/myBase/my/path?a=b'); }); it('should use supplied headers', async () => { fetchMock.get('*', {}); await kfetch({ - pathname: 'my/path', + pathname: '/my/path', headers: { myHeader: 'foo' }, }); expect(fetchMock.lastOptions()!.headers).toEqual({ 'Content-Type': 'application/json', - 'kbn-version': 'my-version', + 'kbn-version': 'kibanaVersion', myHeader: 'foo', }); }); it('should return response', async () => { fetchMock.get('*', { foo: 'bar' }); - const res = await kfetch({ pathname: 'my/path' }); + const res = await kfetch({ pathname: '/my/path' }); expect(res).toEqual({ foo: 'bar' }); }); it('should prepend url with basepath by default', async () => { fetchMock.get('*', {}); - await kfetch({ pathname: 'my/path' }); + await kfetch({ pathname: '/my/path' }); expect(fetchMock.lastUrl()).toBe('http://localhost/myBase/my/path'); }); it('should not prepend url with basepath when disabled', async () => { fetchMock.get('*', {}); - await kfetch({ pathname: 'my/path' }, { prependBasePath: false }); + 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' }); + await kfetch({ pathname: '/my/path' }); - expect(fetchMock.lastOptions()!).toEqual({ + expect(fetchMock.lastOptions()!).toMatchObject({ method: 'GET', credentials: 'same-origin', headers: { 'Content-Type': 'application/json', - 'kbn-version': 'my-version', + 'kbn-version': 'kibanaVersion', }, }); }); it('should reject on network error', async () => { expect.assertions(1); - fetchMock.get('*', { throws: new Error('Network issue') }); + fetchMock.get('*', { status: 500 }); try { - await kfetch({ pathname: 'my/path' }); + await kfetch({ pathname: '/my/path' }); } catch (e) { - expect(e.message).toBe('Network issue'); + expect(e.message).toBe('Internal Server Error'); } }); + it('should return an abortable promise', () => { + const abortable = kfetch({ pathname: '/my/path' }); + + expect(typeof abortable.then).toBe('function'); + expect(typeof abortable.catch).toBe('function'); + expect(typeof abortable.abort).toBe('function'); + expect(abortable.abort()).toBe(abortable); + }); + 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' }); + await kfetch({ pathname: '/my/path' }); } catch (e) { error = e; } @@ -154,7 +172,7 @@ describe('kfetch', () => { fetchMock.get('*', { foo: 'bar' }); interceptorCalls = mockInterceptorCalls([{}, {}, {}]); - resp = await kfetch({ pathname: 'my/path' }); + resp = await kfetch({ pathname: '/my/path' }); }); it('should call interceptors in correct order', () => { @@ -185,12 +203,12 @@ describe('kfetch', () => { fetchMock.get('*', { foo: 'bar' }); interceptorCalls = mockInterceptorCalls([ - { requestError: () => ({}) }, + { requestError: () => ({ pathname: '/my/path' } as KFetchOptions) }, { request: () => Promise.reject(new Error('Error in request')) }, {}, ]); - resp = await kfetch({ pathname: 'my/path' }); + resp = await kfetch({ pathname: '/my/path' }); }); it('should call interceptors in correct order', () => { @@ -227,7 +245,7 @@ describe('kfetch', () => { ]); try { - await kfetch({ pathname: 'my/path' }); + await kfetch({ pathname: '/my/path' }); } catch (e) { error = e; } @@ -267,7 +285,7 @@ describe('kfetch', () => { ]); try { - await kfetch({ pathname: 'my/path' }); + await kfetch({ pathname: '/my/path' }); } catch (e) { error = e; } @@ -313,7 +331,7 @@ describe('kfetch', () => { {}, ]); - resp = await kfetch({ pathname: 'my/path' }); + resp = await kfetch({ pathname: '/my/path' }); }); it('should call in correct order', () => { @@ -351,7 +369,7 @@ describe('kfetch', () => { }), }); - resp = await kfetch({ pathname: 'my/path' }); + resp = await kfetch({ pathname: '/my/path' }); }); it('should modify request', () => { @@ -386,7 +404,7 @@ describe('kfetch', () => { }), }); - resp = await kfetch({ pathname: 'my/path' }); + resp = await kfetch({ pathname: '/my/path' }); }); it('should modify request', () => { @@ -453,6 +471,7 @@ function mockInterceptorCalls(interceptors: Interceptor[]) { describe('withDefaultOptions', () => { it('should remove undefined query params', () => { const { query } = withDefaultOptions({ + pathname: '/withDefaultOptions', query: { foo: 'bar', param1: (undefined as any) as string, @@ -464,9 +483,10 @@ describe('withDefaultOptions', () => { }); it('should add default options', () => { - expect(withDefaultOptions({})).toEqual({ + expect(withDefaultOptions({ pathname: '/addDefaultOptions' })).toEqual({ + pathname: '/addDefaultOptions', credentials: 'same-origin', - headers: { 'Content-Type': 'application/json', 'kbn-version': 'my-version' }, + headers: { 'Content-Type': 'application/json' }, method: 'GET', }); }); diff --git a/src/legacy/ui/public/kfetch/kfetch.ts b/src/legacy/ui/public/kfetch/kfetch.ts index 93d736bd8666eda..21690ec3d2bd332 100644 --- a/src/legacy/ui/public/kfetch/kfetch.ts +++ b/src/legacy/ui/public/kfetch/kfetch.ts @@ -19,17 +19,16 @@ 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'; +import { HttpSetup } from '../../../../core/public'; + export interface KFetchQuery { [key: string]: string | number | boolean | undefined; } export interface KFetchOptions extends RequestInit { - pathname?: string; + pathname: string; query?: KFetchQuery; } @@ -48,32 +47,36 @@ 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 window.fetch(fullUrl, restOptions).then(async res => { - if (!res.ok) { - throw new KFetchError(res, await getBodyAsJson(res)); - } - const contentType = res.headers.get('content-type'); - if (contentType && contentType.split(';')[0] === 'application/ndjson') { - return await getBodyAsBlob(res); - } - return await getBodyAsJson(res); - }); - } - ); +export function createKfetch(http: HttpSetup) { + return function kfetch( + options: KFetchOptions, + { prependBasePath = true }: KFetchKibanaOptions = {} + ) { + const controller = new AbortController(); + const promise = new Promise((resolve, reject) => { + responseInterceptors( + requestInterceptors(withDefaultOptions(options)) + .then(({ pathname, query, ...restOptions }) => + http.fetch(pathname, { + ...restOptions, + signal: controller.signal, + query, + prependBasePath, + }) + ) + .catch(err => { + throw new KFetchError(err.response || { statusText: err.message }, err.body); + }) + ).then(resolve, reject); + }); - return responseInterceptors(promise); + return Object.assign(promise, { + abort() { + controller.abort(); + return promise; + }, + }); + }; } // Request/response interceptors are called in opposite orders. @@ -91,22 +94,6 @@ function responseInterceptors(responsePromise: Promise) { }, responsePromise); } -async function getBodyAsJson(res: Response) { - try { - return await res.json(); - } catch (e) { - return null; - } -} - -async function getBodyAsBlob(res: Response) { - try { - return await res.blob(); - } catch (e) { - return null; - } -} - export function withDefaultOptions(options?: KFetchOptions): KFetchOptions { return merge( { @@ -118,7 +105,6 @@ export function withDefaultOptions(options?: KFetchOptions): KFetchOptions { : { 'Content-Type': 'application/json', }), - 'kbn-version': metadata.version, }, }, options diff --git a/x-pack/plugins/rollup/public/search/rollup_search_strategy.js b/x-pack/plugins/rollup/public/search/rollup_search_strategy.js index 826292c67b0a401..f996570e8bc4daf 100644 --- a/x-pack/plugins/rollup/public/search/rollup_search_strategy.js +++ b/x-pack/plugins/rollup/public/search/rollup_search_strategy.js @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { kfetchAbortable } from 'ui/kfetch'; +import { kfetch } from 'ui/kfetch'; import { SearchError, getSearchErrorType } from 'ui/courier'; function getAllFetchParams(searchRequests, Promise) { @@ -95,10 +95,7 @@ export const rollupSearchStrategy = { failedSearchRequests, } = await serializeAllFetchParams(allFetchParams, searchRequests); - const { - fetching, - abort, - } = kfetchAbortable({ + const abortable = kfetch({ pathname: '../api/rollup/search', method: 'POST', body: serializedFetchParams, @@ -106,7 +103,7 @@ export const rollupSearchStrategy = { return { searching: new Promise((resolve, reject) => { - fetching.then(result => { + abortable.then(result => { resolve(shimHitsInFetchResponse(result)); }).catch(error => { const { @@ -126,7 +123,7 @@ export const rollupSearchStrategy = { reject(searchError); }); }), - abort, + abort: abortable.abort, failedSearchRequests, }; },