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

Implement callback subscription protocol in new plugin `ApolloServerPluginSubscriptionCallback` #7617

Merged

trevor-scheer merged 21 commits into main from trevor/callback-subscriptions

Jul 27, 2023

Member

trevor-scheer commented Jun 23, 2023 •

edited

This PR creates a new plugin for Apollo Server which implements the new subscription callback protocol.

While the plugin approach works, it does have a couple notable pitfalls. I see this as a good first step towards enabling people to use it today. The nature of this implementation is to skip execution in Apollo Server altogether, which means the execution internals are bypassed. This means that some plugin hooks are skipped - notably executionDidStart/End and willResolveField (for field-level instrumentation) when resolving these types of operations.

Fortunately, most of the logic is captured in the SubscriptionManager class. The plugin mostly just wraps it, and Apollo Server should be able to integrate it when we choose to.

netlify bot commented Jun 23, 2023 •

edited

✅ Deploy Preview for apollo-server-docs ready!

Name	Link
🔨 Latest commit	`c7885c8`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-server-docs/deploys/64c077aa92ce4500083b3cac
😎 Deploy Preview	https://deploy-preview-7617--apollo-server-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

trevor-scheer requested review from glasser and a team

June 23, 2023 00:41

codesandbox-ci bot commented Jun 23, 2023 •

edited

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c7885c8:

Sandbox	Source
Apollo Server Typescript	Configuration
Apollo Server	Configuration

trevor-scheer commented

View reviewed changes

.changeset/cuddly-lemons-dance.md Show resolved Hide resolved

trevor-scheer commented

View reviewed changes

packages/server/package.json Show resolved Hide resolved

glasser reviewed

View reviewed changes

Member

glasser left a comment

I took a quick initial glance and have some high level questions, but I think I'll need to learn more about the details of the subscription callback protocol before doing more of a review.

packages/server/src/internalPlugin.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Show resolved Hide resolved

trevor-scheer added 4 commits

June 27, 2023 09:50


          Initial implementation of plugin

c241ed3


          refactor maps, deduplicate redundant info

8afc606


          one FIXME

855b420


          update plugin export types

7bd798a

trevor-scheer force-pushed the trevor/callback-subscriptions branch from 0f1bde6 to 7bd798a Compare

June 27, 2023 16:51

trevor-scheer added 4 commits

June 27, 2023 10:03


          update changeset

ea2f768


          cleanup

ee1c3ab


          address a couple fixmes

b36baa5


          add test for heartbeat failures

61dbdfb

trevor-scheer marked this pull request as ready for review

June 27, 2023 18:32


          use 500 instead of 400 status code

29ab4f1

bnjjj reviewed

View reviewed changes

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

trevor-scheer added 2 commits

July 18, 2023 10:05


          Respond with valid graphql response

5c93e30


          empty commit to check for CCI hiccup

8beaeae

jeffjakub requested a review from clenfest

July 20, 2023 18:58

clenfest reviewed

View reviewed changes

packages/server/src/plugin/subscriptionCallback/index.ts

+                          return;
+                        }
+                        // The `check` request was successful, so we can initialize the actual

clenfest Jul 21, 2023

Do you want to check the uniqueness of the subscription ids before you subscribe?

clenfest Jul 21, 2023

Actually I guess the router probably already does that for you?

Member Author

trevor-scheer Jul 24, 2023

I wouldn't expect the router to send duplicates but I suppose it could, are you suggesting defending against that (maybe by checking to see if we're already subscribed on that id in the SubscriptionManager)?

clenfest Jul 25, 2023

I guess I was thinking about a client that was generating non-unique subscription ids.

packages/server/src/plugin/subscriptionCallback/index.ts Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated

