Add asResponse option to HttpService methods

[joshdover]

Summary

This refactors the client-side HTTP service with a few minor changes:

• Pulls apart different parts of the logic into multiple files
• Adds an optional type parameter to fetch methods to support typesafe response bodies
• Adds a asResponse option to return a more detailed response object rather than just the response body.

This refactoring does not change any tests in order to be sure that the functionality was not broken. In the future, tests could be broken out for different parts of the functionality (interceptors, fetching, etc.).

In order to support the asResponse option without breaking the API, I had to overload the HttpHandler type. This causes some unexpected type errors with Jest mocks as it does not recognize the return type can be two different types. If anyone has an ideas on how to fix this, please let me know, but it does not appear to be supported well.

This is not so much a problem for any code that uses the built in mocks that Core provides. I only had to manually cast in a few spots where custom mocks were used.

This change paves the way for #43970 where we can add methods to the IHttpResponse interface for adding Kibana-specific functionality to responses.

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

[elasticmachine]

Pinging @elastic/kibana-platform (Team:Platform)

[joshdover]

This object does nothing useful now, but will be extended in #43970

[pgayvallet]

👍 I think this is definitely more developer friendly.

[pgayvallet]

NIT: addInterceptor is probably more appropriate. I'm expecting an intercept method on a fetch service to actually intercept a request.

[joshdover]

Agreed. This is named as such to be compatible with the ui/kfetch module as a drop in replacement. However, I think we could change this one easily since it's only used by a handful of plugins. Let me do that in a separate PR to keep the potential impact of this initial refactor minimal.

[mshustov]

Do we still need it? I suspect it's used in tests only. I'd prefer not to make it a part of the public API and remove from http_service contract. Not a blocker for the current PR.

[joshdover]

Correct, it's only used by tests at this time. I'd like to move the detailed interceptor tests out of the overall HttpService tests and into a test that only exercises this class. At that point, we can remove this completely from the public API.

However, I wanted to make sure not to change the tests too much in this initial refactor so as to avoid regressions.

[mshustov]

However, I wanted to make sure not to change the tests too much in this initial refactor so as to avoid regressions.

💯

[elasticmachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 55bc0e1

History

• 💔 Build #14419 failed 51b855aede8127efea3e4aecfe0a36b74e02764c
• 💔 Build #14368 failed 1afa08637a077da08343895a6b1e8d1126c4740b
• 💔 Build #14026 failed 02a500642559425ca5d7c745a2e7382f95289489

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream