From 4e3f2dc8556f8aa98fd148d7add5c97e1c38e0ba Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Wed, 11 Jul 2018 16:40:39 -0700 Subject: [PATCH 1/4] Add ability to abort a kfetch call. --- src/ui/public/kfetch/index.js | 33 ++++++++++++++++++++++++------ src/ui/public/kfetch/index.test.js | 9 ++++++++ 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/ui/public/kfetch/index.js b/src/ui/public/kfetch/index.js index 65efaf95e071d..a3e648d32067f 100644 --- a/src/ui/public/kfetch/index.js +++ b/src/ui/public/kfetch/index.js @@ -31,7 +31,16 @@ class FetchError extends Error { } } -export async function kfetch(fetchOptions, kibanaOptions) { +export function kfetch(fetchOptions, kibanaOptions, isAbortable = false) { + let signal; + let abort; + + if (isAbortable) { + const abortController = new AbortController(); + signal = abortController.signal; + abort = abortController.abort.bind(abortController); + } + // fetch specific options with defaults const { pathname, query, ...combinedFetchOptions } = merge( { @@ -41,8 +50,9 @@ export async function kfetch(fetchOptions, kibanaOptions) { 'Content-Type': 'application/json', 'kbn-version': metadata.version, }, + signal, }, - fetchOptions + fetchOptions, ); // kibana specific options with defaults @@ -56,11 +66,22 @@ export async function kfetch(fetchOptions, kibanaOptions) { query, }); - const res = await fetch(fullUrl, combinedFetchOptions); + const fetching = new Promise(async (resolve, reject) => { + const res = await fetch(fullUrl, combinedFetchOptions); + + if (!res.ok) { + reject(new FetchError(res)); + } + + resolve(res.json()); + }); - if (!res.ok) { - throw new FetchError(res); + if (isAbortable) { + return { + fetching, + abort, + }; } - return res.json(); + return fetching; } diff --git a/src/ui/public/kfetch/index.test.js b/src/ui/public/kfetch/index.test.js index 9b0ffae7c6ab6..73691317f9506 100644 --- a/src/ui/public/kfetch/index.test.js +++ b/src/ui/public/kfetch/index.test.js @@ -114,4 +114,13 @@ describe('kfetch', () => { }); }); }); + + describe('isAbortable', () => { + it('should return an object with a fetching promise and an abort callback', () => { + const { fetching, abort } = kfetch({ pathname: 'my/path' }, {}, true); + expect(typeof fetching.then).toBe('function'); + expect(typeof fetching.catch).toBe('function'); + expect(typeof abort).toBe('function'); + }); + }); }); From d01c26f5f76063d655769b0f33c6c54f5b4abfb1 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 12 Jul 2018 09:52:15 -0700 Subject: [PATCH 2/4] Add abortcontroller-polyfill. --- package.json | 1 + src/ui/public/chrome/chrome.js | 4 ++++ yarn.lock | 4 ++++ 3 files changed, 9 insertions(+) diff --git a/package.json b/package.json index 48e1e3459f2fa..c48f499d589a5 100644 --- a/package.json +++ b/package.json @@ -88,6 +88,7 @@ "@kbn/test-subj-selector": "link:packages/kbn-test-subj-selector", "@kbn/ui-framework": "link:packages/kbn-ui-framework", "JSONStream": "1.1.1", + "abortcontroller-polyfill": "^1.1.9", "angular": "1.6.9", "angular-aria": "1.6.6", "angular-elastic": "2.5.0", diff --git a/src/ui/public/chrome/chrome.js b/src/ui/public/chrome/chrome.js index 2042c0070eceb..54bf84fe126db 100644 --- a/src/ui/public/chrome/chrome.js +++ b/src/ui/public/chrome/chrome.js @@ -21,9 +21,13 @@ import _ from 'lodash'; import angular from 'angular'; import { metadata } from '../metadata'; + +// Polyfills import 'babel-polyfill'; import 'whatwg-fetch'; import 'custom-event-polyfill'; +import 'abortcontroller-polyfill'; + import '../state_management/global_state'; import '../config'; import '../notify'; diff --git a/yarn.lock b/yarn.lock index 348d0dafe3790..36ed0c2ab6f61 100644 --- a/yarn.lock +++ b/yarn.lock @@ -549,6 +549,10 @@ abbrev@1.0.x: version "1.0.9" resolved "https://registry.yarnpkg.com/abbrev/-/abbrev-1.0.9.tgz#91b4792588a7738c25f35dd6f63752a2f8776135" +abortcontroller-polyfill@^1.1.9: + version "1.1.9" + resolved "https://registry.yarnpkg.com/abortcontroller-polyfill/-/abortcontroller-polyfill-1.1.9.tgz#9fefe359fda2e9e0932dc85e6106453ac393b2da" + accept-language-parser@^1.5.0: version "1.5.0" resolved "https://registry.yarnpkg.com/accept-language-parser/-/accept-language-parser-1.5.0.tgz#8877c54040a8dcb59e0a07d9c1fde42298334791" From 94bf026707707c2c154610232960bef34cb6513a Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 12 Jul 2018 11:45:06 -0700 Subject: [PATCH 3/4] Split kfetch module into kfetch and kfetchAbortable sub-modules. --- src/ui/public/kfetch/index.js | 77 +------------------ src/ui/public/kfetch/kfetch.js | 77 +++++++++++++++++++ .../kfetch/{index.test.js => kfetch.test.js} | 12 +-- src/ui/public/kfetch/kfetch_abortable.js | 40 ++++++++++ src/ui/public/kfetch/kfetch_abortable.test.js | 39 ++++++++++ 5 files changed, 160 insertions(+), 85 deletions(-) create mode 100644 src/ui/public/kfetch/kfetch.js rename src/ui/public/kfetch/{index.test.js => kfetch.test.js} (89%) create mode 100644 src/ui/public/kfetch/kfetch_abortable.js create mode 100644 src/ui/public/kfetch/kfetch_abortable.test.js diff --git a/src/ui/public/kfetch/index.js b/src/ui/public/kfetch/index.js index e6f0ff4bce631..ab1cadf0c2371 100644 --- a/src/ui/public/kfetch/index.js +++ b/src/ui/public/kfetch/index.js @@ -17,78 +17,5 @@ * under the License. */ -import 'isomorphic-fetch'; -import url from 'url'; -import chrome from '../chrome'; -import { metadata } from '../metadata'; -import { merge } from 'lodash'; - -class FetchError extends Error { - constructor(res, body) { - super(res.statusText); - this.res = res; - this.body = body; - Error.captureStackTrace(this, FetchError); - } -} - -export function kfetch(fetchOptions, kibanaOptions, isAbortable = false) { - let signal; - let abort; - - if (isAbortable) { - const abortController = new AbortController(); - signal = abortController.signal; - abort = abortController.abort.bind(abortController); - } - - // fetch specific options with defaults - const { pathname, query, ...combinedFetchOptions } = merge( - { - method: 'GET', - credentials: 'same-origin', - headers: { - 'Content-Type': 'application/json', - 'kbn-version': metadata.version, - }, - signal, - }, - fetchOptions, - ); - - // 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) { - let body; - try { - body = await res.json(); - } catch (err) { - // ignore error, may not be able to get body for response that is not ok - } - reject(new FetchError(res, body)); - } - - resolve(res.json()); - }); - - if (isAbortable) { - return { - fetching, - abort, - }; - } - - return fetching; -} +export { kfetch } from './kfetch'; +export { kfetchAbortable } from './kfetch_abortable'; diff --git a/src/ui/public/kfetch/kfetch.js b/src/ui/public/kfetch/kfetch.js new file mode 100644 index 0000000000000..9de431f9e8b7b --- /dev/null +++ b/src/ui/public/kfetch/kfetch.js @@ -0,0 +1,77 @@ +/* + * 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 'isomorphic-fetch'; +import url from 'url'; +import chrome from '../chrome'; +import { metadata } from '../metadata'; +import { merge } from 'lodash'; + +class FetchError extends Error { + constructor(res, body) { + super(res.statusText); + this.res = res; + this.body = body; + Error.captureStackTrace(this, FetchError); + } +} + +export function kfetch(fetchOptions, kibanaOptions) { + // fetch specific options with defaults + const { pathname, query, ...combinedFetchOptions } = merge( + { + method: 'GET', + credentials: 'same-origin', + headers: { + 'Content-Type': 'application/json', + 'kbn-version': metadata.version, + }, + }, + fetchOptions, + ); + + // 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) { + let body; + try { + body = await res.json(); + } catch (err) { + // ignore error, may not be able to get body for response that is not ok + } + reject(new FetchError(res, body)); + } + + resolve(res.json()); + }); + + return fetching; +} diff --git a/src/ui/public/kfetch/index.test.js b/src/ui/public/kfetch/kfetch.test.js similarity index 89% rename from src/ui/public/kfetch/index.test.js rename to src/ui/public/kfetch/kfetch.test.js index 73691317f9506..0bd9f28e8bcd6 100644 --- a/src/ui/public/kfetch/index.test.js +++ b/src/ui/public/kfetch/kfetch.test.js @@ -18,11 +18,12 @@ */ import fetchMock from 'fetch-mock'; -import { kfetch } from './index'; +import { kfetch } from './kfetch'; jest.mock('../chrome', () => ({ addBasePath: path => `myBase/${path}`, })); + jest.mock('../metadata', () => ({ metadata: { version: 'my-version', @@ -114,13 +115,4 @@ describe('kfetch', () => { }); }); }); - - describe('isAbortable', () => { - it('should return an object with a fetching promise and an abort callback', () => { - const { fetching, abort } = kfetch({ pathname: 'my/path' }, {}, true); - expect(typeof fetching.then).toBe('function'); - expect(typeof fetching.catch).toBe('function'); - expect(typeof abort).toBe('function'); - }); - }); }); diff --git a/src/ui/public/kfetch/kfetch_abortable.js b/src/ui/public/kfetch/kfetch_abortable.js new file mode 100644 index 0000000000000..8bd732d2ab5f6 --- /dev/null +++ b/src/ui/public/kfetch/kfetch_abortable.js @@ -0,0 +1,40 @@ +/* + * 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 { kfetch } from './kfetch'; + +function createAbortable() { + const abortController = new AbortController(); + const { signal, abort } = abortController; + + return { + signal, + abort: abort.bind(abortController), + }; +} + +export function kfetchAbortable(fetchOptions, kibanaOptions) { + const { signal, abort } = createAbortable(); + const fetching = kfetch({ ...fetchOptions, signal }, kibanaOptions); + + return { + fetching, + abort, + }; +} diff --git a/src/ui/public/kfetch/kfetch_abortable.test.js b/src/ui/public/kfetch/kfetch_abortable.test.js new file mode 100644 index 0000000000000..053f231b0f965 --- /dev/null +++ b/src/ui/public/kfetch/kfetch_abortable.test.js @@ -0,0 +1,39 @@ +/* + * 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 { kfetchAbortable } from './kfetch_abortable'; + +jest.mock('../chrome', () => ({ + addBasePath: path => `myBase/${path}`, +})); + +jest.mock('../metadata', () => ({ + metadata: { + version: 'my-version', + }, +})); + +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'); + }); +}); From 14228d88b13507d4a61b3c14d5c7a541d77b3f60 Mon Sep 17 00:00:00 2001 From: CJ Cenizal Date: Thu, 12 Jul 2018 13:31:59 -0700 Subject: [PATCH 4/4] Fix bug. --- src/ui/public/kfetch/kfetch.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/public/kfetch/kfetch.js b/src/ui/public/kfetch/kfetch.js index 9de431f9e8b7b..af9c849f88c62 100644 --- a/src/ui/public/kfetch/kfetch.js +++ b/src/ui/public/kfetch/kfetch.js @@ -67,7 +67,7 @@ export function kfetch(fetchOptions, kibanaOptions) { } catch (err) { // ignore error, may not be able to get body for response that is not ok } - reject(new FetchError(res, body)); + return reject(new FetchError(res, body)); } resolve(res.json());