Skip to content

Defer doesn't work with multipart/mixed or text/event-stream #2069

Open
Open
Defer doesn't work with multipart/mixed or text/event-stream#2069
Labels
help wantedExtra attention is needed

Description

@fredericbirke
fredericbirke
opened on Sep 5, 2023

Describe the bug

Schema:

schema {
  query: Query
}

type Query {
  loadFoo: Foo!
}

type Foo {
  name: String!
}


"The `@defer` directive may be provided for fragment spreads and inline fragments to inform the executor to delay the execution of the current fragment to indicate deprioritization of the current fragment. A query with `@defer` directive will cause the request to potentially return multiple responses, where non-deferred data is delivered in the initial response and data deferred is delivered in a subsequent response. `@include` and `@skip` take precedence over `@defer`."
directive @defer(
  "Deferred when true."
  if: Boolean,
  "If this argument label has a value other than null, it will be passed on to the result of this defer directive. This label is intended to give client applications a way to identify to which fragment a deferred result belongs to."
  label: String
) on FRAGMENT_SPREAD | INLINE_FRAGMENT

I use @graphql-codegen/typescript-apollo-angular for codegen.
When using this query:

query foos {
  loadFoo {
    ... @defer {
      name
    }
  }
}

, generate code (which works fine) and then try to run it, I get the following error:

ERROR ApolloError: Http failure during parsing for http://localhost:4200/graphql
    at new ApolloError (index.js:28:28)
    at QueryManager.js:673:19
    at both (asyncMap.js:16:53)
    at asyncMap.js:9:72
    at new ZoneAwarePromise (zone.js:1411:21)
    at Object.then (asyncMap.js:9:24)
    at Object.error (asyncMap.js:18:26)
    at notifySubscription (module.js:137:18)
    at onNotify (module.js:176:3)
    at SubscriptionObserver.error (module.js:229:5)

While digging deeper, you can find another error message:

