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

Stitching schema with wrapSchema seems swallowing or not passing through the error #6477

Open
1 of 4 tasks
inpercima opened this issue Aug 28, 2024 · 2 comments
Open
1 of 4 tasks

Comments

@inpercima
Copy link

inpercima commented Aug 28, 2024

Issue workflow progress

Progress of the issue based on the
Contributor Workflow

  • 1. The issue provides a reproduction available on Github, Stackblitz or CodeSandbox

    Make sure to fork this template and run yarn generate in the terminal.

    Please make sure the GraphQL Tools package versions under package.json matches yours.

  • 2. A failing test has been provided
  • 3. A local solution has been provided
  • 4. A pull request is pending review

Describe the bug

We have the need to combine several remote schemas into one. We use stitching for this.
For the basis and understanding of the following code:
We need both http and ws functionalities and authentication.
We have a Graphql-Playground activated.

Basically, everything works, but if an error and data are returned from the remote schema, the error is no longer present in various constellations after the wrapSchema or shortly before, although the query from the remote schema does contain the error in between.

To Reproduce Steps to reproduce the behavior:

The base:
To handle ws subscriptions and query the rest over http we use hybrid executor to use WS for subscriptions

Also we use Introspecting Schemas using Executors

Stitching:
We stitch a local schema and many other remote schemas to one and use for this stichSchemas and wrapSchema.

Full code:
The most of the code is more described in the links above.

/**
   * Stitches the local schema and the schemas from remote APIs to one new schema.
   * @see https://the-guild.dev/graphql/stitching/docs/getting-started/remote-subschemas
   */
  private async stitch(localSchema: GraphQLSchema): Promise<GraphQLSchema> {
    const remoteSchemas = await Promise.all(
      // remoteApis includes the URL to the services
      remoteApis().map(async (remoteApi: RemoteApi) => this.createRemoteSchema(remoteApi)),
    );

    return stitchSchemas({
      subschemas: [localSchema, ...remoteSchemas.filter(Boolean)],
    });
  }

  /**
   * Fetches the schema from a remote service and
   * wrap it with transformations (renaming) in name and type to a new schema.
   */
  private async createRemoteSchema(remoteApi: RemoteApi): Promise<GraphQLSchema> {
    try {
      const httpExecutor: AsyncExecutor = async ({ document, variables, operationName, extensions }) => {
        const query = print(document);
        const fetchResult = await fetch(remoteApi.url, {
          method: 'POST',
          headers: {
            /**
             * @see https://chillicream.com/docs/hotchocolate/v13/migrating/migrate-from-12-to-13#http-transport
             */
            Accept: 'application/json',
            Authorization: this.authHeaderToken,
            'Content-Type': 'application/json; charset=utf-8',
          },
          body: JSON.stringify({ query, variables, operationName, extensions }),
        });
        return fetchResult.json();
      };

      /**
       * @see https://the-guild.dev/graphql/stitching/docs/getting-started/remote-subschemas#create-a-hybrid-executor-to-use-ws-for-subscriptions
       * @see https://stackoverflow.com/questions/75987086/apollo-server-subscription-middleware-to-intercept-request-body-to-and-request-s
       */
      const subscriptionClient = remoteApi.ws
        ? createClient({
            /**
             * The webSocketImpl is necessary to work.
             *
             * @see https://stackoverflow.com/questions/72116940/apollo-graphql-graphqlwslink-subscriptions-troubles-cannot-get-websocket-imp
             * @see https://the-guild.dev/graphql/ws/recipes#client-usage-in-node-with-custom-headers-not-possible-in-browsers
             */
            webSocketImpl: WebSocket,
            url: remoteApi.ws,
            lazyCloseTimeout: 50000,
            shouldRetry: () => true,
            connectionParams: async () => {
              return {
                Authorization: this.authHeaderToken,
                Accept: 'application/json',
              };
            },
            lazy: true,
            /**
             * onNonLazyError is used if lazy is set to false
             */
            // eslint-disable-next-line @typescript-eslint/no-empty-function
            onNonLazyError: () => {},
            on: {
              connected: () => {
                console.debug(`graphql-ws connected`);
              },
              error: err => console.log(err),
            },
          })
        : ({} as Client);

      const wsExecutor: AsyncExecutor = remoteApi.ws
        ? async ({ document, variables, operationName, extensions }) =>
            observableToAsyncIterable({
              subscribe: observer => ({
                unsubscribe: subscriptionClient.subscribe(
                  {
                    query: print(document),
                    variables: variables as Record<string, any>,
                    operationName,
                    extensions,
                  },
                  {
                    next: data => observer.next?.(data as any),
                    error(err) {
                      if (!observer.error) return;
                      if (err instanceof Error) {
                        observer.error(err);
                      } else if (Array.isArray(err)) {
                        observer.error(new Error(err.map(({ message }) => message).join(', ')));
                      } else {
                        observer.error(new Error(`Socket closed with event: ${err}`));
                      }
                    },
                    complete: () => observer.complete?.(),
                  },
                ),
              }),
            })
        : ({} as AsyncExecutor);

      const executor: AsyncExecutor = async executorRequest => {
        // subscription operations should be handled by the wsExecutor
        if (remoteApi.ws && executorRequest.operationType === 'subscription') {
          return wsExecutor(executorRequest);
        }
        // all other operations should be handles by the httpExecutor
        return httpExecutor(executorRequest);
      };

      return wrapSchema({
        schema: await schemaFromExecutor(executor),
        executor: executor,
        transforms: [
          new RenameTypes(type => remoteApi.prefix + type, { renameBuiltins: false, renameScalars: false }),
          new RenameRootFields((operation, name) => remoteApi.prefix + name),
        ],
      });
    } catch (error) {
      this.logger.error(`failed connecting '${remoteApi.name}'`);
      return error;
    }
  }

