Merge pull request #1024 from apollographql/evans/context-error

Enables context creation to be async and capture errors with opt-in logging
apollographql · May 8, 2018 · 33afa32 · 33afa32
2 parents 4dad99c + fd79cf7
commit 33afa32
Show file tree

Hide file tree

Showing 14 changed files with 219 additions and 114 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -36,10 +36,6 @@ jobs:
   # Platform tests, each with the same tests but different platform or version.
   # The docker tag represents the Node.js version and the full list is available
   # at https://hub.docker.com/r/circleci/node/.
-  Node.js 4:
-    docker: [ { image: 'circleci/node:4' } ]
-    <<: *common_test_steps
-
   Node.js 6:
     docker: [ { image: 'circleci/node:6' } ]
     <<: *common_test_steps
@@ -73,7 +69,6 @@ workflows:
   version: 2
   Build and Test:
     jobs:
-      - Node.js 4
       - Node.js 6
       - Node.js 8
       - Node.js 9

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,8 @@ All of the packages in the `apollo-server` repo are released with the same versi
 
 ### vNEXT
 
+* Remove tests and guaranteed support for Node 4 [PR #1024](https://github.com/apollographql/apollo-server/pull/1024)
+
 ### v1.3.6
 
 * Recognize requests with Apollo Persisted Queries and return `PersistedQueryNotSupported` to the client instead of a confusing error. [PR #982](https://github.com/apollographql/apollo-server/pull/982)

diff --git a/packages/apollo-server-core/CHANGELOG.md b/packages/apollo-server-core/CHANGELOG.md
@@ -2,5 +2,7 @@
 
 ### vNEXT
 
+* `apollo-server-core`: Replace console.error with logFunction for opt-in logging [PR #1024](https://github.com/apollographql/apollo-server/pull/1024)
+* `apollo-server-core`: context creation can be async and errors are formatted to include error code [PR #1024](https://github.com/apollographql/apollo-server/pull/1024)
 * `apollo-server-core`: add `mocks` parameter to the base constructor(applies to all variants) [PR#1017](https://github.com/apollographql/apollo-server/pull/1017)
 * `apollo-server-core`: Remove printing of stack traces with `debug` option and include response in logging function[PR#1018](https://github.com/apollographql/apollo-server/pull/1018)
diff --git a/packages/apollo-server-core/src/ApolloServer.ts b/packages/apollo-server-core/src/ApolloServer.ts
@@ -17,9 +17,9 @@ import {
   ExecutionParams,
 } from 'subscriptions-transport-ws';
 
-import { internalFormatError } from './errors';
+import { formatApolloErrors } from './errors';
 import { GraphQLServerOptions as GraphQLOptions } from './graphqlOptions';
-import { LogFunction } from './runQuery';
+import { LogFunction, LogAction, LogStep } from './logging';
 
 import {
   Config,
@@ -213,7 +213,12 @@ export class ApolloServerBase<Request = RequestInit> {
           connection.formatResponse = (value: ExecutionResult) => ({
             ...value,
             errors:
-              value.errors && value.errors.map(err => internalFormatError(err)),
+              value.errors &&
+              formatApolloErrors(value.errors, {
+                formatter: this.requestOptions.formatError,
+                debug: this.requestOptions.debug,
+                logFunction: this.requestOptions.logFunction,
+              }),
           });
           let context: Context = this.context ? this.context : { connection };
 
@@ -223,8 +228,11 @@ export class ApolloServerBase<Request = RequestInit> {
                 ? await this.context({ connection })
                 : context;
           } catch (e) {
-            console.error(e);
-            throw e;
+            throw formatApolloErrors([e], {
+              formatter: this.requestOptions.formatError,
+              debug: this.requestOptions.debug,
+              logFunction: this.requestOptions.logFunction,
+            })[0];
           }
 
           return { ...connection, context };
@@ -267,20 +275,19 @@ export class ApolloServerBase<Request = RequestInit> {
     if (this.engine || engineInRequestPath) this.engineEnabled = true;
   }
 
-  async request(request: Request) {
+  request(request: Request) {
     let context: Context = this.context ? this.context : { request };
 
+    //Defer context resolution to inside of runQuery
     context =
       typeof this.context === 'function'
-        ? await this.context({ req: request })
+        ? () => this.context({ req: request })
         : context;
 
     return {
       schema: this.schema,
       tracing: Boolean(this.engineEnabled),
       cacheControl: Boolean(this.engineEnabled),
-      formatError: (e: GraphQLError) =>
-        internalFormatError(e, this.requestOptions.debug),
       context,
       // allow overrides from options
       ...this.requestOptions,

diff --git a/packages/apollo-server-core/src/errors.ts b/packages/apollo-server-core/src/errors.ts
@@ -1,4 +1,5 @@
 import { GraphQLError } from 'graphql';
+import { LogStep, LogAction, LogMessage, LogFunction } from './logging';
 
 export class ApolloError extends Error {
   public extensions;
@@ -8,14 +9,20 @@ export class ApolloError extends Error {
     properties?: Record<string, any>,
   ) {
     super(message);
-    this.extensions = { ...properties, code };
+
+    if (properties) {
+      Object.keys(properties).forEach(key => {
+        this[key] = properties[key];
+      });
+    }
+
+    //extensions are flattened to be included in the root of GraphQLError's, so
+    //don't add properties to extensions
+    this.extensions = { code };
   }
 }
 
-export function internalFormatError(
-  error: GraphQLError,
-  debug: boolean = false,
-) {
+function enrichError(error: GraphQLError, debug: boolean = false) {
   const expanded: GraphQLError = {
     message: error.message,
     path: error.path,
@@ -95,7 +102,7 @@ export function fromGraphQLError(error: GraphQLError, options?: ErrorOptions) {
   //copy the original error, while keeping all values non-enumerable, so they
   //are not printed unless directly referenced
   Object.defineProperty(copy, 'originalError', { value: {} });
-  Reflect.ownKeys(error).forEach(key => {
+  Object.getOwnPropertyNames(error).forEach(key => {
     Object.defineProperty(copy.originalError, key, { value: error[key] });
   });
 
@@ -133,3 +140,43 @@ export class ForbiddenError extends ApolloError {
     super(message, 'FORBIDDEN');
   }
 }
+
+export function formatApolloErrors(
+  errors: Array<Error>,
+  options?: {
+    formatter?: Function;
+    logFunction?: LogFunction;
+    debug?: boolean;
+  },
+): Array<Error> {
+  const { formatter, debug, logFunction } = options;
+
+  const enrichedErrors = errors.map(error => enrichError(error, debug));
+
+  if (!formatter) {
+    return enrichedErrors;
+  }
+
+  return enrichedErrors.map(error => {
+    try {
+      return formatter(error);
+    } catch (err) {
+      logFunction({
+        action: LogAction.cleanup,
+        step: LogStep.status,
+        data: err,
+        key: 'error',
+      });
+
+      if (debug) {
+        return enrichError(err, debug);
+      } else {
+        //obscure error
+        const newError: GraphQLError = fromGraphQLError(
+          new GraphQLError('Internal server error'),
+        );
+        return enrichError(newError, debug);
+      }
+    }
+  }) as Array<Error>;
+}
diff --git a/packages/apollo-server-core/src/graphqlOptions.ts b/packages/apollo-server-core/src/graphqlOptions.ts
@@ -3,7 +3,7 @@ import {
   ValidationContext,
   GraphQLFieldResolver,
 } from 'graphql';
-import { LogFunction } from './runQuery';
+import { LogFunction } from './logging';
 import { GraphQLExtension } from 'graphql-extensions';
 
 /*
@@ -21,7 +21,11 @@ import { GraphQLExtension } from 'graphql-extensions';
  * - (optional) debug: a boolean that will print additional debug logging if execution errors occur
  *
  */
-export interface GraphQLServerOptions<TContext = any> {
+export interface GraphQLServerOptions<
+  TContext =
+    | (() => Promise<Record<string, any>> | Record<string, any>)
+    | Record<string, any>
+> {
   schema: GraphQLSchema;
   formatError?: Function;
   rootValue?: any;
@@ -40,15 +44,15 @@ export interface GraphQLServerOptions<TContext = any> {
 export default GraphQLServerOptions;
 
 export async function resolveGraphqlOptions(
-  options: GraphQLServerOptions | Function,
-  ...args
+  options:
+    | GraphQLServerOptions
+    | ((
+        ...args: Array<any>
+      ) => Promise<GraphQLServerOptions> | GraphQLServerOptions),
+  ...args: Array<any>
 ): Promise<GraphQLServerOptions> {
   if (typeof options === 'function') {
-    try {
-      return await options(...args);
-    } catch (e) {
-      throw new Error(`Invalid options provided to ApolloServer: ${e.message}`);
-    }
+    return await options(...args);
   } else {
     return options;
   }

diff --git a/packages/apollo-server-core/src/index.ts b/packages/apollo-server-core/src/index.ts
@@ -1,10 +1,5 @@
-export {
-  runQuery,
-  LogFunction,
-  LogMessage,
-  LogStep,
-  LogAction,
-} from './runQuery';
+export { runQuery } from './runQuery';
+export { LogFunction, LogMessage, LogStep, LogAction } from './logging';
 export { runHttpQuery, HttpQueryRequest, HttpQueryError } from './runHttpQuery';
 export {
   default as GraphQLOptions,
@@ -17,7 +12,7 @@ export {
   ValidationError,
   AuthenticationError,
   ForbiddenError,
-  internalFormatError,
+  formatApolloErrors,
 } from './errors';
 
 // ApolloServer Base class

diff --git a/packages/apollo-server-core/src/logging.ts b/packages/apollo-server-core/src/logging.ts
@@ -0,0 +1,25 @@
+export enum LogAction {
+  request,
+  parse,
+  validation,
+  execute,
+  setup,
+  cleanup,
+}
+
+export enum LogStep {
+  start,
+  end,
+  status,
+}
+
+export interface LogMessage {
+  action: LogAction;
+  step: LogStep;
+  key?: string;
+  data?: any;
+}
+
+export interface LogFunction {
+  (message: LogMessage);
+}