{
    "headers": {
        "normalizedNames": {},
        "lazyUpdate": null
    },
    "status": 200,
    "statusText": "OK",
    "url": "http://localhost:4200/graphql",
    "ok": false,
    "name": "HttpErrorResponse",
    "message": "Http failure during parsing for http://localhost:4200/graphql",
    "error": {
        "error": {
           "message": "No number after minus sign in JSON at position 3"
           "stack: "SyntaxError: No number after minus sign in JSON at position 3
    at JSON.parse (<anonymous>)
    at XMLHttpRequest.onLoad (http://localhost:4200/vendor.js:22534:41)
    at _ZoneDelegate.invokeTask (http://localhost:4200/polyfills.js:8235:171)
    at http://localhost:4200/vendor.js:34143:49
    at AsyncStackTaggingZoneSpec.onInvokeTask (http://localhost:4200/vendor.js:34143:30)
    at _ZoneDelegate.invokeTask (http://localhost:4200/polyfills.js:8235:54)
    at Object.onInvokeTask (http://localhost:4200/vendor.js:34454:25)
    at _ZoneDelegate.invokeTask (http://localhost:4200/polyfills.js:8235:54)
    at Zone.runTask (http://localhost:4200/polyfills.js:8037:37)
    at ZoneTask.invokeTask [as invoke] (http://localhost:4200/polyfills.js:8312:26)"
         },
        "text": "\r\n---\r\nContent-Type: application/json; charset=utf-8\r\n\r\n{\"data\":{\"loadFoo\":{\"__typename\":\"Foo\"}},\"hasNext\":true}\r\n---\r\nContent-Type: application/json; charset=utf-8\r\n\r\n{\"incremental\":[{\"data\":{\"name\":\"Bar\",\"__typename\":\"Foo\"},\"path\":[\"loadFoo\"]}],\"hasNext\":false}\r\n-----\r\n"
    }
}

Which might make sense, because the response is incremental. The Backends response looks like this (this is the multipart/mixed format):

---
Content-Type: application/json; charset=utf-8

{"data":{"loadFoo":{"__typename":"Foo"}},"hasNext":true}
---
Content-Type: application/json; charset=utf-8

{"incremental":[{"data":{"name":"Bar","__typename":"Foo"},"path":["loadFoo"]}],"hasNext":false}
-----

If I try to use text/event-stream the backend anwers like this:

event: next
data: {"data":{"sanctuaryById":{"kennels":[{"id":"ccce9657-fec8-4ce0-9d7e-0160e9cec514","name":"Test","__typename":"Kennel"}],"__typename":"Sanctuary"}},"hasNext":true}

event: next
data: {"incremental":[{"data":{"name":"Dingens","__typename":"Sanctuary"},"path":["sanctuaryById"]}],"hasNext":false}

event: complete

but then the Apollo Error looks like this:

{
    "headers": {
        "normalizedNames": {},
        "lazyUpdate": null
    },
    "status": 200,
    "statusText": "OK",
    "url": "http://localhost:4200/graphql",
    "ok": false,
    "name": "HttpErrorResponse",
    "message": "Http failure during parsing for http://localhost:4200/graphql",
    "error": {
        "error": {
          "message": "Unexpected token 'e', "event: nex"... is not valid JSON"
          "stack":"SyntaxError: Unexpected token 'e', "event: nex"... is not valid JSON
    at JSON.parse (<anonymous>)
    at XMLHttpRequest.onLoad (http://localhost:4200/vendor.js:22534:41)
    at _ZoneDelegate.invokeTask (http://localhost:4200/polyfills.js:8235:171)
    at http://localhost:4200/vendor.js:34143:49
    at AsyncStackTaggingZoneSpec.onInvokeTask (http://localhost:4200/vendor.js:34143:30)
    at _ZoneDelegate.invokeTask (http://localhost:4200/polyfills.js:8235:54)
    at Object.onInvokeTask (http://localhost:4200/vendor.js:34454:25)
    at _ZoneDelegate.invokeTask (http://localhost:4200/polyfills.js:8235:54)
    at Zone.runTask (http://localhost:4200/polyfills.js:8037:37)
    at ZoneTask.invokeTask [as invoke] (http://localhost:4200/polyfills.js:8312:26)"
        },
        "text": "event: next\ndata: {\"data\":{\"loadFoo\":{\"__typename\":\"Foo\"}},\"hasNext\":true}\n\nevent: next\ndata: {\"incremental\":[{\"data\":{\"name\":\"Bar\",\"__typename\":\"Foo\"},\"path\":[\"loadFoo\"]}],\"hasNext\":false}\n\nevent: complete\n\n"
    }
}

I tested this with the react-client and it simply works. Maybe it's a bug in the codegen? I didn't use Codegen in the react test.

To Reproduce

Create a simple Backend with HotChocolate https://chillicream.com/docs/hotchocolate/v13 and defer any field. It doesn't work with the simplest case. If you want I can provide you one.

Expected behavior

Defer should work.

Environment:

├── @angular/cli@16.2.1
├── @angular/core@16.2.3
├── @apollo/client@3.8.2
├── apollo-angular@5.0.1
├── graphql@16.8.0
└── typescript@5.1.3

    "@graphql-codegen/add": "^5.0.0",
    "@graphql-codegen/cli": "^5.0.0",
    "@graphql-codegen/typescript-apollo-angular": "^3.5.6",
    "@graphql-codegen/typescript-operations": "^4.0.1",

Additional context

Activity

  1. alessbell commented on Sep 14, 2023

    @alessbell
    alessbell
    on Sep 14, 2023
    Contributor

    Hi @fredericbirke 👋 I work on Apollo Client and just came across this issue - would you be able to share a Replay recording of this issue? (Note: you don't have to grant Replay access to a private repo to create a recording, but it will contain source maps for your JavaScript code and record all network requests). You can share it directly with me at alessia@apollographql.com.

    I'd love to look into this and see what may be happening. Thanks :)

  2. fredericbirke commented on Sep 28, 2023

    @fredericbirke
    fredericbirke
    on Sep 28, 2023
    Author

    Hey @alessbell,
    Sorry for the late reply, I was on vacation the last few weeks.
    I will create a reproduction repo and share a replay the next few days :)

  3. alessbell commented on Oct 2, 2023

    @alessbell
    alessbell
    on Oct 2, 2023
    Contributor

    Hey @fredericbirke 👋 Thanks for the offer, but I just noticed that Apollo Angular has its own HttpLink which does not yet have @defer support, so I'm guessing that explains the error you're seeing :) I'd love to help fix that.

  4. fredericbirke commented on Oct 24, 2023

    @fredericbirke
    fredericbirke
    on Oct 24, 2023
    Author

    Hey @alessbell,
    Sorry for the absence. That sounds awesome! Do you still need my reproduction repo or can I help you somehow to add the defer support?

  5. PowerKiKi commented on Oct 25, 2023

    @PowerKiKi
    PowerKiKi
    on Oct 25, 2023
    Collaborator

    I won't work on this myself, but if you guys come up with a PR I'll gladly merge it.

  6. alessbell commented on Oct 25, 2023

    @alessbell
    alessbell
    on Oct 25, 2023
    Contributor

    I'm on vacation this week but I don't need your reproduction, @fredericbirke, but thanks for the offer :) I can pick this up when I'm back online next week.

  7. alessbell commented on Nov 1, 2023

    @alessbell
    alessbell
    on Nov 1, 2023
    Contributor

    Hey everyone 👋 I have some updates to share from looking into this today…

    First, I didn’t know when or why Apollo Angular’s HttpLink diverged from Apollo Client's, so I started there. I saw it supports file uploads, which the original HttpLink doesn’t, but the larger reason this implementation was created, imo, is that Angular still uses XHR instead of fetch.

    Indeed, in Apollo Angular’s http/src/utils.ts ”fetch” 😄 function (used by its HttpLink), the network request is made via httpClient.request. That httpClient is Angular’s HttpClientModule.

    The bad news

    XHR doesn’t support text streaming which is needed to power the multipart response format used to deliver the chunks. This is also an issue for React Native which uses XHR and requires a whole host of polyfills in order to get a feature like @defer working.

    The good news

    That said, as far as I can tell there’s nothing preventing us from using Apollo Client’s HttpLink, and by extension fetch. I created a demo doing exactly this, and it works nicely with @defer (or multipart subscriptions, for that matter): https://github.com/alessbell/simple-apollo-angular

    If you're interested in trying it out yourself, the important bits are in src/app/graphql.module.ts where the provider deps contain our InjectableHttpLink which creates an Apollo Client HttpLink instead of using the HttpLink exported by Apollo Angular.

    All that said, I’m not an Angular dev and would love to hear from folks who try this out. There may be other implications of not using Angular’s HttpClientModule I’m not aware of. The two I'm thinking of are retries and errors: maybe you can’t use RxJS’s retry the same way, but I haven’t tried this out with both HttpLinks for comparison so I'm not sure. And I’m not familiar with the error handling conventions in Angular: using Apollo Client’s HttpLink, I was able to receive errors by passing e.g. catchError(this.handleError) as the final argument to pipe, which receives the ApolloError, but I don’t know enough Angular to update the UI :)

    Again, I’d love to hear from folks who try this out! I’m hoping it’s a feasible alternative, and it seems promising so far. As for this issue, it can probably be closed out as first party support in Apollo Angular’s HttpLink is a non-starter until Angular’s HTTP Client uses fetch.

  8. PowerKiKi commented on Nov 2, 2023

    @PowerKiKi
    PowerKiKi
    on Nov 2, 2023
    Collaborator

    Thank you for investigating.

    The bad news

    Not using HttpClientModule to support defer is unfortunately a non-starter for this lib. Because HttpClientModule offers a tight integration into Angular ecosystem. Especially HttpInterceptor which allows to intercept any XHR globally in the entire application, be it a GraphQL XHR or something else. This is typically useful to show some kind of "network activity" indicator to end-users, but it's also useful to handle errors globally.

    The good news

    fetch support has been introduced in Angular 16.1 via angular/angular#50247, and was made stable very recently. So it will land in Angular 17.0.

    If we can leverage this new FetchBackend to support defer, then we could support defer in this lib. And either requires to use FetchBackend, or detect the backend and support defer only if FetchBackend is used.

  9. alessbell commented on Nov 2, 2023

    @alessbell
    alessbell
    on Nov 2, 2023
    Contributor

    Not using HttpClientModule to support defer is unfortunately a non-starter for this lib.

    I didn't suggest removing HttpClientModule :) My point was that if anyone wants to do this today because they don't need what HttpClientModule provides, they can.

    fetch support has been introduced in Angular 16.1 via angular/angular#50247, and was angular/angular#52197. So it will land in Angular 17.0.

    Indeed, by setting the providers with provideHttpClient(withFetch()), HttpClientModule uses fetch with Angular >= 16.1. But unfortunately there's a blocker: Angular doesn't expose the ReadableStream which is necessary for reading individual chunks as they arrive. Instead, it assembles all chunks into a single result which the observable returns: https://github.com/angular/angular/blob/f615f4f90d3a053de2f273f6bd6d3e00ebfbb790/packages/common/http/src/fetch.ts#L115-L143

    @defer, or any feature using the multipart/mixed response format, will not be possible with HttpClientModule until it exposes this functionality.

  10. alessbell commented on Nov 2, 2023

    @alessbell
    alessbell
    on Nov 2, 2023
    Contributor

    Created a feature request here: angular/angular#52494

  11. alessbell commented on Apr 4, 2024

    @alessbell
    alessbell
    on Apr 4, 2024
    Contributor

    Hi @PowerKiKi 👋 Unfortunately I haven't had much luck with my feature request.

    We've gotten a fair number of support requests from users trying to use @defer with apollo-angular, and I fear this issue isn't the first place blocked users will look. Would you consider adding some language to the docs that describes the current @defer limitations and available workarounds? I'd be happy to draft the PR, thanks!

  12. PowerKiKi commented on Apr 5, 2024

    @PowerKiKi
    PowerKiKi
    on Apr 5, 2024
    Collaborator

    You might have a better position to update the docs as you see fit, since I am not super familiar with @defer. Please create a PR when you have time.

  13. DanielRose commented on May 13, 2024

    @DanielRose
    DanielRose
    on May 13, 2024

    I found this issue when I was trying to implement support for batching requests to a Hot Chocolate backend. The error seen in the browser is the same. However, the Apollo Client currently does not support this either; see apollographql/apollo-feature-requests#387

  14. thegreatdane6 commented on Jul 23, 2024

    @thegreatdane6
    thegreatdane6
    on Jul 23, 2024

    I am experiencing the very same issue as @DanielRose when trying to batch with hot chocolate

  15. PowerKiKi
    added
    help wantedExtra attention is needed
    on Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    help wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions