Response to Deferred Query with Accept: multipart/mixed is application/json

[alessbell]

Describe the bug
When making a request containing a query with a @defer directive, e.g.

query DeferVariation {
  allProducts {
    delivery {
      ...MyFragment @defer
    }
    sku,
    id
  }
}

fragment MyFragment on DeliveryEstimates {
  estimatedDelivery
  fastestDelivery
}

and the request header Accept: multipart/mixed; deferSpec=20220822, application/json, the client would expect a response type of multipart/mixed, while it is receiving a response of type application/json.

This is the above query executed against apollographql/supergraph-demo-fed2#212 in Postman (NB: the default Accept: */* header is overridden by the custom Accept header at the bottom; when I uncheck the former the response is the same):

To Reproduce
There are detailed reproduction steps in the PR description jpvajda/ac-defer-example#5.

Expected behavior
If the @defer directive is issued on a fragment in the query and the client indicates it can handle multipart/mixed responses, the response will be multipart.

[glasser]

I think I see the bug

                list.any(|mime| mime.as_ref() == Ok(&multipart_mixed))

This Eq check compares to see if the types are exactly the same including parameters. You probably want to check to see if mime.essence() (which is the type without the parameters) equals multipart_mixed (and then of course look for deferSpec too).

[prasek]

adding deferSpec to smoke test 6 in PR 212 does result in the same error.

[abernix]

Thanks very much for opening this issue @alessbell. Do you mind confirming what version of the Router you were trying?

Presumably it was 1.0.0-alpha.0. It's worth noting that the Router still didn't have any awareness of deferSpec as of that version; such support is coming in #1652. So yes, what @glasser wrote is notable, but you're also sending more than we expect right now with the inclusion of deferSpec.

Anyhow, we'll take a look at this today and we'll get there soon! So close.

[glasser]

Sure, my suggestion is that in general ignoring unknown parameters is probably better than rejecting here, though perhaps you disagree.

…

On Tue, Aug 30, 2022, 11:42 PM Jesse Rosenberger ***@***.***> wrote: Thanks very much for opening this issue @alessbell <https://github.com/alessbell>. Do you mind confirming what version of the Router you were trying? *Presumably* it was 1.0.0-alpha.0. It's worth noting that the Router still didn't have any awareness of deferSpec as of that version; such support is coming in #1652 <#1652>. So yes, what @glasser <https://github.com/glasser> wrote is notable, but you're also sending *more* than we expect right now with the inclusion of deferSpec. Anyhow, we'll take a look at this today and we'll get there soon! So close. — Reply to this email directly, view it on GitHub <#1656 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAECVCJ6Y6KUJJ4NC2RMIDV335F5ANCNFSM6AAAAAAQAYWSWU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Geal]

ignoring unknown parameters should be fine yes, I'll update the filter

[Geal]

this should be fixed by #1652