Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interceptor parity between $http and kfetch #22594

Closed
4 tasks
cjcenizal opened this issue Aug 31, 2018 · 8 comments
Closed
4 tasks

Interceptor parity between $http and kfetch #22594

cjcenizal opened this issue Aug 31, 2018 · 8 comments
Assignees
Labels
chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@cjcenizal
Copy link
Contributor

cjcenizal commented Aug 31, 2018

Goal

Consumers of kfetch will assume it's an on-par subttitute for $http. However, this is not the case because several parts of our codebase add interceptors for $http. We need to update all the places where we consume $httpProvider.interceptors to add the same interceptors for kfetch and then test thoroughly to make sure they all work as expected, in order for people to safely migrate from $http to kfetch.

Prior discussion in #22128.

Parity example

Here's how we would update xsrf.js.

import $ from 'jquery';
import { set } from 'lodash';
import { interceptors } from 'ui/kfetch';

const xsrfRequestInterceptor = {
  request: function (opts) {
    const { kbnXsrfToken = true } = opts;
    if (kbnXsrfToken) {
      set(opts, ['headers', 'kbn-version'], internals.version);
    }
    return opts;
  }
};

export function initChromeXsrfApi(chrome, internals) {
  chrome.getXsrfToken = function () {
    return internals.version;
  };

  $.ajaxPrefilter(function ({ kbnXsrfToken = true }, originalOptions, jqXHR) {
    if (kbnXsrfToken) {
      jqXHR.setRequestHeader('kbn-version', internals.version);
    }
  });

  chrome.$setupXsrfRequestInterceptor = function ($httpProvider) {
    // Ensure all requests go through the interceptor, regardless of which fetch module is used.
    $httpProvider.interceptors.push(xsrfRequestInterceptor);
    interceptors.push(xsrfRequestInterceptor);
  };
}
@cjcenizal cjcenizal added chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Aug 31, 2018
@cjcenizal cjcenizal self-assigned this Aug 31, 2018
@epixa
Copy link
Contributor

epixa commented Aug 31, 2018

@elastic/kibana-platform

@spalger
Copy link
Contributor

spalger commented Aug 31, 2018

Only issue I have with that example is that the interceptor is only added to kfetch when we are using angular.

@cjcenizal
Copy link
Contributor Author

@spalger Are there cases where $setupXsrfRequestInterceptor won't be called? It looks like chrome.js will eventually call this as part of its natural bootstrap process, beginning at https://github.com/elastic/kibana/blob/master/src/ui/public/chrome/chrome.js#L82. Or are you just thinking ahead to the day when we remove Angular?

@spalger
Copy link
Contributor

spalger commented Aug 31, 2018

Oh, true, we don't use things like routing in plugins like APM but we do still setup angular. No reason to couple it to angular right now, but yeah, it should work fine.

@eliperelman
Copy link
Contributor

With kfetch now being backed by the new platform http service, we should move the interceptor functionality into http. @cjcenizal I can take this on since I'm pretty familiar with each of their internals now.

@cjcenizal
Copy link
Contributor Author

@eliperelman FYI I created #22890 to migrate on_session_timeout.js but the work stalled because I kept running into bugs. Feel free to steal from that PR if you find it useful or ignore it if not. Either way, I'll leave it open until you put up a replacement PR.

@cjcenizal
Copy link
Contributor Author

@joshdover Can this issue be closed now?

@joshdover
Copy link
Contributor

Yes, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

No branches or pull requests

5 participants