From e9a0d6ed035d1a4f509ce39f0558dc17dfb9ccd0 Mon Sep 17 00:00:00 2001 From: Jeroen Claassens Date: Mon, 18 Dec 2023 23:00:13 +0100 Subject: [PATCH] feat: allow `stringifyResult` to return a `Promise` (#7803) * allow `stringifyResult` to return a `Promise` * call `stringifyResult` in previously missed error response cases as well Users who implemented the `stringifyResult` hook can now expect error responses to be formatted with the hook as well. Please take care when updating to this version to ensure this is the desired behavior, or implement the desired behavior accordingly in your `stringifyResult` hook. This was considered a non-breaking change as we consider that it was an oversight in the original PR that introduced `stringifyResult` hook. --- .changeset/thirty-hairs-change.md | 7 ++ packages/server/src/ApolloServer.ts | 18 +-- .../server/src/__tests__/ApolloServer.test.ts | 114 ++++++++++++++++-- .../server/src/externalTypes/constructor.ts | 4 +- packages/server/src/runHttpQuery.ts | 2 +- 5 files changed, 125 insertions(+), 20 deletions(-) create mode 100644 .changeset/thirty-hairs-change.md diff --git a/.changeset/thirty-hairs-change.md b/.changeset/thirty-hairs-change.md new file mode 100644 index 00000000000..a9645f896af --- /dev/null +++ b/.changeset/thirty-hairs-change.md @@ -0,0 +1,7 @@ +--- +"@apollo/server": minor +--- + +allow `stringifyResult` to return a `Promise` + +Users who implemented the `stringifyResult` hook can now expect error responses to be formatted with the hook as well. Please take care when updating to this version to ensure this is the desired behavior, or implement the desired behavior accordingly in your `stringifyResult` hook. This was considered a non-breaking change as we consider that it was an oversight in the original PR that introduced `stringifyResult` hook. diff --git a/packages/server/src/ApolloServer.ts b/packages/server/src/ApolloServer.ts index 60f22ee9685..a78dceee4df 100644 --- a/packages/server/src/ApolloServer.ts +++ b/packages/server/src/ApolloServer.ts @@ -180,7 +180,9 @@ export interface ApolloServerInternals { // flip default behavior. status400ForVariableCoercionErrors?: boolean; __testing_incrementalExecutionResults?: GraphQLExperimentalIncrementalExecutionResults; - stringifyResult: (value: FormattedExecutionResult) => string; + stringifyResult: ( + value: FormattedExecutionResult, + ) => string | Promise; } function defaultLogger(): Logger { @@ -1015,7 +1017,7 @@ export class ApolloServer { // This is typically either the masked error from when background startup // failed, or related to invoking this function before startup or // during/after shutdown (due to lack of draining). - return this.errorResponse(error, httpGraphQLRequest); + return await this.errorResponse(error, httpGraphQLRequest); } if ( @@ -1031,7 +1033,7 @@ export class ApolloServer { } catch (maybeError: unknown) { const error = ensureError(maybeError); this.logger.error(`Landing page \`html\` function threw: ${error}`); - return this.errorResponse(error, httpGraphQLRequest); + return await this.errorResponse(error, httpGraphQLRequest); } } @@ -1076,7 +1078,7 @@ export class ApolloServer { // If some random function threw, add a helpful prefix when converting // to GraphQLError. If it was already a GraphQLError, trust that the // message was chosen thoughtfully and leave off the prefix. - return this.errorResponse( + return await this.errorResponse( ensureGraphQLError(error, 'Context creation failed: '), httpGraphQLRequest, ); @@ -1108,14 +1110,14 @@ export class ApolloServer { ); } } - return this.errorResponse(maybeError, httpGraphQLRequest); + return await this.errorResponse(maybeError, httpGraphQLRequest); } } - private errorResponse( + private async errorResponse( error: unknown, requestHead: HTTPGraphQLHead, - ): HTTPGraphQLResponse { + ): Promise { const { formattedErrors, httpFromErrors } = normalizeAndFormatErrors( [error], { @@ -1144,7 +1146,7 @@ export class ApolloServer { ]), body: { kind: 'complete', - string: prettyJSONStringify({ + string: await this.internals.stringifyResult({ errors: formattedErrors, }), }, diff --git a/packages/server/src/__tests__/ApolloServer.test.ts b/packages/server/src/__tests__/ApolloServer.test.ts index 224948ec966..460a7d5cc76 100644 --- a/packages/server/src/__tests__/ApolloServer.test.ts +++ b/packages/server/src/__tests__/ApolloServer.test.ts @@ -1,22 +1,22 @@ -import { ApolloServer, HeaderMap } from '..'; -import type { ApolloServerOptions } from '..'; +import type { GatewayInterface } from '@apollo/server-gateway-interface'; +import { makeExecutableSchema } from '@graphql-tools/schema'; +import { describe, expect, it, jest } from '@jest/globals'; +import assert from 'assert'; import { FormattedExecutionResult, GraphQLError, GraphQLSchema, - parse, TypedQueryDocumentNode, + parse, } from 'graphql'; +import gql from 'graphql-tag'; +import type { ApolloServerOptions } from '..'; +import { ApolloServer, HeaderMap } from '..'; import type { ApolloServerPlugin, BaseContext } from '../externalTypes'; +import type { GraphQLResponseBody } from '../externalTypes/graphql'; import { ApolloServerPluginCacheControlDisabled } from '../plugin/disabled/index.js'; import { ApolloServerPluginUsageReporting } from '../plugin/usageReporting/index.js'; -import { makeExecutableSchema } from '@graphql-tools/schema'; import { mockLogger } from './mockLogger.js'; -import gql from 'graphql-tag'; -import type { GatewayInterface } from '@apollo/server-gateway-interface'; -import { jest, describe, it, expect } from '@jest/globals'; -import type { GraphQLResponseBody } from '../externalTypes/graphql'; -import assert from 'assert'; const typeDefs = gql` type Query { @@ -176,6 +176,100 @@ describe('ApolloServer construction', () => { `); await server.stop(); }); + + it('async stringifyResult', async () => { + const server = new ApolloServer({ + typeDefs, + resolvers, + stringifyResult: async (value: FormattedExecutionResult) => { + let result = await Promise.resolve( + JSON.stringify(value, null, 10000), + ); + result = result.replace('world', 'stringifyResults works!'); // replace text with something custom + return result; + }, + }); + + await server.start(); + + const request = { + httpGraphQLRequest: { + method: 'POST', + headers: new HeaderMap([['content-type', 'application-json']]), + body: { query: '{ hello }' }, + search: '', + }, + context: async () => ({}), + }; + + const { body } = await server.executeHTTPGraphQLRequest(request); + assert(body.kind === 'complete'); + expect(body.string).toMatchInlineSnapshot(` + "{ + "data": { + "hello": "stringifyResults works!" + } + }" + `); + await server.stop(); + }); + + it('throws the custom parsed error from stringifyResult', async () => { + const server = new ApolloServer({ + typeDefs, + resolvers, + stringifyResult: (_: FormattedExecutionResult) => { + throw new Error('A custom synchronous error'); + }, + }); + + await server.start(); + + const request = { + httpGraphQLRequest: { + method: 'POST', + headers: new HeaderMap([['content-type', 'application-json']]), + body: { query: '{ error }' }, + search: '', + }, + context: async () => ({}), + }; + + await expect( + server.executeHTTPGraphQLRequest(request), + ).rejects.toThrowErrorMatchingInlineSnapshot( + `"A custom synchronous error"`, + ); + + await server.stop(); + }); + + it('throws the custom parsed error from async stringifyResult', async () => { + const server = new ApolloServer({ + typeDefs, + resolvers, + stringifyResult: async (_: FormattedExecutionResult) => + Promise.reject('A custom asynchronous error'), + }); + + await server.start(); + + const request = { + httpGraphQLRequest: { + method: 'POST', + headers: new HeaderMap([['content-type', 'application-json']]), + body: { query: '{ error }' }, + search: '', + }, + context: async () => ({}), + }; + + await expect( + server.executeHTTPGraphQLRequest(request), + ).rejects.toMatchInlineSnapshot(`"A custom asynchronous error"`); + + await server.stop(); + }); }); it('throws when an API key is not a valid header value', () => { @@ -501,7 +595,7 @@ describe('ApolloServer executeOperation', () => { const { body, http } = await server.executeOperation({ query: `#graphql - query NeedsArg($arg: CompoundInput!) { needsCompoundArg(aCompound: $arg) } + query NeedsArg($arg: CompoundInput!) { needsCompoundArg(aCompound: $arg) } `, // @ts-expect-error for `null` case variables, diff --git a/packages/server/src/externalTypes/constructor.ts b/packages/server/src/externalTypes/constructor.ts index 04cf0653d82..839a2daaea9 100644 --- a/packages/server/src/externalTypes/constructor.ts +++ b/packages/server/src/externalTypes/constructor.ts @@ -89,7 +89,9 @@ interface ApolloServerOptionsBase { includeStacktraceInErrorResponses?: boolean; logger?: Logger; allowBatchedHttpRequests?: boolean; - stringifyResult?: (value: FormattedExecutionResult) => string; + stringifyResult?: ( + value: FormattedExecutionResult, + ) => string | Promise; introspection?: boolean; plugins?: ApolloServerPlugin[]; persistedQueries?: PersistedQueryOptions | false; diff --git a/packages/server/src/runHttpQuery.ts b/packages/server/src/runHttpQuery.ts index a22df8aa2b2..06341dd1b30 100644 --- a/packages/server/src/runHttpQuery.ts +++ b/packages/server/src/runHttpQuery.ts @@ -260,7 +260,7 @@ export async function runHttpQuery({ ...graphQLResponse.http, body: { kind: 'complete', - string: internals.stringifyResult( + string: await internals.stringifyResult( orderExecutionResultFields(graphQLResponse.body.singleResult), ), },