Include stacktrace flag (#6267)

.changeset/real-sheep-end.md

@@ -0,0 +1,6 @@
+---
+'@keystone-next/keystone': minor
+'@keystone-next/types': minor
+---
+
+Added `config.graphql.debug` option, which can be used to control whether debug information such as stack traces are included in the errors returned by the GraphQL API.

docs/pages/docs/apis/config.mdx

@@ -277,6 +277,8 @@ It has a TypeScript type of `GraphQLConfig`.
 
 Options:
 
+- `debug` (default: `process.env.NODE_ENV !== 'production'`): If `true`, stacktraces from both Apollo errors and Keystone errors will be included in the errors returned from the GraphQL API.
+  These can be filtered out with `apolloConfig.formatError` if you need to process them, but do not want them returned over the GraphQL API.
 - `queryLimits` (default: `undefined`): Allows you to limit the total number of results returned from a query to your GraphQL API.
   See also the per-list `graphql.queryLimits` option in the [Schema API](./schema).
 - `apolloConfig` (default: `undefined`): Allows you to pass extra options into the `ApolloServer` constructor.
@@ -287,6 +289,7 @@ Options:
 ```typescript
 export default config({
   graphql: {
+    debug: process.env.NODE_ENV !== 'production',
     queryLimits: { maxTotalResults: 100 },
     apolloConfig: {
       playground: process.env.NODE_ENV !== 'production',

packages/keystone/src/___internal-do-not-use-will-break-in-patch/next-graphql.ts

@@ -15,7 +15,7 @@ export function nextGraphQLAPIRoute(keystoneConfig: KeystoneConfig, prismaClient
     graphQLSchema,
     createContext: keystone.createContext,
     sessionStrategy: initializedKeystoneConfig.session,
-    apolloConfig: initializedKeystoneConfig.graphql?.apolloConfig,
+    graphqlConfig: initializedKeystoneConfig.graphql,
     connectionPromise: keystone.connect(),
   });
 

packages/keystone/src/admin-ui/templates/api.ts

@@ -9,7 +9,7 @@ const apolloServer = createApolloServerMicro({
   graphQLSchema,
   createContext,
   sessionStrategy: initializedKeystoneConfig.session ? initializedKeystoneConfig.session() : undefined,
-  apolloConfig: initializedKeystoneConfig.graphql?.apolloConfig,
+  graphqlConfig: initializedKeystoneConfig.graphql,
   connectionPromise: keystone.connect(),
 });
 

packages/keystone/src/lib/core/graphql-errors.ts

@@ -12,12 +12,8 @@ export const extensionError = (extension: string, things: { error: Error; tag: s
   return new ApolloError(
     `An error occured while running "${extension}".\n${s}`,
     'INTERNAL_SERVER_ERROR',
-    // Make the original stack traces available in non-production modes.
-    // TODO: We need to have a way to make these stack traces available
-    // for logging in production mode.
-    process.env.NODE_ENV !== 'production'
-      ? { errors: things.map(t => ({ stacktrace: t.error.stack, message: t.error.message })) }
-      : undefined
+    // Make the original stack traces available.
+    { debug: things.map(t => ({ stacktrace: t.error.stack, message: t.error.message })) }
   );
 };
 

packages/keystone/src/lib/server/createApolloServer.ts

@@ -1,22 +1,22 @@
 import type { IncomingMessage, ServerResponse } from 'http';
-import { GraphQLSchema } from 'graphql';
+import { GraphQLError, GraphQLSchema } from 'graphql';
 import { ApolloServer as ApolloServerMicro } from 'apollo-server-micro';
 import { ApolloServer as ApolloServerExpress } from 'apollo-server-express';
 import type { Config } from 'apollo-server-express';
-import type { CreateContext, SessionStrategy } from '@keystone-next/types';
+import type { CreateContext, GraphQLConfig, SessionStrategy } from '@keystone-next/types';
 import { createSessionContext } from '../../session';
 
 export const createApolloServerMicro = ({
   graphQLSchema,
   createContext,
   sessionStrategy,
-  apolloConfig,
+  graphqlConfig,
   connectionPromise,
 }: {
   graphQLSchema: GraphQLSchema;
   createContext: CreateContext;
   sessionStrategy?: SessionStrategy<any>;
-  apolloConfig?: Config;
+  graphqlConfig?: GraphQLConfig;
   connectionPromise: Promise<any>;
 }) => {
   const context = async ({ req, res }: { req: IncomingMessage; res: ServerResponse }) => {
@@ -28,20 +28,20 @@ export const createApolloServerMicro = ({
       req,
     });
   };
-  const serverConfig = _createApolloServerConfig({ graphQLSchema, apolloConfig });
+  const serverConfig = _createApolloServerConfig({ graphQLSchema, graphqlConfig });
   return new ApolloServerMicro({ ...serverConfig, context });
 };
 
 export const createApolloServerExpress = ({
   graphQLSchema,
   createContext,
   sessionStrategy,
-  apolloConfig,
+  graphqlConfig,
 }: {
   graphQLSchema: GraphQLSchema;
   createContext: CreateContext;
   sessionStrategy?: SessionStrategy<any>;
-  apolloConfig?: Config;
+  graphqlConfig?: GraphQLConfig;
 }) => {
   const context = async ({ req, res }: { req: IncomingMessage; res: ServerResponse }) =>
     createContext({
@@ -50,18 +50,19 @@ export const createApolloServerExpress = ({
         : undefined,
       req,
     });
-  const serverConfig = _createApolloServerConfig({ graphQLSchema, apolloConfig });
+  const serverConfig = _createApolloServerConfig({ graphQLSchema, graphqlConfig });
   return new ApolloServerExpress({ ...serverConfig, context });
 };
 
 const _createApolloServerConfig = ({
   graphQLSchema,
-  apolloConfig,
+  graphqlConfig,
 }: {
   graphQLSchema: GraphQLSchema;
-  apolloConfig?: Config;
+  graphqlConfig?: GraphQLConfig;
 }) => {
   // Playground config, is /api/graphql available?
+  const apolloConfig = graphqlConfig?.apolloConfig;
   const pp = apolloConfig?.playground;
   let playground: Config['playground'];
   const settings = { 'request.credentials': 'same-origin' };
@@ -86,6 +87,7 @@ const _createApolloServerConfig = ({
   return {
     uploads: false,
     schema: graphQLSchema,
+    debug: graphqlConfig?.debug, // If undefined, use Apollo default of NODE_ENV !== 'production'
     // FIXME: support for apollo studio tracing
     // ...(process.env.ENGINE_API_KEY || process.env.APOLLO_KEY
     //   ? { tracing: true }
@@ -97,10 +99,32 @@ const _createApolloServerConfig = ({
     //       tracing: dev,
     //     }),
     ...apolloConfig,
+    formatError: formatError(graphqlConfig),
     // Carefully inject the playground
     playground,
     // FIXME: Support for file handling configuration
     // maxFileSize: 200 * 1024 * 1024,
     // maxFiles: 5,
   };
 };
+
+const formatError = (graphqlConfig: GraphQLConfig | undefined) => {
+  return (err: GraphQLError) => {
+    let debug = graphqlConfig?.debug;
+    if (debug === undefined) {
+      debug = process.env.NODE_ENV !== 'production';
+    }
+
+    if (!debug && err.extensions) {
+      // Strip out any `debug` extensions
+      delete err.extensions.debug;
+      delete err.extensions.exception;
+    }
+
+    if (graphqlConfig?.apolloConfig?.formatError) {
+      return graphqlConfig.apolloConfig.formatError(err);
+    } else {
+      return err;
+    }
+  };
+};

packages/keystone/src/lib/server/createExpressServer.ts

@@ -1,9 +1,13 @@
-import type { Config } from 'apollo-server-express';
 import cors, { CorsOptions } from 'cors';
 import express from 'express';
 import { GraphQLSchema } from 'graphql';
 import { graphqlUploadExpress } from 'graphql-upload';
-import type { KeystoneConfig, CreateContext, SessionStrategy } from '@keystone-next/types';
+import type {
+  KeystoneConfig,
+  CreateContext,
+  SessionStrategy,
+  GraphQLConfig,
+} from '@keystone-next/types';
 import { createAdminUIServer } from '../../admin-ui/system';
 import { createApolloServerExpress } from './createApolloServer';
 import { addHealthCheck } from './addHealthCheck';
@@ -16,20 +20,20 @@ const addApolloServer = ({
   graphQLSchema,
   createContext,
   sessionStrategy,
-  apolloConfig,
+  graphqlConfig,
 }: {
   server: express.Express;
   config: KeystoneConfig;
   graphQLSchema: GraphQLSchema;
   createContext: CreateContext;
   sessionStrategy?: SessionStrategy<any>;
-  apolloConfig?: Config;
+  graphqlConfig?: GraphQLConfig;
 }) => {
   const apolloServer = createApolloServerExpress({
     graphQLSchema,
     createContext,
     sessionStrategy,
-    apolloConfig,
+    graphqlConfig,
   });
 
   const maxFileSize = config.server?.maxFileSize || DEFAULT_MAX_FILE_SIZE;
@@ -68,7 +72,7 @@ export const createExpressServer = async (
     graphQLSchema,
     createContext,
     sessionStrategy: config.session,
-    apolloConfig: config.graphql?.apolloConfig,
+    graphqlConfig: config.graphql,
   });
 
   if (config.ui?.isDisabled) {

packages/types/src/config/index.ts

@@ -143,6 +143,40 @@ export type GraphQLConfig = {
    *  @see https://www.apollographql.com/docs/apollo-server/api/apollo-server/#constructor
    */
   apolloConfig?: Config;
+  /*
+   * When an error is returned from the GraphQL API, Apollo can include a stacktrace
+   * indicating where the error occurred. When Keystone is processing mutations, it
+   * will sometimes captures more than one error at a time, and then group these into
+   * a single error returned from the GraphQL API. Each of these errors will include
+   * a stacktrace.
+   *
+   * In general both categories of stacktrace are useful for debugging while developing,
+   * but should not be exposed in production, and this is the default behaviour of Keystone.
+   *
+   * You can use the `debug` option to change this behaviour. A use case for this
+   * would be if you need to send the stacktraces to a log, but do not want to return them
+   * from the API. In this case you could set `debug: true` and use
+   * `apolloConfig.formatError` to log the stacktraces and then strip them out before
+   * returning the error.
+   *
+   * ```
+   * graphql: {
+   *   debug: true,
+   *   apolloConfig: {
+   *     formatError: err => {
+   *       console.error(err);
+   *       delete err.extensions?.errors;
+   *       delete err.extensions?.exception?.errors;
+   *       delete err.extensions?.exception?.stacktrace;
+   *       return err;
+   *     },
+   *   },
+   * }
+   * ```
+   *   *
+   * Default: process.env.NODE_ENV !== 'production'
+   */
+  debug?: boolean;
 };
 
 // config.extendGraphqlSchema

tests/api-tests/access-control/authed.test.ts

@@ -22,7 +22,7 @@ const expectNoAccess = <N extends string>(
   errors: readonly GraphQLError[] | undefined,
   name: N
 ) => {
-  expectAccessDenied(errors, [{ path: [name] }]);
+  expectAccessDenied('dev', false, undefined, errors, [{ path: [name] }]);
   expect(data?.[name]).toBe(null);
 };
 
@@ -158,7 +158,9 @@ describe('Authed', () => {
       expect(data).toEqual({
         authenticatedItem: { id: user.id, yesRead: user.yesRead, noRead: null },
       });
-      expectAccessDenied(errors, [{ path: ['authenticatedItem', 'noRead'] }]);
+      expectAccessDenied('dev', false, undefined, errors, [
+        { path: ['authenticatedItem', 'noRead'] },
+      ]);
     });
 
     (['imperative', 'declarative'] as const).forEach(mode => {
@@ -442,7 +444,7 @@ describe('Authed', () => {
               if (mode === 'imperative') {
                 expectNamedArray(data, errors, multiDeleteMutationName, [validId1, validId2]);
               } else {
-                expectAccessDenied(errors, [
+                expectAccessDenied('dev', false, undefined, errors, [
                   { path: [multiDeleteMutationName, 0] },
                   { path: [multiDeleteMutationName, 1] },
                 ]);
@@ -459,7 +461,9 @@ describe('Authed', () => {
               if (mode === 'imperative') {
                 expectNamedArray(data, errors, multiDeleteMutationName, [validId1, invalidId]);
               } else {
-                expectAccessDenied(errors, [{ path: [multiDeleteMutationName, 1] }]);
+                expectAccessDenied('dev', false, undefined, errors, [
+                  { path: [multiDeleteMutationName, 1] },
+                ]);
                 expect(data).toEqual({ [multiDeleteMutationName]: [{ id: validId1 }, null] });
               }
             });
@@ -468,7 +472,7 @@ describe('Authed', () => {
               const multiDeleteMutationName = `delete${nameFn[mode](access)}s`;
               const query = `mutation { ${multiDeleteMutationName}(where: [{ id: "${FAKE_ID[provider]}" }, { id: "${FAKE_ID_2[provider]}" }]) { id } }`;
               const { data, errors } = await context.graphql.raw({ query });
-              expectAccessDenied(errors, [
+              expectAccessDenied('dev', false, undefined, errors, [
                 { path: [multiDeleteMutationName, 0] },
                 { path: [multiDeleteMutationName, 1] },
               ]);

tests/api-tests/access-control/mutations-field-static.test.ts

@@ -51,7 +51,7 @@ describe('Access control - Imperative => static', () => {
 
       // Returns null and throws an error
       expect(data).toEqual({ createUser: null });
-      expectAccessDenied(errors, [{ path: ['createUser'] }]);
+      expectAccessDenied('dev', false, undefined, errors, [{ path: ['createUser'] }]);
 
       // Only the original user should exist
       const _users = await context.lists.User.findMany({ query: 'id name other' });
@@ -79,7 +79,7 @@ describe('Access control - Imperative => static', () => {
 
       // Returns null and throws an error
       expect(data).toEqual({ updateUser: null });
-      expectAccessDenied(errors, [{ path: ['updateUser'] }]);
+      expectAccessDenied('dev', false, undefined, errors, [{ path: ['updateUser'] }]);
 
       // User should have its original name
       const _users = await context.lists.User.findMany({ query: 'id name other' });
@@ -118,7 +118,10 @@ describe('Access control - Imperative => static', () => {
       });
 
       // The invalid updates should have errors which point to the nulls in their path
-      expectAccessDenied(errors, [{ path: ['createUsers', 1] }, { path: ['createUsers', 3] }]);
+      expectAccessDenied('dev', false, undefined, errors, [
+        { path: ['createUsers', 1] },
+        { path: ['createUsers', 3] },
+      ]);
 
       // Valid users should exist in the database
       const users = await context.lists.User.findMany({
@@ -172,7 +175,10 @@ describe('Access control - Imperative => static', () => {
       });
 
       // The invalid updates should have errors which point to the nulls in their path
-      expectAccessDenied(errors, [{ path: ['updateUsers', 1] }, { path: ['updateUsers', 3] }]);
+      expectAccessDenied('dev', false, undefined, errors, [
+        { path: ['updateUsers', 1] },
+        { path: ['updateUsers', 3] },
+      ]);
 
       // All users should still exist in the database
       const _users = await context.lists.User.findMany({

tests/api-tests/access-control/mutations-list-declarative.test.ts

@@ -40,7 +40,7 @@ describe('Access control - Imperative => declarative', () => {
 
       // Returns null and throws an error
       expect(data).toEqual({ updateUser: null });
-      expectAccessDenied(errors, [{ path: ['updateUser'] }]);
+      expectAccessDenied('dev', false, undefined, errors, [{ path: ['updateUser'] }]);
 
       // User should have its original name
       const _users = await context.lists.User.findMany({ query: 'id name' });
@@ -65,7 +65,7 @@ describe('Access control - Imperative => declarative', () => {
 
       // Returns null and throws an error
       expect(data).toEqual({ deleteUser: null });
-      expectAccessDenied(errors, [{ path: ['deleteUser'] }]);
+      expectAccessDenied('dev', false, undefined, errors, [{ path: ['deleteUser'] }]);
 
       // Bad users should still be in the database.
       const _users = await context.lists.User.findMany({ query: 'id name' });
@@ -120,7 +120,10 @@ describe('Access control - Imperative => declarative', () => {
           null,
         ],
       });
-      expectAccessDenied(errors, [{ path: ['updateUsers', 1] }, { path: ['updateUsers', 3] }]);
+      expectAccessDenied('dev', false, undefined, errors, [
+        { path: ['updateUsers', 1] },
+        { path: ['updateUsers', 3] },
+      ]);
 
       // All users should still exist in the database
       const _users = await context.lists.User.findMany({
@@ -161,7 +164,10 @@ describe('Access control - Imperative => declarative', () => {
         },
       });
 
-      expectAccessDenied(errors, [{ path: ['deleteUsers', 1] }, { path: ['deleteUsers', 3] }]);
+      expectAccessDenied('dev', false, undefined, errors, [
+        { path: ['deleteUsers', 1] },
+        { path: ['deleteUsers', 3] },
+      ]);
 
       // Valid users are returned, invalid come back as null
       // The invalid deletes should have errors which point to the nulls in their path