Expose an HTTP-request browser client #35486

eliperelman · 2019-04-23T16:38:32Z

Summary

This patch exposes a browser client for simplifying HTTP requests. The long-term goal of this service/client is to replace the legacy kfetch service. This patch also swaps out the internals of kfetch for this new service, meaning legacy and NP plugins are both using this.

From a sat up HttpService, there are now 5 additional methods:

http.fetch(path, options): This has an API comparable to window.fetch with the addition of a few custom options, status code error handling, default HTTP header additions, and automatic body parsing resolution for JSON and NDJSON.
http.get(path, options): The GET-enforced shorthand for http.fetch().
http.post(path, options): The POST-enforced shorthand for http.fetch().
http.put(path, options): The PUT-enforced shorthand for http.fetch().
http.delete(path, options): The DELETE-enforced shorthand for http.fetch().
http.patch(path, options): The PATCH-enforced shorthand for http.fetch().

Fixes #18855.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~This was checked for cross-browser compatibility, including a check against IE11~~
~~Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
~~This was checked for keyboard-only and screenreader accessibility~~

For maintainers

This was checked for breaking API changes and was labeled appropriately
This includes a feature addition or change that requires a release note and was labeled appropriately

Dev Docs

The HTTP browser client now exposes methods for simplifying HTTP requests. The long-term goal of this service/client is to replace the legacy kfetch service. This also swaps out the internals of kfetch for this new service, meaning legacy and new platform plugins are both backed by this functionality.

From a sat up HttpService, there are now 5 additional methods:

http.fetch(path, options): This has an API comparable to window.fetch with the addition of a few custom options, status code error handling, default HTTP header additions, and automatic body parsing resolution for JSON and NDJSON.
http.get(path, options): The GET-enforced shorthand for http.fetch().
http.post(path, options): The POST-enforced shorthand for http.fetch().
http.put(path, options): The PUT-enforced shorthand for http.fetch().
http.delete(path, options): The DELETE-enforced shorthand for http.fetch().
http.patch(path, options): The PATCH-enforced shorthand for http.fetch().

elasticmachine · 2019-04-23T16:38:35Z

Pinging @elastic/kibana-platform

elasticmachine · 2019-04-23T16:54:18Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-23T20:25:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-24T17:19:14Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-24T17:48:55Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mshustov · 2019-04-25T12:48:33Z

src/core/public/http/http_service.mock.ts

