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.

Sign up for GitHub

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

Jump to bottom

onError link not working for HTTP multipart subscriptions #12258

Open
ssmilin opened this issue Jan 9, 2025 · 4 comments · May be fixed by #12281
Open

onError link not working for HTTP multipart subscriptions #12258

ssmilin opened this issue Jan 9, 2025 · 4 comments · May be fixed by #12281
Assignees
@jerelmiller
Labels
🏓 awaiting-team-response requires input from the apollo team 🐞 bug

Comments

@ssmilin
Copy link

ssmilin commented Jan 9, 2025 •
edited by jerelmiller
Loading

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
Copy link
Member

phryneas commented Jan 9, 2025

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
Copy link
Author

ssmilin commented Jan 9, 2025 •
edited by jerelmiller
Loading

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 jerelmiller added the ℹ needs-more-info Needs more information to determine root cause label Jan 11, 2025
@jerelmiller
Copy link
Member

@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 jerelmiller added 🏓 awaiting-team-response requires input from the apollo team 🐞 bug and removed ℹ needs-more-info Needs more information to determine root cause labels Jan 11, 2025
@jerelmiller
Copy link
Member

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!

@jerelmiller jerelmiller self-assigned this Jan 17, 2025
@jerelmiller jerelmiller linked a pull request Jan 17, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jerelmiller jerelmiller

Labels
🏓 awaiting-team-response requires input from the apollo team 🐞 bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make protocol errors available in the error link apollographql/apollo-client
3 participants
@jerelmiller @phryneas @ssmilin