upgrade to subscription callback protocol GA #7793

bnjjj · 2023-11-29T10:03:32Z

This is a PR to upgrade the subscription callback protocol to GA. We made different changes listed in this issue

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

netlify · 2023-11-29T10:03:38Z

✅ Deploy Preview for apollo-server-docs ready!

Name	Link
🔨 Latest commit	`ee56889`
🔍 Latest deploy log	https://app.netlify.com/sites/apollo-server-docs/deploys/657c63272390490008029859
😎 Deploy Preview	https://deploy-preview-7793--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.

bnjjj · 2023-11-29T10:04:10Z

For now the tests don't pass because it doesn't see the mocked call for a check message in the protocol and I don't know why

codesandbox-ci · 2023-11-29T10:04:13Z

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 ee56889:

Sandbox	Source
Apollo Server Typescript	Configuration
Apollo Server	Configuration

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

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

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

…jjj/update_callback_plugin

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

trevor-scheer · 2023-12-07T17:59:28Z

Some high-level notes while following the doc here

Doc still shows snake case for the extensions object, should it be updated already or waiting for GA?
There's no reference to the mentioned headers in the plugin's code or tests, which I expect we need. Apollo server does some special accept header handling early in the request pipeline so we need to make sure this new header works as expected. We can "expect" specific headers in requests with the nock testing utility with the matchHeader(...) fn.
- accept: application/json+graphql+callback/1.0
- subscription-protocol: callback/1.0
It looks like there's a unique callback URL per subscription now. Can we update the mocks and tests to reflect that format and represent more accurately what the requests look like between router and plugin? i.e. the defaulted path: '/' should probably be updated to '/callback/1234-cats' for all the mocks, etc.

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

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

Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

…ql/apollo-server into bnjjj/update_callback_plugin

trevor-scheer

I had a comment on last iteration about the headers that still needs to be addressed.

There's no reference to the mentioned headers in the plugin's code or tests, which I expect we need. Apollo server does some special accept header handling early in the request pipeline so we need to make sure this new header works as expected. We can "expect" specific headers in requests with the nock testing utility with the matchHeader(...) fn.

accept: application/json+graphql+callback/1.0
subscription-protocol: callback/1.0

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

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

The callback protocol requires specific accept/content-type headers as well as a subscription-protocol header in its requests/responses between router and server. In order to test the correct behavior here, we need to include the HTTP layer in the testing details of the callback plugin.

The subscription-protocol header should be sent for `check` requests The content-type: application/json+graphql+callback/1.0 is only necessary in response to the original subscription request.

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

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

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

trevor-scheer · 2023-12-14T17:21:25Z

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

+      if (
+        heartbeatIntervalMs == null &&
+        subscriptionExtension.heartbeat_interval_ms != null
+      ) {
+        // Deprecated field name
+        heartbeatIntervalMs = subscriptionExtension.heartbeat_interval_ms;
+      } else if (
+        heartbeatIntervalMs == null &&
+        subscriptionExtension.heartbeat_interval_ms == null
+      ) {
+        // Default value
+        heartbeatIntervalMs = 5000;
+      }


This is just handling the 0 is falsey bug, yeah? Nullish coalesce handles this well (0 is preserved):

heartbeatIntervalMs = heartbeatIntervalMs ?? subscriptionExtension.heartbeat_interval_ms ?? 5000;

?? is generally a more correct replacement for the commonly used || fallback in JS.

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

@ganemone

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-integration-testsuite@4.10.0 ### Minor Changes - [#7786](#7786) [`869ec98`](869ec98) Thanks [@ganemone](https://github.com/ganemone)! - Restore missing v1 `skipValidation` option as `dangerouslyDisableValidation`. Note that enabling this option exposes your server to potential security and unexpected runtime issues. Apollo will not support issues that arise as a result of using this option. ### Patch Changes - [#7740](#7740) [`fe68c1b`](fe68c1b) Thanks [@barnisanov](https://github.com/barnisanov)! - Uninstalled `body-parser` and used `express` built-in `body-parser` functionality instead(mainly the json middleware) - Updated dependencies \[[`869ec98`](869ec98), [`9bd7748`](9bd7748), [`63dc50f`](63dc50f), [`fe68c1b`](fe68c1b), [`e9a0d6e`](e9a0d6e)]: - @apollo/server@4.10.0 ## @apollo/server@4.10.0 ### Minor Changes - [#7786](#7786) [`869ec98`](869ec98) Thanks [@ganemone](https://github.com/ganemone)! - Restore missing v1 `skipValidation` option as `dangerouslyDisableValidation`. Note that enabling this option exposes your server to potential security and unexpected runtime issues. Apollo will not support issues that arise as a result of using this option. - [#7803](#7803) [`e9a0d6e`](e9a0d6e) Thanks [@favna](https://github.com/favna)! - allow `stringifyResult` to return a `Promise<string>` Users who implemented the `stringifyResult` hook can now expect error responses to be formatted with the hook as well. Please take care when updating to this version to ensure this is the desired behavior, or implement the desired behavior accordingly in your `stringifyResult` hook. This was considered a non-breaking change as we consider that it was an oversight in the original PR that introduced `stringifyResult` hook. ### Patch Changes - [#7793](#7793) [`9bd7748`](9bd7748) Thanks [@bnjjj](https://github.com/bnjjj)! - General availability of subscription callback protocol - [#7799](#7799) [`63dc50f`](63dc50f) Thanks [@stijnbe](https://github.com/stijnbe)! - Fix type of ApolloServerPluginUsageReporting reportTimer - [#7740](#7740) [`fe68c1b`](fe68c1b) Thanks [@barnisanov](https://github.com/barnisanov)! - Uninstalled `body-parser` and used `express` built-in `body-parser` functionality instead(mainly the json middleware) Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

