apollographql / apollo-client

:rocket:  A fully-featured, production ready caching GraphQL client for every UI framework and GraphQL server.
https://apollographql.com/client
MIT License
19.28k stars 2.64k forks source link

GraphQLWsLink doesn't throw ApolloError when network error happens #11916

Open UchihaYuki opened 1 week ago

UchihaYuki commented 1 week ago

Issue Description

You can connect to any non-existing endpoint to prove this. Code snippet:

import { ApolloClient, InMemoryCache } from "@apollo/client/core/index.js";
import { GraphQLWsLink } from "@apollo/client/link/subscriptions/index.js";
import { createClient } from "graphql-ws";
import { EventDocument } from "./graphql-client/generated/api.codegen";
import ws from "ws";

// @ts-ignore
globalThis.WebSocket = ws;

const client = new ApolloClient({
  cache: new InMemoryCache(),
  link: new GraphQLWsLink(
    createClient({
      url: "https://not-exit.com/graphql",
      lazy: true,
    })
  ),
});

client
  .subscribe({
    query: EventDocument,
  })
  .subscribe({
    error: (err) => {
      console.error(err);
    },
  });

Shouldn't it throw an ApolloError with networkError field like client.query and client.mutate for unity?

Link to Reproduction

I already paste the complete code above.

Reproduction Steps

transpile it and run with node. It will print

Error: Socket closed
...

@apollo/client version

3.10.7

UchihaYuki commented 1 week ago

Is there any workaround I can do now? I hope to unify errors.

jerelmiller commented 1 week ago

Hey @UchihaYuki 👋

You're right. Very odd that we don't wrap raw errors in ApolloError. Unfortunately changing this now would require a major version since this would be a breaking change. I don't entirely know the historical reason why non-GraphQL errors aren't wrapped in ApolloError instances.

For now, if you'd like to unify the errors, you might consider wrapping it yourself with a custom link so that all errors that come back through your link chain are ApolloError instances:

// warning: untested
const wrapApolloErrorLink = new ApolloLink((operation, forward) => {
  return new Observable((observer) => {
    const subscription = forward(observable).subscribe({
      complete: observer.complete.bind(observer),
      next: observer.next.bind(observer),
      error: (error) => {
        if (error.name !== 'ApolloError') {
          error = new ApolloError({ networkError: error });
        }

        observer.error(error);
      }
    })

    return () => subscription.unsubscribe();
  });
});

I don't entirely know the ramifications of this, so I'd encourage you to test this out and make sure it behaves as you'd expect.

Hopefully though this gives you a starting point. Feel free to play around with this to get your desired outcome. If you only want this for subscriptions and you're using split, consider adding this only to the branch that handles subscriptions.

I'll leave this issue open for now as a potential improvement we can make for a future major.