redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
16.92k stars 973 forks source link

[Bug?]: Trusted Documents does not work with Realtime. #10892

Open ladderschool opened 5 days ago

ladderschool commented 5 days ago

What's not working?

Trusted Documents does not work with Realtime.

Interestingly if you inspect the network tab, the subscription subscribes properly (client establishes websocket) and the trusted store does indeed have an entry for the subscription query. Furthermore, using dev tools you can inspect the request and see that it is indeed transmitting the hash properly.

However, the realtime functionality of receiving the published event is not happening. The error is logged and displayed in a toast if you clone the demo repo.

How do we reproduce the bug?

Clone the repo here: https://github.com/ladderschool/fragments-deploy-docker (latest main commit has realtime and trusted documents)

When you start it up, you'll immediately get a toast error saying the subscription is not a trusted query. You can click the button to create a new message, but you'll have to refresh the page to get the most recent ones instead of them automatically being appended through the pubSub.

What's your environment? (If it applies)

System: 
  OS: Windows 11
Binaries:
  Node: v20.14.0
  Yarn: 4.3.0
Redwood:
  Version 7.7.2

Are you interested in working on this?

dthyresson commented 5 days ago

Hi @ladderschool thanks for reporting. I'll work to reproduce and follow up with what I find. If you could share some of the error logs or even screenshots of the errors just to make sure I am seeing the same behavior.

BTW I am considering adding the options to allow arbitrary ops and other settings found in https://the-guild.dev/graphql/yoga-server/docs/features/persisted-operations#allowing-arbitrary-graphql-operations

Not that this might help since seems like the subscription is received and handled based on your notes -- just somehow not handled in the response back.

dthyresson commented 5 days ago

I have a small realtime app that here has some chat room send and receive messages.

When TD is on:

image

Can see a differently in a query vs a subscription in the request:

{
  "operationName": "RoomsQuery",
  "variables": {},
  "extensions": {
    "persistedQuery": {
      "version": 1,
      "sha256Hash": "f2c9406d0553283e93af7ee7a414def38dae3a9e"
    }
  }
}
{
  "variables": { "roomId": "1" },
  "extensions": {
    "persistedQuery": {
      "version": 1,
      "sha256Hash": "a8c92d9b67b0765d1899b9f5129bd61400d4ee52"
    }
  },
  "operationName": "ListenForNewMessagesInRoom",
  "query": "subscription ListenForNewMessagesInRoom($roomId: ID!) {\n  newMessage(roomId: $roomId) {\n    __typename\n    body\n    from\n  }\n}"
}

While the hash is in both, the subscription also contains the details.

There is code to specifically handle subscriptions:

// Our terminating link needs to be smart enough to handle subscriptions, and if the GraphQL query
  // is subscription it needs to use the SSELink (server sent events link).
  const httpOrSSELink =
    typeof SSELink !== 'undefined'
      ? split(
          ({ query }) => {
            const definition = getMainDefinition(query)

            return (
              definition.kind === 'OperationDefinition' &&
              definition.operation === 'subscription'
            )
          },
          // @ts-expect-error  Due to CJS imports
          new SSELink({
            url: uri,
            auth: { authProviderType, tokenFn: getToken },
            httpLinkConfig,
            headers,
          }),
          httpLink,
        )
      : httpLink

  /**
   * Use Trusted Documents aka Persisted Operations aka Queries
   *
   * When detecting a meta hash, Apollo Client will send the hash from the document and not the query itself.
   *
   * You must configure your GraphQL server to support this feature with the useTrustedDocuments option.
   *
   * See https://www.apollographql.com/docs/react/api/link/persisted-queries/
   */
  interface DocumentNodeWithMeta extends DocumentNode {
    __meta__?: {
      hash: string
    }
  }

  // Check if the query made includes the hash, and if so then make the request with the persisted query link
  const terminatingLink = split(
    ({ query }) => {
      const documentQuery = query as DocumentNodeWithMeta
      return documentQuery?.['__meta__']?.['hash'] !== undefined
    },
    createPersistedQueryLink({
      generateHash: (document: any) => document['__meta__']['hash'],
    }).concat(httpOrSSELink),
    httpOrSSELink,
  )

Perhaps because the request has both the has and graphql doc it is being rejected.

Will debug more.

dthyresson commented 5 days ago

After some investigation, it appears that because the request param has both the persisted query document hash and the query, then the Yoga plugin enters here:

https://github.com/dotansimha/graphql-yoga/blob/66bef8a894a8a65137668a82906f7350457199f6/packages/plugins/persisted-operations/src/index.ts#L121

And thus, says the "only" persisted are allow .. not persisted and/or the has is allowed.