+                          self.logger?.debug('`next` request successful', id);
+                        } catch (e) {
+                          self.logger?.error(`\`next\` request failed: ${e}`, id);
+                          // TODO: handle this error (terminate subscription / retry?)

clenfest Jul 21, 2023

This could fail due to a transient error, right? I'm not sure what the spec says about guaranteed delivery vs. ordering trade off, but I wouldn't think we'd want to throw the first time there's an error.

trevor-scheer added 4 commits

July 24, 2023 15:47


          save intermediate constants for reuse later in the block

c904436


          refine log message

61525a8


          handle unexpected status codes in heartbeat response

934ca4b


          max failures configurable

191e4d0

trevor-scheer force-pushed the trevor/callback-subscriptions branch from 1294b14 to 191e4d0 Compare

July 25, 2023 16:47


          don't await in-flight heartbeat to send subscription payload

9f09eed

trevor-scheer force-pushed the trevor/callback-subscriptions branch from 67a7335 to 0cdceda Compare

July 25, 2023 20:17


          implement retry/error handling on update

5ab55aa

trevor-scheer force-pushed the trevor/callback-subscriptions branch from 0cdceda to 5ab55aa Compare

July 25, 2023 20:22

clenfest reviewed

View reviewed changes

packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts Outdated Show resolved Hide resolved

packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Outdated Show resolved Hide resolved

packages/server/src/plugin/subscriptionCallback/index.ts Show resolved Hide resolved

trevor-scheer added 3 commits

July 25, 2023 14:47


          use allSettled

c0faf33


          set reasonable defaults

b187bef


          fixme -> xxx; i'm ok with this for now but the comment is worth keepi…

4e3b4b6

…ng around

Member Author

trevor-scheer commented Jul 25, 2023 •

edited

@clenfest I wrapped the other request types with the retry lib as well. There's almost a good abstraction to be made there but there is the one special case for check, I'll look at it some more. I'm thinking heartbeat shouldn't be retried - if it fails and starts backing off it's going to be outside the 5s window and terminated anyway.

Update: pretty happy with the outcome in c7885c8, but the diff is really awful. Looks like git found a bit too much similarity in 2 different functions and clobbered it.

trevor-scheer requested a review from clenfest

July 25, 2023 22:51

trevor-scheer force-pushed the trevor/callback-subscriptions branch from a573c69 to c42ec16 Compare

July 26, 2023 01:30


          retry fetch refactor, consolidate 404/4xx termination logic

c7885c8

trevor-scheer force-pushed the trevor/callback-subscriptions branch from c42ec16 to c7885c8 Compare

July 26, 2023 01:32

clenfest approved these changes

View reviewed changes

clenfest left a comment

lgtm

trevor-scheer merged commit 4ff81ca into main

20 checks passed

trevor-scheer deleted the trevor/callback-subscriptions branch

July 27, 2023 19:46

github-actions bot mentioned this pull request

Version Packages #7661

Merged

trevor-scheer pushed a commit that referenced this pull request


          Version Packages (#7661)

This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @apollo/server@4.9.0

### Minor Changes

- [#7617](#7617)
[`4ff81ca50`](4ff81ca)
Thanks [@trevor-scheer](https://github.com/trevor-scheer)! - Introduce
new `ApolloServerPluginSubscriptionCallback` plugin. This plugin
implements the [subscription callback
protocol](https://github.com/apollographql/router/blob/dev/dev-docs/callback_protocol.md)
which is used by Apollo Router. This feature implements subscriptions
over HTTP via a callback URL which Apollo Router registers with Apollo
Server. This feature is currently in preview and is subject to change.

    You can enable callback subscriptions like so:

    ```ts
import { ApolloServerPluginSubscriptionCallback } from
'@apollo/server/plugin/subscriptionCallback';
    import { ApolloServer } from '@apollo/server';

    const server = new ApolloServer({
      // ...
      plugins: [ApolloServerPluginSubscriptionCallback()],
    });
    ```

Note that there is currently no tracing or metrics mechanism in place
for callback subscriptions. Additionally, this plugin "intercepts"
callback subscription requests and bypasses some of Apollo Server's
internals. The result of this is that certain plugin hooks (notably
`executionDidStart` and `willResolveField`) will not be called when
handling callback subscription requests or when sending subscription
events.

For more information on the subscription callback protocol, visit the
docs:

<https://www.apollographql.com/docs/router/executing-operations/subscription-callback-protocol/>

### Patch Changes

- [#7659](#7659)
[`4784f46fb`](4784f46)
Thanks [@renovate](https://github.com/apps/renovate)! - Update
graphql-http dependency

## @apollo/server-integration-testsuite@4.9.0

### Patch Changes

- [#7659](#7659)
[`4784f46fb`](4784f46)
Thanks [@renovate](https://github.com/apps/renovate)! - Update
graphql-http dependency

- Updated dependencies
\[[`4ff81ca50`](4ff81ca),
[`4784f46fb`](4784f46)]:
    -   @apollo/server@4.9.0

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

trevor-scheer mentioned this pull request

fix: options and observeHeadersSent flag #7669

Closed

Contributor

enisdenjo commented Aug 18, 2023

I have a small question regarding the subscription completion.

The emitter notifies the router about completion using the complete message, but what about the other way around - how does a router notify the emitter? Say someone subscribed to a never-ending subscription on the router and then unsubscribes, how does the emitter know when to stop sending messages?

o0Ignition0o commented Aug 18, 2023 •

edited

Hi @enisdenjo, it's nice to meet you!

When a subgraph sends a payload to the router, the router will use the http status_code to indicate the emitter some or all clients unsubscribed.

The error-states section of the docs, and the heartbeat answer provide details as to what to expect.

Hope it helps!

Contributor

enisdenjo commented Aug 18, 2023

Hey there @o0Ignition0o, nice to meet you too!

Thanks for the explanation and the links, they were very helpful. I have the complete picture now!

github-actions bot locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.