urql icon indicating copy to clipboard operation
urql copied to clipboard

urql not parsing response if `payload` is present in the first element (apollo federation)

Open SkArchon opened this issue 9 months ago • 3 comments

Describe the bug

While testing our backend with urql, we noticed that the urql code seemed to work only if the first element did not contain payload. (see attached screenshot). This is related to subscriptions when using multipart.

Image

It seems that after result is not null, payload can be used as the root element

Image

Related To: https://github.com/urql-graphql/urql/pull/3499

Reproduction client

const { Client, cacheExchange, fetchExchange } = require('@urql/core');

const client = new Client({
  url: 'http://localhost:3002/graphql',
  exchanges: [
    cacheExchange, fetchExchange
  ],
  fetchSubscriptions: true,
  fetchOptions: () => {
    return {
      headers: {
        accept: 'multipart/mixed;boundary="graphql"'
      },
    };
  },
});

async function query() {
  const QUERY = `
     subscription Test {
    countEmp2(max: 5, intervalMilliseconds: 1000)
  }
  `;
  const { unsubscribe } = client.subscription(QUERY).subscribe(result => {
    result.data
    console.log(result.data); // { data: ... }
  });
}

query();

Reproduction

Reproduction Is In Description, don't have a backend available to share atm

Urql version

@urql/core, 5.1.1

Validations

  • [x] I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • [x] Read the docs.
  • [x] Follow our Code of Conduct

SkArchon avatar Mar 12 '25 10:03 SkArchon

payload as a property on response is not valid when it comes to the GraphQL specification https://spec.graphql.org/October2021/#sec-Response-Format - it's likely that you're reaching out to a sub-graph without a supergraph in between which could lead to this discrepancy. After reading your code, you seem to allude only to subscriptions, which support for was added in https://github.com/urql-graphql/urql/pull/3499 feel free to make a reproduction test of the failing case as a PR. This issue does not contain a reproduction so...

JoviDeCroock avatar Mar 12 '25 11:03 JoviDeCroock

Hey yes, apologies. I forgot to explicitly mention it is related to when using subscriptions with multipart. I will try and get a reproduction out sometime next week.

It is just that the first response fails when it is wrapped with "payload" for multipart, however after the first response urql can handle the response when it is wrapped with "payload".

SkArchon avatar Mar 14 '25 09:03 SkArchon

You can probably reproduce this in our unit tests, if that's more convenient: https://github.com/urql-graphql/urql/blob/e850c988aaa0dc4e0707dbb589c4bf65bc0b473f/packages/core/src/utils/result.test.ts

This doesn't have much to do with the multipart transport protocol, but instead with the "Incremental Delivery" spec that defines how incremental (defer/stream) results are formatted and delivered to clients.

None of the spec, as it's currently been submitted, requires the payload wrapper, and I think, if I recall correctly, that must either be an old version of the spec, or a transport wrapping payload that's not specified. The whole situation is confusing and frustrating as there's multiple versions of the Incremental Delivery proposal spec, it's been changed without a history, implemented without versions/references to specs, there's no exact spec docs for some versions, etc etc.

I suspect that Apollo has basically stuck to one version on the server (which likely differs, for example, what GraphQL Yoga stuck to).

You can specifically see that the payload property is often Federation specific: https://github.com/urql-graphql/urql/pull/3499 There's a lot of references to (result.payload || result).data for example. It's possible changes have been made that we're not aware of or that there's a specific condition that the result transformation doesn't account for.

But if this can't be traced back to the Apollo Federation spec-addition, then it'd be best to capture what the result format is from your actual server, compare that to the tests, track the changes, and document the failure/difference

kitten avatar Mar 15 '25 14:03 kitten