I'm still trying to see why the query param is being added just to subscriptions by either Apollo client or the SSE link. I'll catch with Apollo and the Guild for some help but right now am looking at this line https://github.com/apollographql/apollo-client/blob/d914d689abd9a3364cfa9c6bef8617d97df67a1c/src/link/persisted-queries/index.ts#L225 to see maybe query is always being added.

I can see if the request doesn't add that query param, then the trusted doc subscription is being handled:

image

And with some dist console logging, when making requests in app can see that the subscription has both the query as well as the hash vs a RoomsQuery:

api | 11:11:27 🌲 incoming request POST xxx /graphql
api | >>>>>>>>>>>>> onParams {"operationName":"RoomsQuery","variables":{},"extensions":{"persistedQuery":{"version":1,"sha256Hash":"f2c9406d0553283e93af7ee7a414def38dae3a9e"}}}
api | 11:11:27 πŸ› Parsing request to extract GraphQL parameters
api | 11:11:27 πŸ› Processing GraphQL Parameters
api | 11:11:27 πŸ› graphql-server GraphQL execution started: RoomsQuery
api | 11:11:27 πŸ› graphql-server GraphQL execution completed: RoomsQuery
api | 11:11:27 πŸ› Processing GraphQL Parameters done.
api | 11:11:27 🌲 request completed 30ms
api | 11:11:27 🌲 incoming request POST xxx /graphql
api | >>>>>>>>>>>>> onParams {"variables":{"roomId":"1"},"extensions":{"persistedQuery":{"version":1,"sha256Hash":"a8c92d9b67b0765d1899b9f5129bd61400d4ee52"}},"operationName":"ListenForNewMessagesInRoom","query":"subscription ListenForNewMessagesInRoom($roomId: ID!) {\n  newMessage(roomId: $roomId) {\n    __typename\n    body\n    from\n  }\n}"}
dthyresson commented 5 days ago

Confirmed that if I patch the plugin:

function usePersistedOperations({ allowArbitraryOperations = false, extractPersistedOperationId = exports.defaultExtractPersistedOperationId, getPersistedOperation, skipDocumentValidation = false, customErrors, }) {
    const operationASTByRequest = new WeakMap();
    const persistedOperationRequest = new WeakSet();
    const notFoundErrorFactory = createErrorFactory('PersistedQueryNotFound', customErrors?.notFound);
    const keyNotFoundErrorFactory = createErrorFactory('PersistedQueryKeyNotFound', customErrors?.keyNotFound);
    const persistentQueryOnlyErrorFactory = createErrorFactory('PersistedQueryOnly', customErrors?.persistedQueryOnly);
    return {
        async onParams(payload) {
          console.log('>>>>>>>>>>>>> onParams', JSON.stringify(payload.params))
            const { request, params, setParams } = payload;
            // if (params.query) {
            //     if ((typeof allowArbitraryOperations === 'boolean'
            //         ? allowArbitraryOperations
            //         : await allowArbitraryOperations(request)) === false) {
            //         throw persistentQueryOnlyErrorFactory(payload);
            //     }
            //     return;
            // }
            const persistedOperationKey = extractPersistedOperationId(params, request);

then subscriptions with the hash are allowed.

So, have to figure out why for subscriptions have both query and the hash. And excluding query hopefully should work.

It might be in the SSELink or Apollo client or elsewhere. Haven't pinpointed it yet.

dthyresson commented 5 days ago

Think I found it and have tested a fix.

I based the SSELink implementation on https://the-guild.dev/graphql/sse/recipes#with-apollo which adds in the operation and also writes the query.

return this.client.subscribe<FetchResult>(
        {
          ...operation,
          query: print(operation.query),
        },
        {
          next: sink.next.bind(sink),
          complete: sink.complete.bind(sink),
          error: sink.error.bind(sink),
        },
      )

What I have tried -- and looks to work --- is if there's a hash (aka trusted docs being used), then delete the query form the operation and return

      if (operation.extensions?.persistedQuery?.sha256Hash) {
        delete operation.query

        return this.client.subscribe<FetchResult>(
          {
            ...operation,
          },
          {
            next: sink.next.bind(sink),
            complete: sink.complete.bind(sink),
            error: sink.error.bind(sink),
          },
        )
      }

Going to test more and cleanup code -- and check with Apollo/Guild -- but think have narrowed it down.

To test locally can patch your web/node_modules/@redwoodjs/web/dist/apollo/sseLink.js to delete operation.query.

dthyresson commented 5 days ago

@ladderschool Have a PR in which 🀞 fixes. I'll have to test more but can see https://github.com/redwoodjs/redwood/pull/10893

ladderschool commented 5 days ago

Amazing stuff, you're the greatest! Thank you for all you do πŸ™