fix: Introduce status code regression mitigation

.changeset/purple-paws-yell.md

@@ -0,0 +1,16 @@
+---
+'@apollo/server': minor
+---
+
+Introduce new opt-in configuration option to mitigate v4 status code regression
+
+Apollo Server v4 accidentally started responding to requests with an invalid `variables` object with a 200 status code, where v3 previously responded with a 400. In order to not break current behavior (potentially breaking users who have creatively worked around this issue) and offer a mitigation, we've added the following configuration option which we recommend for all users.
+
+```ts
+new ApolloServer({
+  // ...
+  status400WithErrorsAndNoData: true,
+});
+```
+
+This will become the default behavior in Apollo Server v5 and the configuration option will be ignored / no longer needed.

docs/source/api/apollo-server.mdx

@@ -475,6 +475,21 @@ If this is set to any string value, use that value instead of the environment va
 </td>
 </tr>
 
+<tr>
+<td>
+
+###### `status400WithErrorsAndNoData`
+
+`boolean`
+
+</td>
+<td>
+
+This option is a v4 regression mitigation that we recommend users set to `true`. This option will be ignored in Apollo Server v5 and its `true` behavior will become the default. Visit the [migration guide](../migration/#known-regressions) for more details on the regression.
+
+</td>
+</tr>
+
 </tbody>
 </table>
 

docs/source/data/errors.mdx

@@ -5,6 +5,8 @@ description: Making errors actionable on the client and server
 
 import TopLevelAwait from "../shared/top-level-await.mdx"
 
+> Apollo Server v4 introduced a regression where providing invalid variables incorrectly yields a 200 status code. The mitigation for this regression is to provide the `status400WithErrorsAndNoData: true` option to your `ApolloServer` constructor. More details can be found [in the migration guide](../migration/#known-regressions).
+
 <!-- cSpell:ignore typenam -->
 
 Whenever Apollo Server encounters errors while processing a GraphQL operation, its response to the client includes an `errors` array containing each error that occurred. Each error in the array has an `extensions` field that provides additional useful information, including an error `code` and (while in development mode) a `stacktrace`.

docs/source/migration.mdx

@@ -344,6 +344,23 @@ You can also import each plugin's associated TypeScript types (e.g., `ApolloServ
 
 Once you've updated your imports, you can remove your project's dependency on `apollo-server-core`.
 
+## Known regressions
+
+### Appropriate 400 status codes
+
+Apollo Server v4 will respond to an invalid `variables` object with a _200_ status code, where v3 would correctly respond with a 400 status code. This regression was introduced in [PR #6502](https://github.com/apollographql/apollo-server/pull/6502) and brought to our attention in [Issue #7462](https://github.com/apollographql/apollo-server/issues/7462).
+
+The mitigation for this regression is an opt-in configuration option that we recommend for all users who have not already worked around this regression. Simply add the `status400WithErrorsAndNoData: true` option to your `ApolloServer` constructor like so:
+
+```ts
+new ApolloServer({
+  // ...
+  status400WithErrorsAndNoData: true,
+})
+```
+
+This option will no longer be needed (and be ignored) in Apollo Server v5.
+
 ## Bumped dependencies
 
 ### Node.js

packages/server/src/ApolloServer.ts

@@ -174,7 +174,9 @@ export interface ApolloServerInternals<TContext extends BaseContext> {
   rootValue?: ((parsedQuery: DocumentNode) => unknown) | unknown;
   validationRules: Array<ValidationRule>;
   fieldResolver?: GraphQLFieldResolver<any, TContext>;
-
+  // TODO(AS5): remove OR warn + ignore with this option set, ignore option and
+  // flip default behavior.
+  status400WithErrorsAndNoData?: boolean;
   __testing_incrementalExecutionResults?: GraphQLExperimentalIncrementalExecutionResults;
 }
 
@@ -325,6 +327,8 @@ export class ApolloServer<in out TContext extends BaseContext = BaseContext> {
           ? null
           : config.csrfPrevention.requestHeaders ??
             recommendedCsrfPreventionRequestHeaders,
+      status400WithErrorsAndNoData:
+        config.status400WithErrorsAndNoData ?? false,
       __testing_incrementalExecutionResults:
         config.__testing_incrementalExecutionResults,
     };

packages/server/src/__tests__/ApolloServer.test.ts

@@ -23,6 +23,7 @@ const typeDefs = gql`
     hello: String
     error: Boolean
     contextFoo: String
+    needsStringArg(aString: String): String
   }
 `;
 
@@ -378,6 +379,31 @@ describe('ApolloServer executeOperation', () => {
     await server.stop();
   });
 
+  // TODO(AS5): expect an update here when default flips
+  it.each([
+    { status400WithErrorsAndNoData: false, expectedStatus: undefined },
+    { status400WithErrorsAndNoData: true, expectedStatus: 400 },
+  ])(
+    'variable coercion errors',
+    async ({ status400WithErrorsAndNoData, expectedStatus }) => {
+      const server = new ApolloServer({
+        typeDefs,
+        resolvers,
+        status400WithErrorsAndNoData,
+      });
+      await server.start();
+
+      const { body, http } = await server.executeOperation({
+        query: 'query NeedsArg($arg: String) { needsStringArg(aString: $arg) }',
+        variables: { arg: 1 },
+      });
+      const result = singleResult(body);
+      expect(result.errors?.[0].extensions?.code).toBe('BAD_USER_INPUT');
+      expect(http.status).toBe(expectedStatus);
+      await server.stop();
+    },
+  );
+
   it('passes its second argument as context object', async () => {
     const server = new ApolloServer({
       typeDefs,

packages/server/src/externalTypes/constructor.ts

@@ -102,6 +102,15 @@ interface ApolloServerOptionsBase<TContext extends BaseContext> {
   // parsing the schema.
   parseOptions?: ParseOptions;
 
+  // TODO(AS5): remove OR warn + ignore with this option set, ignore option and
+  // flip default behavior. Default false. This opt-in configuration fixes a
+  // regression introduced in v4. In v3, Apollo Server would correctly respond
+  // to a request with invalid `variables` with a 400 status code. AS4 responds
+  // with a 200 status code by default. We recommend setting this to `true`
+  // unless you've explicitly worked around this regression already (and maybe
+  // consider undoing the workaround).
+  status400WithErrorsAndNoData?: boolean;
+
   // For testing only.
   __testing_incrementalExecutionResults?: GraphQLExperimentalIncrementalExecutionResults;
 }

packages/server/src/requestPipeline.ts

@@ -474,6 +474,18 @@ export async function processGraphQLRequest<TContext extends BaseContext>(
       const { formattedErrors, httpFromErrors } = resultErrors
         ? formatErrors(resultErrors)
         : { formattedErrors: undefined, httpFromErrors: newHTTPGraphQLHead() };
+
+      // TODO(AS5) This becomes the default behavior and the
+      // `status400WithErrorsAndNoData` configuration option is removed /
+      // ignored.
+      if (
+        internals.status400WithErrorsAndNoData &&
+        resultErrors?.length &&
+        result.data === undefined
+      ) {
+        httpFromErrors.status = httpFromErrors.status ?? 400;
+      }
+
       mergeHTTPGraphQLHead(requestContext.response.http, httpFromErrors);
 
       if ('singleResult' in fullResult) {