upgrade to subscription callback protocol GA

937e096

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

bnjjj requested a review from trevor-scheer November 29, 2023 10:03

bnjjj self-assigned this Nov 29, 2023

bnjjj added 2 commits December 5, 2023 17:21

fix tests

c3c584f

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

add more tests

81540a7

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

trevor-scheer reviewed Dec 6, 2023

View reviewed changes

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

trevor-scheer reviewed Dec 6, 2023

View reviewed changes

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

get rid of subscriptionsById

2abb6a7

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

bnjjj marked this pull request as ready for review December 7, 2023 15:41

bnjjj mentioned this pull request Dec 7, 2023

Put callback mode in GA for subscriptions apollographql/router#3884

Closed

bnjjj added 2 commits December 7, 2023 17:28

Merge branch 'main' of github.com:apollographql/apollo-server into bn…

7e04082

…jjj/update_callback_plugin

add changeset

9d69367

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

trevor-scheer requested changes Dec 7, 2023

View reviewed changes

Apply suggestions from code review

dd5d680

Co-authored-by: Trevor Scheer <trevor.scheer@gmail.com>

bnjjj requested a review from trevor-scheer December 11, 2023 10:18

bnjjj added 2 commits December 11, 2023 11:19

review

c06d93a

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

Merge branch 'bnjjj/update_callback_plugin' of github.com:apollograph…

90d3277

…ql/apollo-server into bnjjj/update_callback_plugin

trevor-scheer requested changes Dec 11, 2023

View reviewed changes

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

fix tests

acb69f7

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

bnjjj requested a review from trevor-scheer December 12, 2023 13:41

trevor-scheer and others added 5 commits December 13, 2023 07:53

Apply and test for special headers appropriately

b4839f0

The subscription-protocol header should be sent for `check` requests The content-type: application/json+graphql+callback/1.0 is only necessary in response to the original subscription request.

for tests purposes

7e04f1c

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

change accept header and content-type

8d57450

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

handle callback subscriptions properly when heartbeat is disabled

83369e3

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

bnjjj force-pushed the bnjjj/update_callback_plugin branch from bc55b44 to 83369e3 Compare December 14, 2023 11:10

trevor-scheer reviewed Dec 14, 2023

View reviewed changes

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

trevor-scheer reviewed Dec 14, 2023

View reviewed changes

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

trevor-scheer reviewed Dec 14, 2023

View reviewed changes

trevor-scheer approved these changes Dec 14, 2023

View reviewed changes

refactor

ee56889

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

trevor-scheer approved these changes Dec 18, 2023

View reviewed changes

trevor-scheer merged commit 9bd7748 into main Dec 18, 2023
21 checks passed

trevor-scheer deleted the bnjjj/update_callback_plugin branch December 18, 2023 19:04

github-actions bot mentioned this pull request Dec 18, 2023

Version Packages #7780

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jan 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade to subscription callback protocol GA #7793

upgrade to subscription callback protocol GA #7793

bnjjj commented Nov 29, 2023 •

edited by trevor-scheer

Loading

netlify bot commented Nov 29, 2023 •

edited

Loading

bnjjj commented Nov 29, 2023

codesandbox-ci bot commented Nov 29, 2023 •

edited

Loading

trevor-scheer commented Dec 7, 2023

trevor-scheer left a comment

trevor-scheer Dec 14, 2023

upgrade to subscription callback protocol GA #7793

upgrade to subscription callback protocol GA #7793

Conversation

bnjjj commented Nov 29, 2023 • edited by trevor-scheer Loading

netlify bot commented Nov 29, 2023 • edited Loading

✅ Deploy Preview for apollo-server-docs ready!

bnjjj commented Nov 29, 2023

codesandbox-ci bot commented Nov 29, 2023 • edited Loading

trevor-scheer commented Dec 7, 2023

trevor-scheer left a comment

Choose a reason for hiding this comment

trevor-scheer Dec 14, 2023

Choose a reason for hiding this comment

bnjjj commented Nov 29, 2023 •

edited by trevor-scheer

Loading

netlify bot commented Nov 29, 2023 •

edited

Loading

codesandbox-ci bot commented Nov 29, 2023 •

edited

Loading