+  getLoadingCount$: jest.fn(),
+});
+const createMock = (): jest.Mocked<PublicMethodsOf<HttpService>> => ({
+  setup: jest.fn().mockReturnValue(createSetupContractMock()),


I didn't use it because when you call .mockReturnValue(createSetupContractMock()), typescript doesn't infer type to HttpService.setup yet.
you can pass .mockReturnValue({}), and it won't complain.

I didn't use it

Could you elaborate on what you mean by this?

that's why I prefer next form:

// here we infer type manually const startContract: StartContract = { setup: jest.fn() } // now we have proper validation startContract.mockReturnValue(..)

This is currently inconsistent with the way the rest of the client-side mocks work, so I'm going to leave the current form. We can re-evaluate this again in the future.

Ah, now I know why you did it this way, @restrry. I think having the type-safety is preferable and we should probably change the ones I had changed back.

src/core/public/http/http_service.ts

src/legacy/ui/public/kfetch/kfetch.ts

src/core/public/http/http_service.test.ts

src/legacy/ui/public/kfetch/kfetch.ts

tylersmalley · 2019-04-25T20:08:51Z

@mistic, would you mind taking a look at this error? Additionally, we need to come up with a way for folks to be able to run the typechecks performed in the build task locally. Currently, it's isolated to the build.

elasticmachine · 2019-04-25T20:11:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

eliperelman · 2019-04-25T21:08:27Z

According to @spalger this was due to kfetch_test_setup.ts being included in the build when it shouldn't have been. I resolved that particular issue by moving it to test_utils.

elasticmachine · 2019-04-25T21:15:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

mistic · 2019-04-26T16:54:17Z

During the build the typecheck we did against the built assets are basically tsc --noEmit true --pretty true --project ${KIBANA_BASE_DIR}/build/kibana-oss/tsconfig.browser.json which is basically the same as we did with node scripts/type_check with the different that the first one is applied against the built assets. I think the problem here was the one highlighted by @spalger

elasticmachine · 2019-04-26T18:37:49Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-30T16:22:52Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

eliperelman · 2019-04-30T16:23:22Z

retest

elasticmachine · 2019-04-30T17:27:30Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-04-30T20:37:50Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-05-08T15:55:55Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request

* Expose an HTTP-request browser client * Fix failing tests from kfetch refactor * Make abort() non-enumerable, fix review issues * Move kfetch test setup to build-excluded location * Add ndjson tests to browser http service * Lint fixes * Fix missing update of del to delete in http mock * Fix problems with merging headers with undefined Content-Type * Delete correct property from updated options * Linting fix * Fix reference to kfetch_test_setup due to moving test file * Add tests and fix implementation of abortables * Add missing http start mock contract, fix test in CI * Remove abortable promise functionality * Fix DELETE method handler, remove unnecessary promise wrapper

elasticmachine · 2019-05-22T15:29:16Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

elasticmachine · 2019-06-04T22:51:37Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request

eliperelman added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform v8.0.0 v7.2.0 labels Apr 23, 2019

eliperelman requested a review from a team as a code owner April 23, 2019 16:38

eliperelman self-assigned this Apr 23, 2019

eliperelman force-pushed the browser-http-service branch 2 times, most recently from 838b2b2 to fff35e0 Compare April 23, 2019 19:55

eliperelman force-pushed the browser-http-service branch 2 times, most recently from ab8fe91 to 32af0ba Compare April 24, 2019 16:27

mshustov reviewed Apr 25, 2019

View reviewed changes

eliperelman force-pushed the browser-http-service branch from a02bda7 to 0e86ce1 Compare April 25, 2019 19:40

eliperelman force-pushed the browser-http-service branch from 6fe3a69 to e7c48ff Compare April 30, 2019 16:13

eliperelman force-pushed the browser-http-service branch from e7c48ff to b2e04a4 Compare April 30, 2019 19:45

eliperelman added 14 commits May 8, 2019 09:30

Fix failing tests from kfetch refactor

941db30

Make abort() non-enumerable, fix review issues

ab4aa06

Move kfetch test setup to build-excluded location

233ae67

Add ndjson tests to browser http service

d9ba9ee

Lint fixes

7bd98c9

Fix missing update of del to delete in http mock

568b6d2

Fix problems with merging headers with undefined Content-Type

4dd708e

Delete correct property from updated options

1626c45

Linting fix

a6b30b8

Fix reference to kfetch_test_setup due to moving test file

42cc816

Add tests and fix implementation of abortables

e9ae8ed

Add missing http start mock contract, fix test in CI

dc98cd9

Remove abortable promise functionality

a10236f

Fix DELETE method handler, remove unnecessary promise wrapper

e3db5be

eliperelman force-pushed the browser-http-service branch from 51b1883 to e3db5be Compare May 8, 2019 14:40

eliperelman merged commit 95a3ceb into elastic:master May 8, 2019

rudolf mentioned this pull request May 9, 2019

[7.x] Fix core api type exports (#36108) #36332

Merged

joshdover mentioned this pull request May 9, 2019

Capabilities endpoint and authentication #36319

Closed

eliperelman mentioned this pull request May 9, 2019

[7.x] Expose an HTTP-request browser client (#35486) #36397

Merged

rudolf mentioned this pull request Jul 1, 2019

Create basic window.fetch() abstraction #18744

Closed

jfsiii mentioned this pull request Sep 9, 2019

[Discuss] Add NP core.http.route #44174

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose an HTTP-request browser client #35486

Expose an HTTP-request browser client #35486

eliperelman commented Apr 23, 2019 •

edited by timroes

Loading

elasticmachine commented Apr 23, 2019

elasticmachine commented Apr 23, 2019

elasticmachine commented Apr 23, 2019

elasticmachine commented Apr 24, 2019

elasticmachine commented Apr 24, 2019

mshustov Apr 25, 2019

eliperelman Apr 25, 2019

mshustov Apr 26, 2019 •

edited

Loading

eliperelman Apr 30, 2019

joshdover May 3, 2019

tylersmalley commented Apr 25, 2019

elasticmachine commented Apr 25, 2019

eliperelman commented Apr 25, 2019

elasticmachine commented Apr 25, 2019

mistic commented Apr 26, 2019

elasticmachine commented Apr 26, 2019

elasticmachine commented Apr 30, 2019

eliperelman commented Apr 30, 2019

elasticmachine commented Apr 30, 2019

elasticmachine commented Apr 30, 2019

elasticmachine commented May 8, 2019

elasticmachine commented May 22, 2019

elasticmachine commented Jun 4, 2019

Expose an HTTP-request browser client #35486

Expose an HTTP-request browser client #35486

Conversation

eliperelman commented Apr 23, 2019 • edited by timroes Loading

Summary

Checklist

For maintainers

Dev Docs

elasticmachine commented Apr 23, 2019

elasticmachine commented Apr 23, 2019

💔 Build Failed

elasticmachine commented Apr 23, 2019

💔 Build Failed

elasticmachine commented Apr 24, 2019

💔 Build Failed

elasticmachine commented Apr 24, 2019

💔 Build Failed

mshustov Apr 25, 2019

Choose a reason for hiding this comment

eliperelman Apr 25, 2019

Choose a reason for hiding this comment

mshustov Apr 26, 2019 • edited Loading

Choose a reason for hiding this comment

eliperelman Apr 30, 2019

Choose a reason for hiding this comment

joshdover May 3, 2019

Choose a reason for hiding this comment

tylersmalley commented Apr 25, 2019

elasticmachine commented Apr 25, 2019

💔 Build Failed

eliperelman commented Apr 25, 2019

elasticmachine commented Apr 25, 2019

💔 Build Failed

mistic commented Apr 26, 2019

elasticmachine commented Apr 26, 2019

💔 Build Failed

elasticmachine commented Apr 30, 2019

💔 Build Failed

eliperelman commented Apr 30, 2019

elasticmachine commented Apr 30, 2019

💔 Build Failed

elasticmachine commented Apr 30, 2019

💔 Build Failed

elasticmachine commented May 8, 2019

💚 Build Succeeded

elasticmachine commented May 22, 2019

💔 Build Failed

elasticmachine commented Jun 4, 2019

💔 Build Failed

eliperelman commented Apr 23, 2019 •

edited by timroes

Loading

mshustov Apr 26, 2019 •

edited

Loading