Repository: apollographql/apollo-server
Pull request #7843 (2024-03-05 09:55:02)

[fix] whenResultIsFinished works on Promise of Array of Promises
Paired with @bbeesley.

Updates whenResultIsFinished to call callback also on Promise of Array of Promises.
Fixes (partially) https://github.com/apollographql/apollo-server/issues/5372 and https://github.com/apollographql/apollo-server/issues/4667.

The test case we're fixing is `'passes result of Promise of Array of Promises to the callback'`, the other tests test existing behaviour.

The existing bug prevented us from dynamically setting cache hints in the reference resolver depending on the result of an asynchronous operation. We discovered the bug within an integration test. If you @glasser have an idea how to add an integration test and would like to add it, that would be amazing!


apollo-cla: @bscherlein: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/
netlify[bot]: ### <span aria-hidden="true">👷</span> Deploy request for *apollo-server-docs* pending review.
[Visit the deploys page to approve it](https://app.netlify.com/sites/apollo-server-docs/deploys)

|  Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 235d9405de42d5a3c876ceea85e129821bb93707 |
codesandbox-ci[bot]: This pull request is automatically built and testable in [CodeSandbox](https://codesandbox.io).

  To see build info of the built libraries, click [here](https://ci.codesandbox.io/status/apollographql/apollo-server/pr/7843/builds/480092) or the icon next to each commit SHA.

Patch:
diff --git a/.changeset/long-crabs-reply.md b/.changeset/long-crabs-reply.md
new file mode 100644
index 000000000..79fc2cb01
--- /dev/null
+++ b/.changeset/long-crabs-reply.md
@@ -0,0 +1,5 @@
+---
+"@apollo/server": patch
+---
+
+Improves timing of the `willResolveField` end hook on fields which return Promises resolving to Arrays. This makes the use of the `setCacheHint` method more reliable.
diff --git a/.changeset/loud-oranges-melt.md b/.changeset/loud-oranges-melt.md
new file mode 100644
index 000000000..a845151cc
--- /dev/null
+++ b/.changeset/loud-oranges-melt.md
@@ -0,0 +1,2 @@
+---
+---
diff --git a/.circleci/config.yml b/.circleci/config.yml
index ad05bc72a..44938d685 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -64,7 +64,7 @@ jobs:
           node-version: "18"
       # Install a prerelease of graphql-js 17 with incremental delivery support.
       # --legacy-peer-deps because nothing expects v17 yet.
-      - run: npm i --legacy-peer-deps graphql@17.0.0-alpha.1.canary.pr.3361.04ab27334641e170ce0e05bc927b972991953882
+      - run: npm i --legacy-peer-deps graphql@17.0.0-alpha.2
       - run: npm run test:ci
       - run: npm run test:smoke
 
diff --git a/docs/source/schema/custom-scalars.mdx b/docs/source/schema/custom-scalars.mdx
index fede86a49..85d228035 100644
--- a/docs/source/schema/custom-scalars.mdx
+++ b/docs/source/schema/custom-scalars.mdx
@@ -16,7 +16,14 @@ scalar MyCustomScalar
 
 You can now use `MyCustomScalar` in your schema anywhere you can use a default scalar (e.g., as the type of an object field, input type field, or argument).
 
-However, Apollo Server still needs to know how to interact with values of this new scalar type.
+### Including scalar specification
+Updated in the [October 2021 version of the GraphQL spec](https://spec.graphql.org/October2021/#sec--specifiedBy), you can include a `@specifiedBy` directive as metadata for schema consumers to understand what format the scalar is using. This directive does not provide automatic validation, but can provide useful context for humans reading the schema.
+
+```graphql
+scalar MyCustomScalar @specifiedBy(url: "https://specs.example.com/rfc111")
+```
+
+However, Apollo Server still needs to know how to interact with and generate values of this new scalar type.
 
 ## Defining custom scalar logic
 
diff --git a/docs/source/schema/schema.md b/docs/source/schema/schema.md
index 816253e6a..7cf406c04 100644
--- a/docs/source/schema/schema.md
+++ b/docs/source/schema/schema.md
@@ -55,6 +55,8 @@ type Author {
 }
 ```
 
+> List fields can be nested by using multiple pairs of square brackets `[[]]`.
+
 ### Field nullability
 
 By default, it's valid for any field in your schema to return `null` instead of its specified type. You can require that a particular field _doesn't_ return `null` with an exclamation mark `!`, like so:
@@ -82,6 +84,15 @@ type Author {
 - If `!` appears _outside_ the square brackets, _the list itself_ can't be `null`.
 - In _any_ case, it's valid for a list field to return an _empty_ list.
 
+Based on the above principles, the below return types can potentially return these sample values:
+
+| Return Type | Example Allowed Return Values                            |
+|-------------|----------------------------------------------------------|
+| `[Book]`    | `[]`, `null`, `[null]`, and `[{title: "City of Glass"}]` |
+| `[Book!]`   | `[]`, `null`, and `[{title: "City of Glass"}]`           |
+| `[Book]!`   | `[]`, `[null]`, and `[{title: "City of Glass"}]`         |
+| `[Book!]!`  | `[]` and `[{title: "City of Glass"}]`                    |
+
 ## Supported types
 
 Every type definition in a GraphQL schema belongs to one of the following categories:
diff --git a/docs/source/security/authentication.mdx b/docs/source/security/authentication.mdx
index 7b87727d0..0f423650a 100644
--- a/docs/source/security/authentication.mdx
+++ b/docs/source/security/authentication.mdx
@@ -10,6 +10,14 @@ Your GraphQL API probably needs to control which users can see and interact with
 - **Authentication** is determining whether a given user is logged in, and subsequently determining _which_ user someone is.
 - **Authorization** is then determining what a given user has permission to do or see.
 
+<Tip>
+
+Apollo Router can now provide authentication and authorization for your entire supergraph. While it may make sense to re-apply auth checks at a subgraph level or in a monolith graph, the Apollo Router has built and provided standard JWT checks that can be setup with a simple YAML configuration and enforce this in a central location for all subgraphs:
+
+https://www.apollographql.com/blog/centrally-enforce-policy-as-code-for-graphql-apis
+
+</Tip>
+
 ## Putting authenticated user info in your `contextValue`
 
 <TopLevelAwait />
diff --git a/docs/source/workflow/generate-types.mdx b/docs/source/workflow/generate-types.mdx
index 59ebf1192..18fc22fba 100644
--- a/docs/source/workflow/generate-types.mdx
+++ b/docs/source/workflow/generate-types.mdx
@@ -163,9 +163,7 @@ import { QueryResolvers } from '__generated__/resolvers-types';
 // Use the generated `QueryResolvers`
 // type to type check our queries!
 const queries: QueryResolvers = {
-  Query: {
   // ...queries
-  },
 };
 
 export default queries;
@@ -177,9 +175,7 @@ import { MutationResolvers } from '__generated__/resolvers-types';
 // Use the generated `MutationResolvers` type
 // to type check our mutations!
 const mutations: MutationResolvers = {
-  Mutation: {
-    // ...mutations
-  },
+  // ...mutations
 };
 
 export default mutations;
diff --git a/docs/source/workflow/requests.md b/docs/source/workflow/requests.md
index c057221a0..f06096eff 100644
--- a/docs/source/workflow/requests.md
+++ b/docs/source/workflow/requests.md
@@ -91,7 +91,7 @@ For more details, see [the CSRF prevention documentation](../security/cors#preve
 
 ## Incremental delivery (experimental)
 
-Incremental delivery is a [Stage 2: Draft Proposal](https://github.com/graphql/graphql-spec/pull/742) to the GraphQL specification which adds `@defer` and `@stream` executable directives. These directives allow clients to specify that parts of an operation can be sent after an initial response, so that slower fields do not delay all other fields. As of September 2022, the `graphql` library (also known as `graphql-js`) upon which Apollo Server is built implements incremental delivery only in the unreleased major version 17. If a pre-release of `graphql@17` is installed in your server, Apollo Server 4 can execute these incremental delivery directives and provide streaming [`multipart/mixed`](https://github.com/graphql/graphql-over-http/blob/main/rfcs/IncrementalDelivery.md) responses.
+Incremental delivery is a [Stage 2: Draft Proposal](https://github.com/graphql/graphql-spec/pull/742) to the GraphQL specification which adds `@defer` and `@stream` executable directives. These directives allow clients to specify that parts of an operation can be sent after an initial response, so that slower fields do not delay all other fields. As of September 2022, the `graphql` library (also known as `graphql-js`) upon which Apollo Server is built implements incremental delivery only in the unreleased major version 17. If a pre-release of `graphql@17.0.0-alpha.2` is installed in your server, Apollo Server 4 can execute these incremental delivery directives and provide streaming [`multipart/mixed`](https://github.com/graphql/graphql-over-http/blob/main/rfcs/IncrementalDelivery.md) responses.
 
 Support for incremental delivery in graphql version 17 is [opt-in](https://github.com/robrichard/defer-stream-wg/discussions/12), meaning the directives are not defined by default. In order to use `@defer` or `@stream`, you must provide the appropriate definition(s) in your SDL. The definitions below can be pasted into your schema as-is:
 
diff --git a/packages/server/src/utils/schemaInstrumentation.ts b/packages/server/src/utils/schemaInstrumentation.ts
index c2880f92b..d6295e7f7 100644
--- a/packages/server/src/utils/schemaInstrumentation.ts
+++ b/packages/server/src/utils/schemaInstrumentation.ts
@@ -106,14 +106,15 @@ function isPromise(x: any): boolean {
 
 // Given result (which may be a Promise or an array some of whose elements are
 // promises) Promises, set up 'callback' to be invoked when result is fully
-// resolved.
+// resolved. (Unfortunately, this does not perfectly handle every possible
+// return value shape, such as arrays of arrays of Promises.)
 export function whenResultIsFinished(
   result: any,
   callback: (err: Error | null, result?: any) => void,
 ) {
   if (isPromise(result)) {
     result.then(
-      (r: any) => callback(null, r),
+      (r: any) => whenResultIsFinished(r, callback),
       (err: Error) => callback(err),
     );
   } else if (Array.isArray(result)) {


Test patch:
diff --git a/packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts b/packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts
index 4dc843ead..a622c6177 100644
--- a/packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts
+++ b/packages/server/src/__tests__/plugin/subscriptionCallback/index.test.ts
@@ -892,6 +892,134 @@ describe('SubscriptionCallbackPlugin', () => {
     `);
   });
 
+  it('handles router termination via 404', async () => {
+    const server = await startSubscriptionServer({ logger });
+
+    mockRouterCheckResponse();
+    // Mock the heartbeat response from the router. We'll trigger it once below
+    // after the subscription is initialized to make sure it works.
+    const heartbeat = mockRouterCheckResponse();
+
+    // Start the subscriptions; this triggers the initial check request and
+    // starts the heartbeat interval. This simulates an incoming subscription
+    // request from the router.
+    const result = await server.executeHTTPGraphQLRequest(
+      buildHTTPGraphQLRequest({
+        body: {
+          query: `#graphql
+            subscription {
+              count
+            }
+          `,
+          extensions: {
+            subscription: {
+              callbackUrl: 'http://mock-router-url.com',
+              subscriptionId: '1234-cats',
+              verifier: 'my-verifier-token',
+            },
+          },
+        },
+      }),
+    );
+
+    expect(result.status).toEqual(200);
+
+    // Advance timers to trigger the heartbeat. This consumes the one
+    // heartbeat mock from above.
+    jest.advanceTimersByTime(5000);
+    await heartbeat;
+
+    // Next we'll trigger some subscription events. In advance, we'll mock the
+    // router responses.
+    const firstUpdate = mockRouterNextResponse({ payload: { count: 1 } });
+    const secondUpdate = mockRouterNextResponse({ payload: { count: 2 } });
+
+    // Trigger a couple updates. These send `next` requests to the router.
+    logger.debug('TESTING: Triggering first update');
+    await server.executeHTTPGraphQLRequest(
+      buildHTTPGraphQLRequest({
+        body: {
+          query: `#graphql
+            mutation {
+              addOne
+            }
+          `,
+        },
+      }),
+    );
+
+    await firstUpdate;
+
+    logger.debug('TESTING: Triggering second update');
+    await server.executeHTTPGraphQLRequest(
+      buildHTTPGraphQLRequest({
+        body: {
+          query: `#graphql
+            mutation {
+              addOne
+            }
+          `,
+        },
+      }),
+    );
+
+    await secondUpdate;
+
+    // We've established two subscriptions are functional at this point. Now
+    // let's have the router invalidate them with a 404 response to the `next`.
+    const terminationResponse = mockRouterNextResponse({
+      statusCode: 404,
+      payload: { count: 3 },
+    });
+
+    // Trigger a 3rd update to make sure both subscriptions are cancelled.
+    logger.debug('TESTING: Triggering third (terminating) update');
+    await server.executeHTTPGraphQLRequest(
+      buildHTTPGraphQLRequest({
+        body: {
+          query: `#graphql
+            mutation {
+              addOne
+            }
+          `,
+        },
+      }),
+    );
+
+    await terminationResponse;
+
+    await server.stop();
+
+    expect(logger.orderOfOperations).toMatchInlineSnapshot(`
+      [
+        "SubscriptionCallback[1234-cats]: Received new subscription request",
+        "SubscriptionManager[1234-cats]: Sending \`check\` request to router",
+        "SubscriptionManager[1234-cats]: \`check\` request successful",
+        "SubscriptionCallback[1234-cats]: Starting graphql-js subscription",
+        "SubscriptionCallback[1234-cats]: graphql-js subscription successful",
+        "SubscriptionManager[1234-cats]: Starting new heartbeat interval for http://mock-router-url.com",
+        "SubscriptionManager[1234-cats]: Listening to graphql-js subscription",
+        "SubscriptionCallback[1234-cats]: Responding to original subscription request",
+        "SubscriptionManager: Sending \`check\` request to http://mock-router-url.com for ID: 1234-cats",
+        "SubscriptionManager: Heartbeat received response for ID: 1234-cats",
+        "SubscriptionManager: Heartbeat request successful, ID: 1234-cats",
+        "TESTING: Triggering first update",
+        "SubscriptionManager[1234-cats]: Sending \`next\` request to router",
+        "SubscriptionManager[1234-cats]: \`next\` request successful",
+        "TESTING: Triggering second update",
+        "SubscriptionManager[1234-cats]: Sending \`next\` request to router",
+        "SubscriptionManager[1234-cats]: \`next\` request successful",
+        "TESTING: Triggering third (terminating) update",
+        "SubscriptionManager[1234-cats]: Sending \`next\` request to router",
+        "SubscriptionManager[1234-cats]: \`next\` request received 404, terminating subscription",
+        "SubscriptionManager: Terminating subscriptions for ID: 1234-cats",
+        "SubscriptionManager: Terminating heartbeat interval for http://mock-router-url.com",
+        "SubscriptionCallback: Server is shutting down. Cleaning up outstanding subscriptions and heartbeat intervals",
+        "SubscriptionCallback: Successfully cleaned up outstanding subscriptions and heartbeat intervals.",
+      ]
+    `);
+  });
+
   it('sends a `complete` when a subscription terminates successfully', async () => {
     const server = await startSubscriptionServer({ logger });
 
diff --git a/packages/server/src/__tests__/utils/schemaInstrumentation.test.ts b/packages/server/src/__tests__/utils/schemaInstrumentation.test.ts
new file mode 100644
index 000000000..ef5c9782e
--- /dev/null
+++ b/packages/server/src/__tests__/utils/schemaInstrumentation.test.ts
@@ -0,0 +1,37 @@
+import { describe, expect, it, jest } from '@jest/globals';
+import { whenResultIsFinished } from '../../utils/schemaInstrumentation';
+
+describe('whenResultIsFinished', () => {
+  it('passes result of Promise to the callback', async () => {
+    const expected = 1;
+    const result = Promise.resolve(expected);
+    const callback = jest.fn();
+    whenResultIsFinished(result, callback);
+    await new Promise((r) => setImmediate(r));
+    expect(callback).toBeCalledWith(null, expected);
+  });
+  it('passes result of Array of Promises to the callback', async () => {
+    const expected = 1;
+    const result = [Promise.resolve(expected)];
+    const callback = jest.fn();
+    whenResultIsFinished(result, callback);
+    await new Promise((r) => setImmediate(r));
+    expect(callback).toBeCalledWith(null, [expected]);
+  });
+  it('passes result which is not asynchronous directly to the callback', async () => {
+    const expected = 1;
+    const result = expected;
+    const callback = jest.fn();
+    whenResultIsFinished(result, callback);
+    await new Promise((r) => setImmediate(r));
+    expect(callback).toBeCalledWith(null, expected);
+  });
+  it('passes result of Promise of Array of Promises to the callback', async () => {
+    const expected = 1;
+    const result = Promise.resolve([Promise.resolve(expected)]);
+    const callback = jest.fn();
+    whenResultIsFinished(result, callback);
+    await new Promise((r) => setImmediate(r));
+    expect(callback).toBeCalledWith(null, [expected]);
+  });
+});
diff --git a/smoke-test/prepare.sh b/smoke-test/prepare.sh
index 7352432a8..a5f6b0de8 100755
--- a/smoke-test/prepare.sh
+++ b/smoke-test/prepare.sh
@@ -21,7 +21,7 @@ npm i
 if [[ -n "${INCREMENTAL_DELIVERY_TESTS_ENABLED:-}" ]]; then
   npm i --no-save --legacy-peer-deps \
     "$TARBALL_DIR"/*.tgz \
-    graphql@17.0.0-alpha.1.canary.pr.3361.04ab27334641e170ce0e05bc927b972991953882
+    graphql@17.0.0-alpha.2
 else
   npm i --no-save "$TARBALL_DIR"/*.tgz
 fi