Migrate error handling/formatting for Yoga

Author: CarsonF

src/core/exception/exception.filter.ts

@@ -10,7 +10,11 @@ import { HttpAdapter } from '~/core/http';
 import { ConfigService } from '../config/config.service';
 import { ILogger, Logger, LogLevel } from '../logger';
 import { ValidationException } from '../validation';
-import { ExceptionJson, ExceptionNormalizer } from './exception.normalizer';
+import {
+  ExceptionJson,
+  ExceptionNormalizer,
+  NormalizedException,
+} from './exception.normalizer';
 import { isFromHackAttempt } from './is-from-hack-attempt';
 
 @Catch()
@@ -47,16 +51,11 @@ export class ExceptionFilter implements IExceptionFilter {
     this.logIt(normalized, exception);
 
     if (args.getType() === 'graphql') {
-      this.respondToGraphQL(normalized, args);
+      // re-throw our result so that the GraphQL Server picks it up
+      throw new NormalizedException(normalized);
     }
-    await this.respondToHttp(normalized, args);
-  }
 
-  private respondToGraphQL(ex: ExceptionJson, _args: ArgumentsHost) {
-    const { message, stack, ...extensions } = ex;
-    const out = { message, stack, extensions };
-    // re-throw our result so that Apollo Server picks it up
-    throw Object.assign(new Error(), out);
+    await this.respondToHttp(normalized, args);
   }
 
   private async respondToHttp(ex: ExceptionJson, args: ArgumentsHost) {
@@ -96,10 +95,6 @@ export class ExceptionFilter implements IExceptionFilter {
       // Jest will log exceptions, don't duplicate.
       return;
     }
-    if (info.code === 'PersistedQueryNotFound') {
-      // This is the normal flow. Tells the client to send full operation to be cached.
-      return;
-    }
 
     if (info.codes.has('Validation')) {
       const inputErrors =

src/core/exception/exception.normalizer.ts

@@ -1,15 +1,8 @@
-import { ApolloServerErrorCode as ApolloCode } from '@apollo/server/errors';
 import { ArgumentsHost, Inject, Injectable } from '@nestjs/common';
 // eslint-disable-next-line no-restricted-imports,@seedcompany/no-restricted-imports
 import * as Nest from '@nestjs/common';
 import { GqlExecutionContext } from '@nestjs/graphql';
-import {
-  entries,
-  isNotFalsy,
-  setHas,
-  setOf,
-  simpleSwitch,
-} from '@seedcompany/common';
+import { entries, isNotFalsy, simpleSwitch } from '@seedcompany/common';
 import * as Edge from 'edgedb';
 import * as EdgeDBTags from 'edgedb/dist/errors/tags.js';
 import { GraphQLError } from 'graphql';
@@ -35,9 +28,12 @@ import { normalizeFramePath } from './normalize-frame-path';
 
 interface NormalizeParams {
   ex: Error;
-  /** Errors thrown in Query/Mutation/Controller methods will have this. */
+  /** Errors thrown in Query/Mutation/Controller methods will have this via ExceptionFilter. */
   context?: ArgumentsHost;
-  /** Errors thrown in ResolveField methods will have this. */
+  /**
+   * Errors thrown in ResolveField methods (or other GQL communication problems) will have this.
+   * This is essentially mutatally exclusuve with {@link context}.
+   */
   gql?: GraphQLError;
 }
 
@@ -49,6 +45,16 @@ export interface ExceptionJson {
   [key: string]: unknown;
 }
 
+/**
+ * Denote normalization has already happened for this error.
+ * So the GQL server can know acturately if needs to normalize or not.
+ */
+export class NormalizedException extends Error {
+  constructor(readonly normalized: ExceptionJson) {
+    super(normalized.message);
+  }
+}
+
 @Injectable()
 export class ExceptionNormalizer {
   constructor(
@@ -159,14 +165,12 @@ export class ExceptionNormalizer {
       };
     }
 
-    // Apollo Errors
     if (ex instanceof GraphQLError) {
-      const codes = this.errorToCodes(ex);
-      const isClient = setHas(
-        apolloErrorCodesThatAreClientProblem,
-        ex.extensions.code!,
-      );
-      return { codes: [codes[0], 'GraphQL', isClient ? 'Client' : 'Server'] };
+      const isClient =
+        (ex.extensions.http?.status ?? 500) < 500 ||
+        // Guessing here. No execution path - client problem.
+        !ex.path;
+      return { codes: ['GraphQL', isClient ? 'Client' : 'Server'] };
     }
 
     // Bad output from API, that doesn't match the schema
@@ -199,7 +203,7 @@ export class ExceptionNormalizer {
       ? this.resources.getByEdgeDB(type).name
       : type;
 
-    if (gql?.path) {
+    if (gql?.path && gql.path.length > 1) {
       // This error was thrown from a field resolver.
       // Because this is not directly from user input, it is a server error.
       // Still make the error nicer.
@@ -311,12 +315,3 @@ export class ExceptionNormalizer {
     return type.name.replace(/(Exception|Error)$/, '');
   }
 }
-
-const apolloErrorCodesThatAreClientProblem = setOf([
-  ApolloCode.GRAPHQL_PARSE_FAILED,
-  ApolloCode.GRAPHQL_VALIDATION_FAILED,
-  ApolloCode.PERSISTED_QUERY_NOT_FOUND,
-  ApolloCode.PERSISTED_QUERY_NOT_SUPPORTED,
-  ApolloCode.BAD_USER_INPUT,
-  ApolloCode.BAD_REQUEST,
-]);

src/core/graphql/graphql-error-formatter.ts

@@ -1,15 +1,14 @@
-import { unwrapResolverError } from '@apollo/server/errors';
 import { Injectable } from '@nestjs/common';
 import { ModuleRef } from '@nestjs/core';
-import {
-  GraphQLErrorExtensions as ErrorExtensions,
-  GraphQLFormattedError as FormattedError,
-  GraphQLError,
-} from 'graphql';
+import { GraphQLError } from 'graphql';
+import { handleStreamOrSingleExecutionResult } from 'graphql-yoga';
 import { LazyGetter } from 'lazy-get-decorator';
-import { JsonSet } from '~/common';
 import { ExceptionFilter } from '../exception/exception.filter';
-import { ExceptionNormalizer } from '../exception/exception.normalizer';
+import {
+  ExceptionNormalizer,
+  NormalizedException,
+} from '../exception/exception.normalizer';
+import { Plugin } from './plugin.decorator';
 
 declare module 'graphql' {
   interface GraphQLErrorExtensions {
@@ -19,6 +18,7 @@ declare module 'graphql' {
   }
 }
 
+@Plugin()
 @Injectable()
 export class GraphqlErrorFormatter {
   constructor(private readonly moduleRef: ModuleRef) {}
@@ -30,60 +30,61 @@ export class GraphqlErrorFormatter {
     return this.moduleRef.get(ExceptionFilter, { strict: false });
   }
 
-  formatError = (
-    formatted: FormattedError,
-    error: unknown | /* but probably a */ GraphQLError,
-  ): FormattedError => {
-    const { message, ...extensions } = this.getErrorExtensions(
-      formatted,
-      error,
-    );
+  onValidate: Plugin['onValidate'] =
+    () =>
+    ({ result, setResult }) => {
+      if (result.length > 0) {
+        const errors = result.map((error) => this.formatError(error));
+        setResult(errors);
+      }
+    };
 
-    const codes = (extensions.codes ??= new JsonSet(['Server']));
-    delete extensions.code;
+  onExecute: Plugin['onExecute'] = () => ({
+    onExecuteDone: (params) =>
+      handleStreamOrSingleExecutionResult(params, ({ result, setResult }) => {
+        if (result.errors && result.errors.length > 0) {
+          const errors = result.errors.map((error) => this.formatError(error));
+          setResult({ ...result, errors });
+        }
+      }),
+  });
 
-    // Schema & validation errors don't have meaningful stack traces, so remove them
-    const worthlessTrace = codes.has('Validation') || codes.has('GraphQL');
-    if (worthlessTrace) {
-      delete extensions.stacktrace;
+  formatError = (error: unknown) => {
+    if (!(error instanceof GraphQLError)) {
+      // I don't think this happens.
+      return new GraphQLError(
+        error instanceof Error ? error.message : String(error),
+      );
     }
 
-    return {
-      message:
-        message && typeof message === 'string' ? message : formatted.message,
-      extensions,
-      locations: formatted.locations,
-      path: formatted.path,
-    };
-  };
+    const normalized =
+      error.originalError instanceof NormalizedException
+        ? error.originalError.normalized
+        : this.normalizer.normalize({
+            ex: error.originalError ?? error,
+            gql: error,
+          });
 
-  private getErrorExtensions(
-    formatted: FormattedError,
-    error: unknown | /* but probably a */ GraphQLError,
-  ): ErrorExtensions {
-    // ExceptionNormalizer has already been called
-    if (formatted.extensions?.codes instanceof Set) {
-      return { ...formatted.extensions };
+    // If this is an error has not gone through the ExceptionFilter,
+    // the logging was skipped - log error now.
+    if (!(error.originalError instanceof NormalizedException)) {
+      this.filter.logIt(normalized, error.originalError ?? error);
     }
 
-    const original = unwrapResolverError(error);
-    // Safety check
-    if (!(original instanceof Error)) {
-      return { ...formatted.extensions };
+    const { message, stack, code: _, ...extensions } = normalized;
+    const { codes } = extensions;
+
+    // Schema & validation errors don't have meaningful stack traces, so remove them
+    const worthlessTrace = codes.has('Validation') || codes.has('GraphQL');
+    if (!worthlessTrace) {
+      extensions.stacktrace = stack.split('\n');
     }
 
-    // Some errors do not go through the global exception filter.
-    // ResolveField() calls is one of them.
-    // Normalized & log here.
-    const normalized = this.normalizer.normalize({
-      ex: original,
-      gql: error instanceof GraphQLError ? error : undefined,
+    return new GraphQLError(message, {
+      nodes: error.nodes,
+      positions: error.positions,
+      path: error.path,
+      extensions: { ...error.extensions, ...extensions },
     });
-    this.filter.logIt(normalized, original);
-    const { stack, ...extensions } = normalized;
-    return {
-      ...extensions,
-      stacktrace: stack.split('\n'),
-    };
-  }
+  };
 }

src/core/graphql/graphql.options.ts

@@ -15,7 +15,6 @@ import { getRegisteredScalars } from '~/common/scalars';
 import { ConfigService } from '../config/config.service';
 import { VersionService } from '../config/version.service';
 import { isGqlContext } from './gql-context.host';
-import { GraphqlErrorFormatter } from './graphql-error-formatter';
 import { GraphqlTracingPlugin } from './graphql-tracing.plugin';
 
 type ServerContext = YogaDriverServerContext<'fastify'>;
@@ -27,7 +26,6 @@ export class GraphqlOptions implements GqlOptionsFactory {
     private readonly cache: CacheService,
     private readonly tracing: GraphqlTracingPlugin,
     private readonly versionService: VersionService,
-    private readonly errorFormatter: GraphqlErrorFormatter,
   ) {}
 
   async createGqlOptions(): Promise<DriverConfig> {
@@ -53,6 +51,7 @@ export class GraphqlOptions implements GqlOptionsFactory {
         credentials: 'include',
       },
       context: this.context,
+      maskedErrors: false, // Errors are formatted in plugin
       sortSchema: true,
       buildSchemaOptions: {
         // fieldMiddleware: [this.tracing.fieldMiddleware()],