onError link not working for HTTP multipart subscriptions

[ssmilin]

onError link is not invoked when an error occurs during an HTTP multipart subscription. It successfully checks the error response from the 'query' operation but fails to do the same for the 'subscription' operation.

Subscription Response:

{
  "payload": null,
  "errors": [
    {
      "message": "cannot read message from websocket",
      "extensions": { "code": "WEBSOCKET_MESSAGE_ERROR" }
    }
  ]
}

Client:

const errorLink = onError(({ graphQLErrors, networkError }) => {
  if (graphQLErrors) {
    graphQLErrors.forEach(({ message, locations, path }) =>
      console.log(
        `[GraphQL error]: Message: ${message}, Location: ${locations}, Path: ${path}`
      )
    );
  }
  if (networkError) console.error(`[Network error]: ${networkError}`);
});

return new ApolloClient<any>({
  link: from([errorLink, httpLink]),
  cache: new InMemoryCache()
});

[phryneas]

Could you please also show the httpLink setup and generally tidy this issue up a bit? It's very hard to see what's going on there.

[ssmilin]

Sure. httpLink is a simple URL that points to our local Apollo router.

const httpLink = new HttpLink({
  uri: "http://localhost:4000/graphql"
});

We are trying to catch errors to automatically handle re-authentication when the token expires. Please note that there is another custom link to inject the token header, which I have skipped for now. Currently, I am testing with the errorLink and httpLink to check error handling.

[jerelmiller]

@ssmilin I believe what you're seeing are protocol errors which are fatal transport-level errors in multipart subscriptions. See this section in the docs that mention the top-level errors property which describes what you're seeing.

See this test as well which looks like the exact error/format you're seeing:

apollo-client/src/__tests__/graphqlSubscriptions.ts

Lines 459 to 506 in cda321c

	it("should throw an error if the result has protocolErrors on it", async () => {
	const link = mockObservableLink();
	const queryManager = new QueryManager(
	getDefaultOptionsForQueryManagerTests({
	link,
	cache: new InMemoryCache({ addTypename: false }),
	})
	);
	const obs = queryManager.startGraphQLSubscription(options);
	const promise = new Promise<void>((resolve, reject) => {
	obs.subscribe({
	next(result) {
	reject("Should have hit the error block");
	},
	error(error) {
	expect(error).toMatchSnapshot();
	resolve();
	},
	});
	});
	const errorResult = {
	result: {
	data: null,
	extensions: {
	[PROTOCOL_ERRORS_SYMBOL]: [
	{
	message: "cannot read message from websocket",
	extensions: [
	{
	code: "WEBSOCKET_MESSAGE_ERROR",
	},
	],
	} as any,
	],
	},
	},
	};
	// Silence expected warning about missing field for cache write
	using _consoleSpy = spyOnConsole("warn");
	link.simulateResult(errorResult);
	await promise;
	});

To be honest, I don't remember the reason we bury that error in a symbol which is not accessible in the onError link. Let me ask around and see if I can figure out why, though I can't promise anything until next week since I'm about to start my weekend.

[jerelmiller]

Hey @ssmilin 👋

@phryneas and I talked about this and we think that making these errors available in onError as protocolErrors makes sense. The symbol added for these protocol errors was done to avoid naming collisions any user-defined fields in extensions.

I can't guarantee a time-frame on the fix but we'll try to get to it when we can. Thanks for raising the issue!

[github-actions]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

[jerelmiller]

Hey @ssmilin 👋

This was just released in 3.12.7. Please upgrade to that version and you should be good to go. Let us know if you have any problems working with it. Thanks!

[ssmilin]

I have tried using the updated package. The errorLink is invoked the first time, but it is not called on subsequent attempts. Please find the attached screenshot for the network call.

const errorLink = onError(({ protocolErrors, operation, forward }) => {
    if (protocolErrors) {
        return forward(operation);
    }
});

return new ApolloClient<CacheShape>({
    link: from([errorLink, httpLink]),
    cache: new InMemoryCache()
});

[phryneas]

That is really curious, thank you for the report!

[jerelmiller]

I have a suspicion that this is a limitation of the onError link in general. I believe it only allows you to retry only once with the way its constructed, so the 2nd failure isn't caught. This would be a limitation for other kinds of errors as well (not just this one). The RetryLink is more suited toward multiple retries, though as I look at that one, I don't believe that one handles protocol errors so we'd need to add support for them there as well.