While debugging and logging I can see on executing httpExecutor(executorRequest) that all data we need, error and data is given.

// all other operations should be handles by the httpExecutor
const debug = httpExecutor(executorRequest);
debug.then(result => console.log(result));
return httpExecutor(executorRequest);

Behavior A:
If an error is given and the data is filled with an element which is null, the result with error is given in httpExecutor and Graphql-Playground.

// logging from httpExecutor section
{
    errors: [ { message: 'Specified key "X" is already used.' } ],
    data: { createProduct: null }
}
// logging from Graphql-Playground
"errors": [
    {
        "message": "Specified key \"X\" is already used.",
        "path": [
             "createProduct"
        ],
        ...
    }
],
"data": {
    "createProduct": null
}

Behavior B:
If and error is given and the data is filled with an element which is not null, the result with error is given in the httpExecutor only and not passed through to the Graphql-Playground or other clients connecting to the server.

// logging from httpExecutor section
{
    errors: [ { message: 'Specified key "PO" is already used.' } ]
    data: { updateProduct: { id: 'a1e12327-6456-43df-b789-b4ab32d23012' }
}
// logging from  Graphql-Playground
"data": {
    "updateProduct": {
        "id": "a1e12327-6456-43df-b789-b4ab32d23012"
    }
}

So the question is, what happen in wrapSchema or schemaFromExecutor that the error is swallowing or not passing through?

Expected behavior

The error should always present even if data is given.

Environment:

  • OS: Linux/Windows
  • "@graphql-tools/stitch": "9.2.10":
  • "@graphql-tools/utils": "10.5.4",:
  • "@graphql-tools/wrap": "10.0.5",:
  • NodeJS: v20.15.1

Additional context

I found some other older issues (2020) related to this and some resolutions working with the problem but these all 4 years old and I hope there is another solution:

We tested now the transformResult and this seems to help but I don't think that should be necessary, right? Or is this the way do handle the error?

@ardatan
Copy link
Owner

ardatan commented Aug 28, 2024

Thanks for creating the issue. Would you create a PR with a failing test?

@inpercima
Copy link
Author

@ardatan Thank you for your feedback. I'll see that I create a simple demonstration and consider how to create a failed test for this one.

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