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

RTK Query mutation/POST request headers are missing #2614

Open
lucasgarfield opened this issue Aug 16, 2023 · 3 comments
Open

RTK Query mutation/POST request headers are missing #2614

lucasgarfield opened this issue Aug 16, 2023 · 3 comments

Comments

@lucasgarfield
Copy link
Contributor

I am not sure whether the issue I am encountering is caused by insights-chrome or not, but it seems to be very similar to issue #2461. This is related to https://github.com/RedHatInsights/image-builder-frontend. I am using the library RTK Query to make requests to other services on console.redhat.com.

My RTK Query mutation hook (which is their terminology for a hook that makes a POST request) is missing the "content-type": "application/json" header. Our backend rejects the request as it is missing this header.

RTK Query mutation's use a small wrapper around fetch() and I am wondering whether insights-chrome has a polyfill for fetch that is somehow causing the headers to get dropped/lost. This has been to known to happen before.***

I noticed reviewing #2465 there seems to be a wrapper around fetch... specifically here: 13f3a2f#diff-1a0b511d1eb317649220c672597fd6a2ab6e384023efde9f4ccf05d30fe4a2b8R106

Do all fetch requests use this wrapper? If the answer is no - are there any other wrappers around fetch?

If this wrapper (or another wrapper) is affecting fetch(), is it possible that it is causing the missing headers? The first argument in the chrome-insights fetch wrapper I found is a string, but I believe that the RTK Query mutation expects for the first argument to be a Request class instance. This is in the fetch specification: https://fetch.spec.whatwg.org/#requestinfo

I would be happy to look into this issue a bit more on my own but I need some guidance at this point on how to continue. Thanks!

*** Here is an example of a similar issue, the bug was fixed in this PR after some back and forth with the RTK Query maintainer:
supertokens/supertokens-website#116

@Hyperkid123
Copy link
Contributor

RTK Query mutation's use a small wrapper around fetch() and I am wondering whether insights-chrome has a polyfill for fetch that is somehow causing the headers to get dropped/lost. This has been to known to happen before.***

The wrapper is most likely the reason why the headers are not being added. Chrome is using the native version of fetch. We will not adopt any polyfill or any other implementation of fetch because it can cause issues for the rest of the platform.

You will have to add the auth header yourself to the RTK fetch wrapper.

@lucasgarfield
Copy link
Contributor Author

I'm certainly not asking for insights-chrome to adopt a polyfill or other implementation of fetch just for me... that would really be outrageous. 😅

Rather, I believe that the fetch wrapper in insights-chrome is dropping the headers for the reasons that the RTK Query maintainer describes in this comment:
supertokens/supertokens-website#116 (comment)

In iqeEnablements.ts there is the following code:

  window.fetch = function fetchReplacement(path = '', options, ...rest) {
    const tid = Math.random().toString(36);
    const additionalHeaders: any = spreadAdditionalHeaders(options);

    const prom = oldFetch.apply(this, [
      path,
      {
        ...(options || {}),
        headers: {
          // If app wants to set its own Auth header it can do so
          ...(checkOrigin(path) && libJwt?.()?.jwt.isAuthenticated() && { Authorization: `Bearer ${libJwt?.()?.jwt.getEncodedToken()}` }),
          ...additionalHeaders,
        },
      },
      ...rest,
    ]);

I believe that this code does not conform to the fetch specification (https://fetch.spec.whatwg.org/#requestinfo). If path is a Request object (https://developer.mozilla.org/en-US/docs/Web/API/Request) that contains headers (which is the case for the POST request from the RTKQ mutation), then the headers provided in the second argument to oldFetch.apply() will actually overwrite them completely instead of merging them.

I haven't had time to work on a PR yet to prove this, but this is my current working theory... I will try to get a PR to confirm soon.

@lucasgarfield
Copy link
Contributor Author

I believe my theory was correct, I've managed to get the correct headers from my RTKQ mutation after implementing these changes: #2615

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants