usage reporting/inline trace plugins: mask errors, change option name (…

…#6794)

We replace the `rewriteError` option with `sendErrorsInTraces` (usage
reporting) and `includeErrors` (inline trace), which takes `{unmodified:
true}`, `{masked:true}`, and `{transform}` variants. This is similar to
`sendVariableValues` and `sendHeaders`.

Like those two options, this now defaults to a more redacted version:
`{masked: true}`. This will reduce unintentional reporting of PII.

Part of #6051.

Paired with @bonnici.

.changeset/twenty-cooks-repair.md

@@ -0,0 +1,6 @@
+---
+"@apollo/server-integration-testsuite": patch
+"@apollo/server": patch
+---
+
+Usage reporting and inline trace plugins: replace `rewriteError` with `sendErrorsInTraces`/`includeErrors`, and mask all errors by default.

docs/source/migration.mdx

@@ -930,6 +930,50 @@ ApolloServerPluginUsageReporting({
 });
 ```
 
+### `rewriteError` plugin option
+
+In Apollo Server 3, you can specify a function to rewrite errors before sending them to Apollo's server via the `rewriteError` option to `ApolloServerPluginUsageReporting` (for monoliths) and `ApolloServerPluginInlineTrace` (for subgraphs).
+
+In Apollo Server 4, you specify the same function as the `transform` option on the `sendErrorsInTrace` option to `ApolloServerPluginUsageReporting` and the `includeErrors` option to `ApolloServerPluginInlineTrace`.
+
+(Additionally, the [default behavior has changed to mask errors](#usage-reporting-and-inline-trace-plugins-mask-errors-by-default).)
+
+So, where you previously wrote:
+
+```ts
+// monoliths
+new ApolloServer({
+  plugins: [ApolloServerPluginUsageReporting({ rewriteError })],
+  // ...
+})
+
+// subgraphs
+new ApolloServer({
+  plugins: [ApolloServerPluginInlineTrace({ rewriteError })],
+  // ...
+})
+```
+
+you can now write:
+
+```ts
+// monoliths
+new ApolloServer({
+  plugins: [ApolloServerPluginUsageReporting({
+    sendErrorsInTrace: { transform: rewriteError },
+  })],
+  // ...
+})
+
+// subgraphs
+new ApolloServer({
+  plugins: [ApolloServerPluginInlineTrace({
+    includeErrors: { transform: rewriteError },
+  })],
+  // ...
+})
+```
+
 
 ### Doubly-escaped `variables` and `extensions` in requests
 
@@ -1503,6 +1547,35 @@ new ApolloServer({
 
 </MultiCodeBlock>
 
+
+### Usage reporting and inline trace plugins mask errors by default
+
+In Apollo Server 3, traces sent to Apollo's servers from monolith servers by the usage reporting plugin contain the full message and extensions of every GraphQL error that occurs in the operation by default. Similarly, inline traces sent from subgraphs to Apollo Gateways (which are then sent to Apollo's servers) contain full error details by default. You can modify or drop these errors with `rewriteError` options to the appropriate plugins.
+
+In Apollo Server 4, error details are *not* included in traces by default. By default, error messages are replaced with `<masked>` and error extensions are replaced with a single extension `maskedBy` naming the plugin which performed the masking (`ApolloServerPluginUsageReporting` or `ApolloServerPluginInlineTrace`).
+
+To restore the Apollo Server 3 behavior, you can pass `{ unmodified: true }` to an option on each plugin:
+
+```ts
+// monoliths
+new ApolloServer({
+  plugins: [ApolloServerPluginUsageReporting({
+    sendErrorsInTrace: { unmodified: true },
+  })],
+  // ...
+})
+
+// subgraphs
+new ApolloServer({
+  plugins: [ApolloServerPluginInlineTrace({
+    includeErrors: { unmodified: true },
+  })],
+  // ...
+})
+```
+
+(As [described above](#rewriteerror-plugin-option), the `rewriteError` option has been replaced by a `transform` option on `sendErrorsInTrace` or `includeErrors`.)
+
 ## Renamed packages
 
 The following packages have been renamed in Apollo Server 4:

packages/integration-testsuite/src/apolloServerTests.ts

@@ -1139,15 +1139,17 @@ export function defineIntegrationTestSuiteApolloServerTests(
           });
 
           describe('error munging', () => {
-            describe('rewriteError', () => {
+            describe('sendErrorsInTraces', () => {
               it('new error', async () => {
                 throwError.mockImplementationOnce(() => {
-                  throw new Error('rewriteError nope');
+                  throw new Error('transform nope');
                 });
 
                 await setupApolloServerAndFetchPair({
-                  rewriteError: () =>
-                    new GraphQLError('rewritten as a new error'),
+                  sendErrorsInTraces: {
+                    transform: () =>
+                      new GraphQLError('rewritten as a new error'),
+                  },
                 });
 
                 const result = await apolloFetch({
@@ -1159,7 +1161,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
                 expect(result.errors).toBeDefined();
 
                 // The original error message should be sent to the client.
-                expect(result.errors[0].message).toEqual('rewriteError nope');
+                expect(result.errors[0].message).toEqual('transform nope');
                 expect(throwError).toHaveBeenCalledTimes(1);
 
                 const reports = await reportIngress.promiseOfReports;
@@ -1185,13 +1187,15 @@ export function defineIntegrationTestSuiteApolloServerTests(
 
               it('modified error', async () => {
                 throwError.mockImplementationOnce(() => {
-                  throw new Error('rewriteError mod nope');
+                  throw new Error('transform mod nope');
                 });
 
                 await setupApolloServerAndFetchPair({
-                  rewriteError: (err) => {
-                    err.message = 'rewritten as a modified error';
-                    return err;
+                  sendErrorsInTraces: {
+                    transform: (err) => {
+                      err.message = 'rewritten as a modified error';
+                      return err;
+                    },
                   },
                 });
 
@@ -1202,9 +1206,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
                   fieldWhichWillError: null,
                 });
                 expect(result.errors).toBeDefined();
-                expect(result.errors[0].message).toEqual(
-                  'rewriteError mod nope',
-                );
+                expect(result.errors[0].message).toEqual('transform mod nope');
                 expect(throwError).toHaveBeenCalledTimes(1);
 
                 const reports = await reportIngress.promiseOfReports;
@@ -1230,11 +1232,11 @@ export function defineIntegrationTestSuiteApolloServerTests(
 
               it('nulled error', async () => {
                 throwError.mockImplementationOnce(() => {
-                  throw new Error('rewriteError null nope');
+                  throw new Error('transform null nope');
                 });
 
                 await setupApolloServerAndFetchPair({
-                  rewriteError: () => null,
+                  sendErrorsInTraces: { transform: () => null },
                 });
 
                 const result = await apolloFetch({
@@ -1244,9 +1246,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
                   fieldWhichWillError: null,
                 });
                 expect(result.errors).toBeDefined();
-                expect(result.errors[0].message).toEqual(
-                  'rewriteError null nope',
-                );
+                expect(result.errors[0].message).toEqual('transform null nope');
                 expect(throwError).toHaveBeenCalledTimes(1);
 
                 const reports = await reportIngress.promiseOfReports;
@@ -1267,11 +1267,14 @@ export function defineIntegrationTestSuiteApolloServerTests(
 
             it('undefined error', async () => {
               throwError.mockImplementationOnce(() => {
-                throw new Error('rewriteError undefined whoops');
+                throw new Error('transform undefined whoops');
               });
 
               await setupApolloServerAndFetchPair({
-                rewriteError: () => undefined as any,
+                sendErrorsInTraces: {
+                  // @ts-expect-error (not allowed to be undefined)
+                  transform: () => undefined,
+                },
               });
 
               const result = await apolloFetch({
@@ -1282,7 +1285,7 @@ export function defineIntegrationTestSuiteApolloServerTests(
               });
               expect(result.errors).toBeDefined();
               expect(result.errors[0].message).toEqual(
-                'rewriteError undefined whoops',
+                'transform undefined whoops',
               );
               expect(throwError).toHaveBeenCalledTimes(1);
 
@@ -1301,8 +1304,8 @@ export function defineIntegrationTestSuiteApolloServerTests(
               // rewritten.
               expect(trace.root!.child![0].error).toMatchObject([
                 {
-                  json: '{"message":"rewriteError undefined whoops","locations":[{"line":1,"column":2}],"path":["fieldWhichWillError"]}',
-                  message: 'rewriteError undefined whoops',
+                  json: '{"message":"transform undefined whoops","locations":[{"line":1,"column":2}],"path":["fieldWhichWillError"]}',
+                  message: 'transform undefined whoops',
                   location: [{ column: 2, line: 1 }],
                 },
               ]);
@@ -2203,9 +2206,11 @@ export function defineIntegrationTestSuiteApolloServerTests(
           },
           plugins: [
             ApolloServerPluginInlineTrace({
-              rewriteError(err) {
-                err.message = `Rewritten for Usage Reporting: ${err.message}`;
-                return err;
+              includeErrors: {
+                transform(err) {
+                  err.message = `Rewritten for Usage Reporting: ${err.message}`;
+                  return err;
+                },
               },
             }),
           ],

packages/server/src/ApolloServer.ts

@@ -59,6 +59,7 @@ import {
 } from './runHttpQuery.js';
 import { SchemaManager } from './utils/schemaManager.js';
 import { isDefined } from './utils/isDefined.js';
+import { UnreachableCaseError } from './utils/UnreachableCaseError.js';
 import type { WithRequired } from '@apollo/utils.withrequired';
 import type { ApolloServerOptionsWithStaticSchema } from './externalTypes/constructor';
 import type { GatewayExecutor } from '@apollo/server-gateway-interface';
@@ -128,15 +129,6 @@ type ServerState =
       stopError: Error | null;
     };
 
-// Throw this in places that should be unreachable (because all other cases have
-// been handled, reducing the type of the argument to `never`). TypeScript will
-// complain if in fact there is a valid type for the argument.
-class UnreachableCaseError extends Error {
-  constructor(val: never) {
-    super(`Unreachable case: ${val}`);
-  }
-}
-
 // TODO(AS4): Move this to its own file or something. Also organize the fields.
 
 export interface ApolloServerInternals<TContext extends BaseContext> {

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

@@ -1,18 +1,31 @@
 import { Trace } from '@apollo/usage-reporting-protobuf';
 import { TraceTreeBuilder } from '../traceTreeBuilder.js';
-import type { ApolloServerPluginUsageReportingOptions } from '../usageReporting/options';
+import type { SendErrorsOptions } from '../usageReporting/index.js';
 import { internalPlugin } from '../../internalPlugin.js';
 import { schemaIsFederated } from '../schemaIsFederated.js';
 import type { ApolloServerPlugin, BaseContext } from '../../externalTypes';
 
 export interface ApolloServerPluginInlineTraceOptions {
   /**
-   * By default, all errors from this service get included in the trace.  You
-   * can specify a filter function to exclude specific errors from being
-   * reported by returning an explicit `null`, or you can mask certain details
-   * of the error by modifying it and returning the modified error.
+   * By default, if a trace contains errors, the errors are included in the
+   * trace with the message `<masked>`. The errors are associated with specific
+   * paths in the operation, but do not include the original error message or
+   * any extensions such as the error `code`, as those details may contain your
+   * users' private data. The extension `maskedBy:
+   * 'ApolloServerPluginInlineTrace'` is added.
+   *
+   * If you'd like details about the error included in traces, set this option.
+   * This option can take several forms:
+   *
+   * - { masked: true }: mask error messages and omit extensions (DEFAULT)
+   * - { unmodified: true }: include all error messages and extensions
+   * - { transform: ... }: a custom function for transforming errors. This
+   *   function receives a `GraphQLError` and may return a `GraphQLError`
+   *   (either a new error, or its potentially-modified argument) or `null`.
+   *   This error is used in the trace; if `null`, the error is not included in
+   *   traces or error statistics.
    */
-  rewriteError?: ApolloServerPluginUsageReportingOptions<never>['rewriteError'];
+  includeErrors?: SendErrorsOptions;
   /**
    * This option is for internal use by `@apollo/server` only.
    *
@@ -60,7 +73,8 @@ export function ApolloServerPluginInlineTrace<TContext extends BaseContext>(
       }
 
       const treeBuilder = new TraceTreeBuilder({
-        rewriteError: options.rewriteError,
+        maskedBy: 'ApolloServerPluginInlineTrace',
+        sendErrors: options.includeErrors,
       });
 
       // XXX Provide a mechanism to customize this logic.