feat(apollo-engine-reporting) Introduce rewriteError to munge error…

…s for reporting. (#2618) * Add errorFilter option * update changelog * Rename errorFilter to filterErrors, return GraphQLError or null * Add back `maskErrorDetails` type to maintain support. ...though it's currently not functional as of this commit. * Introduce additional tests for `filterErrors` and `maskErrorDetails`. Even though `maskErrorDetails` is going away, it seems important to continue to test it along with the new functionality introduced by `filterErrors`. Note: These tests highlight some failures in the current `filterErrors` logic which need to be addressed in follow-up commits. * Finish up new `filterErrors` functionality, sans renaming to a new name. The intention is to rename the `filterErrors` function to `rewriteError` in a follow-up commit, but this maintains the previous name during the re-implementation phase. * Add a test that ensures that the `stack` is not transmitted. The new implementation of `rewriteError` (previously `formatErrors` and prior to that `maskErrorDetails`, do nothing to ensure that the `stack` property (i.e. `Error.prototype.stack`) is regenerated using the new combination of `${err.name}: ${err.message`, suffixed with the rest of `Error.captureStackTrace`. That's okay, but we should at least guard against that and make sure that no future code starts to add the `stack` property, without properly redacting it within `rewriteError` internals. That redaction is _slightly_ less than performant (string manipulation), so it didn't seem worth the effort, since we don't actually send it anywhere. * Rename new `filterErrors` function to a more accurate `rewriteError`. While the new `filterErrors` functionality did allow complete filtering of errors prior to reporting them to Apollo Engine, it also provides the ability to change the error entirely. This more thorough functionality deserves a name which more accurately defines its functionality. The use-defined `rewriteError` function (defined within the `engine` configuration on the `ApolloServer` constructor options) can still be used to "filter" an error from going to Engine by re-writing the error to `null` To accomplish this nullification of an error, an explicit `null` can be returned from the `rewriteError` function rather than the normal practice of returning a `GraphQLError` (in a presumably modified form). * Adjust the line lengths of some comments. Because prettier doesn't do it! * Add CHANGELOG.md for `rewriteError`. * Update deprecated to use rewriteError * Update rewriteError in docs - Deprecate `formatError` in docs in favor of `rewriteError` - lint-fix * Remove now-unnecessary guard which checks for `errors`. This is no longer necessary after the changes in @jbaxleyiii's #2574 (and @martijnwalraven's #2416), which changed the error reporting to be captured via the newly introduced `didEncounterErrors`, rather than in `willResolveField` where it used to happen. Yay! * Remove long-skipped test that was never close to working. I don't think this is providing much value at this point and it's been skipped for over a year. It appears that the test was intended to test `reportErrorFunction`, though the description claims that it's trying to test something else entirely. Regardless, the testing of traces is actually handled elsewhere now, so this is good to remove. * Fix test structure after merge. * Partially Revert "Update rewriteError in docs" This partially reverts commit 21b8b86, which inadvertently changed `formatError`, which is a top-level function which can be used to adjust the response to the client, which is intrinsically different than the new `rewriteError` which modifies the shape of the error in the tracing data which is sent to the Apollo Platform. * Update the API section with more clarity. * Update docs for rewriteError. (#2585) * (docs) Add information on `rewriteError` to the `errors.md`. This adds additional clarity fro the new functionality provided in #1639. * Fix syntax error and use `String.prototype.startsWith` rather `St.pr.match`. Regular expressions are just not as approachable! * Update errors example. The previous example here wasn't a wonderful example since it was suggesting a pattern which is unnecessary when `NODE_ENV=production`, where stack traces are automatically hidden. * Update errors.md * Update errors.md * Update CHANGELOG.md Co-authored-by: GerA <[email protected]>
apollographql · Apr 30, 2019 · 6f2b693 · 6f2b693
1 parent a97ff7e
commit 6f2b693
Show file tree

Hide file tree

Showing 9 changed files with 627 additions and 206 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - `apollo-server-express`: Fix Playground URL when Apollo Server is mounted inside of another Express app by utilizing `req.originalUrl`. [PR #2451](https://github.com/apollographql/apollo-server/pull/2451)
 - New plugin package `apollo-server-plugin-response-cache` implementing a full query response cache based on `apollo-cache-control` hints. The implementation added a few hooks and context fields; see the PR for details. There is a slight change to `cacheControl` object: previously, `cacheControl.stripFormattedExtensions` defaulted to false if you did not provide a `cacheControl` option object, but defaulted to true if you provided (eg) `cacheControl: {defaultMaxAge: 10}`. Now `stripFormattedExtensions` defaults to false unless explicitly provided as `true`, or if you use the legacy boolean `cacheControl: true`. [PR #2437](https://github.com/apollographql/apollo-server/pull/2437)
 - Allow `GraphQLRequestListener` callbacks in plugins to depend on `this`. [PR #2470](https://github.com/apollographql/apollo-server/pull/2470)
+- Add `rewriteError` option to `EngineReportingOptions` (i.e. the `engine` property of the `ApolloServer` constructor).  When defined as a `function`, it will receive an `err` property as its first argument which can be used to manipulate (e.g. redaction) an error prior to sending it to Apollo Engine by modifying, e.g., its `message` property.  The error can also be suppressed from reporting entirely by returning an explicit `null` value.  For more information, [read the documentation](https://www.apollographql.com/docs/apollo-server/features/errors#for-apollo-engine-reporting) and the [`EngineReportingOptions` API reference](https://www.apollographql.com/docs/apollo-server/api/apollo-server#enginereportingoptions). [PR #1639](https://github.com/apollographql/apollo-server/pull/1639)
 - `apollo-server-testing`: Add `variables` and `operationName` to `Query` and `Mutation` types. [PR #2307](https://github.com/apollographql/apollo-server/pull/2307) [Issue #2172](https://github.com/apollographql/apollo-server/issue/2172)
 - `apollo-datasource-rest`: Correctly allow a TTL value of `0` to represent "not-cacheable". [PR #2588](https://github.com/apollographql/apollo-server/pull/2588)
 - `apollo-datasource-rest`: Fix `Invalid argument` in IE11, when `this.headers` is `undefined`. [PR #2607](https://github.com/apollographql/apollo-server/pull/2607)

diff --git a/docs/source/api/apollo-server.md b/docs/source/api/apollo-server.md
@@ -364,13 +364,19 @@ addMockFunctionsToSchema({
    'sendReport()' on other signals if you'd like. Note that 'sendReport()'
    does not run synchronously so it cannot work usefully in an 'exit' handler.
 
-*  `maskErrorDetails`: boolean
-
-   Set to true to remove error details from the traces sent to Apollo's servers. Defaults to false.
+*  `rewriteError`: (err: GraphQLError) => GraphQLError | null
+
+   By default, all errors are reported to Apollo Engine.  This function
+   can be used to exclude specific errors from being reported.  This function
+   receives a copy of the `GraphQLError` and can manipulate it for the
+   purposes of Apollo Engine reporting.  The modified error (e.g. after changing
+   the `err.message` property) should be returned or the function should return
+   an explicit `null` to avoid reporting the error entirely.  It is not
+   permissable to return `undefined`.
 
 *  `schemaTag`: String
 
-   A human readable name to tag this variant of a schema (i.e. staging, EU). Setting this value will cause metrics to be segmented in the Apollo Platform's UI. Additionally schema validation with a schema tag will only check metrics associate with the same string.
+   A human-readable name to tag this variant of a schema (i.e. staging, EU). Setting this value will cause metrics to be segmented in the Apollo Platform's UI. Additionally schema validation with a schema tag will only check metrics associate with the same string.
 
 *  `generateClientInfo`: (GraphQLRequestContext) => ClientInfo **AS 2.2**
 
@@ -392,4 +398,3 @@ addMockFunctionsToSchema({
    > [WARNING] If you specify a `clientReferenceId`, Engine will treat the
    > `clientName` as a secondary lookup, so changing a `clientName` may result
    > in an unwanted experience.
-
diff --git a/docs/source/features/errors.md b/docs/source/features/errors.md
@@ -14,9 +14,9 @@ When an error occurs in Apollo server both inside and outside of resolvers, each
 The first step to improving the usability of a server is providing the error stack trace by default. The following example demonstrates the response returned from Apollo server with a resolver that throws a node [`SystemError`](https://nodejs.org/api/errors.html#errors_system_errors).
 
 ```js line=14-16
-const { 
-  ApolloServer, 
-  gql, 
+const {
+  ApolloServer,
+  gql,
 } = require('apollo-server');
 
 const typeDefs = gql`
@@ -45,10 +45,10 @@ The response will return:
 In addition to stacktraces, Apollo Server's exported errors specify a human-readable string in the `code` field of `extensions` that enables the client to perform corrective actions. In addition to improving the client experience, the `code` field allows the server to categorize errors. For example, an `AuthenticationError` sets the code to `UNAUTHENTICATED`, which enables the client to reauthenticate and would generally be ignored as a server anomaly.
 
 ```js line=4,15-17
-const { 
-  ApolloServer, 
-  gql, 
-  AuthenticationError, 
+const {
+  ApolloServer,
+  gql,
+  AuthenticationError,
 } = require('apollo-server');
 
 const typeDefs = gql`
@@ -72,13 +72,13 @@ The response will return:
 
 ## Augmenting error details
 
-When clients provide bad input, you may want to return additional information 
-like a localized message for each field or argument that was invalid. The 
+When clients provide bad input, you may want to return additional information
+like a localized message for each field or argument that was invalid. The
 following example demonstrates how you can use `UserInputError` to augment
 your error messages with additional details.
 
 ```js line=15-21
-const { 
+const {
   ApolloServer,
   UserInputError,
   gql,
@@ -114,27 +114,160 @@ application, you can use the base `ApolloError` class.
 
 ```js
 new ApolloError(message, code, additionalProperties);
-``` 
+```
 
 ## Masking and logging errors
 
+### For the client response
+
 The Apollo server constructor accepts a `formatError` function that is run on each error passed back to the client. This can be used to mask errors as well as for logging.
-This example demonstrates masking (or suppressing the stacktrace):
+
+> Note that while this changes the error which is sent to the client, it
+> doesn't change the error which is sent to Apollo Engine.  See the
+> `rewriteError` function in the _For Apollo Engine reporting_ section below
+> if this behavior is desired.
+
+This example demonstrates throwing a different error when the error's message starts with `Database Error: `:
 
 ```js line=4-10
 const server = new ApolloServer({
   typeDefs,
   resolvers,
-  formatError: error => {
-    console.log(error);
-    return new Error('Internal server error');
-    // Or, you can delete the exception information
-    // delete error.extensions.exception;
-    // return error;
+  formatError: (err) => {
+    // Don't give the specific errors to the client.
+    if (err.message.startsWith("Database Error: ")) {
+      return new Error('Internal server error');
+    }
+
+    // Otherwise return the original error.  The error can also
+    // be manipulated in other ways, so long as it's returned.
+    return err;
   },
 });
 
 server.listen().then(({ url }) => {
   console.log(`🚀 Server ready at ${url}`);
 });
 ```
+
+### For Apollo Engine reporting
+
+With the Apollo Platform, it's possible to observe error rates within Apollo
+Engine, rather than simply logging them to the console. While all errors are
+sent to Apollo Engine by default, depending on the severity of the error, it
+may be desirable to not send particular errors which may be caused by a
+user's actions. Alternatively, it may be necessary to modify or redact errors
+before transmitting them.
+
+To account for these needs, a `rewriteError` function can be provided within
+the `engine` settings to Apollo Server. At a high-level, the function will
+receive the error to be reported (i.e. a `GraphQLError` or an `ApolloError`)
+as the first argument. The function should then return a modified form of
+that error (e.g. after changing the `err.message` to remove potentially
+sensitive information), or should return an explicit `null` in order to avoid
+reporting the error entirely.
+
+The following sections give some examples of various use-cases for `rewriteError`.
+
+#### Avoid reporting lower-severity predefined errors.
+
+If an application is utilizing the predefined errors noted above (e.g.
+`AuthenticationError`, `ForbiddenError`, `UserInputError`, etc.), these can
+be used with `rewriteError`.
+
+For example, if the current server is `throw`ing the `AuthenticationError`
+when a mis-typed password is supplied, an implementor can avoid reporting
+this to Apollo Engine by defining `rewriteError` as follows:
+
+```js line=5-15
+const { ApolloServer, AuthenticationError } = require("apollo-server");
+const server = new ApolloServer({
+  typeDefs,  // (Not defined in this example)
+  resolvers, //    "      "      "      "
+  engine: {
+    rewriteError(err) {
+      // Return `null` to avoid reporting `AuthenticationError`s
+      if (err instanceof AuthenticationError) {
+        return null;
+      }
+
+      // All other errors will be reported.
+      return err;
+    }
+  },
+});
+```
+
+This example configuration would ensure that any `AuthenticationError` which
+was thrown within a resolver would only be reported to the client, and never
+sent to Apollo Engine. All other errors would be transmitted to Apollo Engine
+normally.
+
+#### Avoid reporting an error based on other properties.
+
+When generating an error (e.g. `new ApolloError("Failure!")`), the `message`
+is the most common property (i.e. `err.message`, which is `Failure!` in this
+case). However, any number of properties can be attached to the error (e.g.
+adding a `code` property). These properties can be checked when determining
+whether an error should be reported to Apollo Engine using the `rewriteError`
+function as follows:
+
+```js line=5-16
+const { ApolloServer } = require("apollo-server");
+const server = new ApolloServer({
+  typeDefs,  // (Not defined in this example)
+  resolvers, //    "      "      "      "
+  engine: {
+    rewriteError(err) {
+      // Using a more stable, known error property (e.g. `err.code`) would be
+      // more defensive, however checking the `message` might serve most needs!
+      if (err.message && err.message.startsWith("Known error message")) {
+        return null;
+      }
+
+      // All other errors should still be reported!
+      return err;
+    }
+  },
+});
+```
+
+This example configuration would ensure that any error which started with
+`Known error message` was not transmitted to Apollo Engine, but all other
+errors would continue to be sent.
+
+#### Redacting the error message.
+
+If it is necessary to change the error prior to reporting it to Apollo Engine
+– for example, if there is personally identifiable information in the error
+`message` — the `rewriteError` function can also help.
+
+Consider an example where the error contained a piece of information like
+an API key (e.g. `throw new ApolloError("The x-api-key:12345 doesn't have
+sufficient privileges.");`).
+
+While a best practice would suggest not including such information in the
+error message itself, the `rewriteError` function could be used to make sure
+it it's sent to Apollo Engine and potentially revealed outside its intended
+scope:
+
+```js line=5-11
+const { ApolloServer } = require("apollo-server");
+const server = new ApolloServer({
+  typeDefs,  // (Not defined in this example)
+  resolvers, //    "      "      "      "
+  engine: {
+    rewriteError(err) {
+      // Make sure that a specific pattern is removed from all error messages.
+      err.message = err.message.replace(/x-api-key:[A-Z0-9-]+/, "REDACTED");
+      return err;
+    }
+  },
+});
+```
+
+In this case, the example error used above would be reported in Apollo Engine as:
+
+```
+The x-api-key:REDACTED doesn't have sufficient privileges.
+```
diff --git a/packages/apollo-engine-reporting/src/agent.ts b/packages/apollo-engine-reporting/src/agent.ts
@@ -1,6 +1,6 @@
 import os from 'os';
 import { gzip } from 'zlib';
-import { DocumentNode } from 'graphql';
+import { DocumentNode, GraphQLError } from 'graphql';
 import {
   FullTracesReport,
   ReportHeader,
@@ -76,8 +76,14 @@ export interface EngineReportingOptions<TContext> {
   handleSignals?: boolean;
   // Sends the trace report immediately. This options is useful for stateless environments
   sendReportsImmediately?: boolean;
-  // To remove the error message from traces, set this to true. Defaults to false
+  // (DEPRECATED; Use `rewriteError` instead) To remove the error message
+  // from traces, set this to true. Defaults to false.
   maskErrorDetails?: boolean;
+  // By default, all errors get reported to Engine servers. You can specify a
+  // 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.
+  rewriteError?: (err: GraphQLError) => GraphQLError | null;
   // A human readable name to tag this variant of a schema (i.e. staging, EU)
   schemaTag?: string;
   //Creates the client information for operation traces.