no full messages in hooks

enisdenjo · enisdenjo · commit 68721ae2fd92 · 2025-01-14T21:05:43.000+01:00
diff --git a/.changeset/strange-ties-mix.md b/.changeset/strange-ties-mix.md
@@ -0,0 +1,107 @@
+---
+'graphql-ws': major
+---
+
+`onSubscribe`, `onOperation`, `onError`, `onNext` and `onComplete` hooks don't have the full accompanying message anymore, only the ID and the relevant part from the message
+
+There is really no need to pass the full `SubscribeMessage` to the `onSubscribe` hook. The only relevant parts from the message are the `id` and the `payload`, the `type` is useless since the hook inherently has it (`onNext` is `next` type, `onError` is `error` type, etc).
+
+The actual techincal reason for not having the full message is to avoid serialising results and errors twice. Both `onNext` and `onError` allow the user to augment the result and return it to be used instead. `onNext` originally had the `NextMessage` argument which already has the `FormattedExecutionResult`, and `onError` originally had the `ErrorMessage` argument which already has the `GraphQLFormattedError`, and they both also returned `FormattedExecutionResult` and `GraphQLFormattedError` respectivelly - meaning, if the user serialised the results - the serialisation would happen **twice**.
+
+### Migrating from v5 to v6
+
+#### `onSubscribe`
+
+```diff
+import { ServerOptions, SubscribePayload } from 'graphql-ws';
+
+const opts: ServerOptions = {
+- onSubscribe(ctx, message) {
+-   const messageId = message.id;
+-   const messagePayload: SubscribePayload = message.payload;
+- },
++ onSubscribe(ctx, id, payload) {
++   const messageId = id;
++   const messagePayload: SubscribePayload = payload;
++ },
+};
+```
+
+#### `onOperation`
+
+The `SubscribeMessage.payload` is not useful here at all, the `payload` has been parsed to ready-to-use graphql execution args and should be used instead.
+
+```diff
+import { ExecutionArgs } from 'graphql';
+import { ServerOptions, SubscribePayload } from 'graphql-ws';
+
+const opts: ServerOptions = {
+- onOperation(ctx, message) {
+-   const messageId = message.id;
+-   const messagePayload: SubscribePayload = message.payload;
+- },
++ onOperation(ctx, id, args) {
++   const messageId = id;
++   const executionArgs: ExecutionArgs = args;
++ },
+};
+```
+
+#### `onError`
+
+The `ErrorMessage.payload` (`GraphQLFormattedError[]`) is not useful here at all, the user has access to `GraphQLError[]` that are true instances of the error containing object references to `originalError`s and other properties. The user can always convert and return `GraphQLFormattedError[]` by using the `.toJSON()` method.
+
+```diff
+import { GraphQLError, GraphQLFormattedError } from 'graphql';
+import { ServerOptions } from 'graphql-ws';
+
+const opts: ServerOptions = {
+- onError(ctx, message, errors) {
+-   const messageId = message.id;
+-   const graphqlErrors: readonly GraphQLError[] = errors;
+-   const messagePayload: readonly GraphQLFormattedError[] = message.payload;
+- },
++ onError(ctx, id, errors) {
++   const messageId = id;
++   const graphqlErrors: readonly GraphQLError[] = errors;
++   const messagePayload: readonly GraphQLFormattedError[] = errors.map((e) => e.toJSON());
++ },
+};
+```
+
+#### `onNext`
+
+The `NextMessage.payload` (`FormattedExecutionResult`) is not useful here at all, the user has access to `ExecutionResult` that contains actual object references to error instances. The user can always convert and return `FormattedExecutionResult` by serialising the errors with `GraphQLError.toJSON()` method.
+
+```diff
+import { ExecutionResult, FormattedExecutionResult } from 'graphql';
+import { ServerOptions } from 'graphql-ws';
+
+const opts: ServerOptions = {
+- onNext(ctx, message, result) {
+-   const messageId = message.id;
+-   const graphqlResult: ExecutionResult = result;
+-   const messagePayload: FormattedExecutionResult = message.payload;
+- },
++ onNext(ctx, id, result) {
++   const messageId = id;
++   const graphqlResult: ExecutionResult = result;
++   const messagePayload: FormattedExecutionResult = { ...result, errors: result.errors?.map((e) => e.toJSON()) };
++ },
+};
+```
+
+#### `onComplete`
+
+```diff
+import { ServerOptions } from 'graphql-ws';
+
+const opts: ServerOptions = {
+- onComplete(ctx, message) {
+-   const messageId = message.id;
+- },
++ onComplete(ctx, id) {
++   const messageId = id;
++ },
+};
+```
diff --git a/src/server.ts b/src/server.ts
@@ -20,9 +20,7 @@ import {
 } from 'graphql';
 import {
   CloseCode,
-  CompleteMessage,
   ConnectionInitMessage,
-  ErrorMessage,
   ExecutionPatchResult,
   ExecutionResult,
   FormattedExecutionPatchResult,
@@ -32,19 +30,14 @@ import {
   JSONMessageReviver,
   Message,
   MessageType,
-  NextMessage,
   parseMessage,
   PingMessage,
   PongMessage,
   stringifyMessage,
   SubscribeMessage,
+  SubscribePayload,
 } from './common';
-import {
-  areGraphQLErrors,
-  isAsyncGenerator,
-  isAsyncIterable,
-  isObject,
-} from './utils';
+import { isAsyncGenerator, isAsyncIterable, isObject } from './utils';
 
 /** @category Server */
 export type OperationResult =
@@ -308,7 +301,8 @@ export interface ServerOptions<
     | undefined
     | ((
         ctx: Context<P, E>,
-        message: SubscribeMessage,
+        id: string,
+        payload: SubscribePayload,
       ) =>
         | Promise<ExecutionArgs | readonly GraphQLError[] | void>
         | ExecutionArgs
@@ -338,7 +332,7 @@ export interface ServerOptions<
     | undefined
     | ((
         ctx: Context<P, E>,
-        message: SubscribeMessage,
+        id: string,
         args: ExecutionArgs,
         result: OperationResult,
       ) => Promise<OperationResult | void> | OperationResult | void);
@@ -359,7 +353,7 @@ export interface ServerOptions<
     | undefined
     | ((
         ctx: Context<P, E>,
-        message: ErrorMessage,
+        id: string,
         errors: readonly GraphQLError[],
       ) =>
         | Promise<readonly GraphQLFormattedError[] | void>
@@ -383,7 +377,7 @@ export interface ServerOptions<
     | undefined
     | ((
         ctx: Context<P, E>,
-        message: NextMessage,
+        id: string,
         args: ExecutionArgs,
         result: ExecutionResult | ExecutionPatchResult,
       ) =>
@@ -408,7 +402,7 @@ export interface ServerOptions<
    */
   onComplete?:
     | undefined
-    | ((ctx: Context<P, E>, message: CompleteMessage) => Promise<void> | void);
+    | ((ctx: Context<P, E>, id: string) => Promise<void> | void);
   /**
    * An optional override for the JSON.parse function used to hydrate
    * incoming messages to this server. Useful for parsing custom datatypes
@@ -699,57 +693,46 @@ export function makeServer<
                 args: ExecutionArgs,
               ) => {
                 const { errors, ...resultWithoutErrors } = result;
-                let nextMessage: NextMessage = {
-                  id,
-                  type: MessageType.Next,
-                  payload: {
-                    ...resultWithoutErrors,
-                    ...(errors
-                      ? { errors: errors.map((e) => e.toJSON()) }
-                      : {}),
-                  },
-                };
-                const maybeResult = await onNext?.(
-                  ctx,
-                  nextMessage,
-                  args,
-                  result,
-                );
-                if (maybeResult)
-                  nextMessage = {
-                    ...nextMessage,
-                    payload: maybeResult,
-                  };
+                const maybeResult = await onNext?.(ctx, id, args, result);
                 await socket.send(
-                  stringifyMessage<MessageType.Next>(nextMessage, replacer),
+                  stringifyMessage<MessageType.Next>(
+                    {
+                      id,
+                      type: MessageType.Next,
+                      payload: maybeResult || {
+                        ...resultWithoutErrors,
+                        // omit errors completely if not defined
+                        ...(errors
+                          ? { errors: errors.map((e) => e.toJSON()) }
+                          : {}),
+                      },
+                    },
+                    replacer,
+                  ),
                 );
               },
               error: async (errors: readonly GraphQLError[]) => {
-                let errorMessage: ErrorMessage = {
-                  id,
-                  type: MessageType.Error,
-                  payload: errors.map((e) => e.toJSON()),
-                };
-                const maybeErrors = await onError?.(ctx, errorMessage, errors);
-                if (maybeErrors)
-                  errorMessage = {
-                    ...errorMessage,
-                    payload: maybeErrors,
-                  };
+                const maybeErrors = await onError?.(ctx, id, errors);
                 await socket.send(
-                  stringifyMessage<MessageType.Error>(errorMessage, replacer),
+                  stringifyMessage<MessageType.Error>(
+                    {
+                      id,
+                      type: MessageType.Error,
+                      payload: maybeErrors || errors.map((e) => e.toJSON()),
+                    },
+                    replacer,
+                  ),
                 );
               },
               complete: async (notifyClient: boolean) => {
-                const completeMessage: CompleteMessage = {
-                  id,
-                  type: MessageType.Complete,
-                };
-                await onComplete?.(ctx, completeMessage);
+                await onComplete?.(ctx, id);
                 if (notifyClient)
                   await socket.send(
                     stringifyMessage<MessageType.Complete>(
-                      completeMessage,
+                      {
+                        id,
+                        type: MessageType.Complete,
+                      },
                       replacer,
                     ),
                   );
@@ -758,7 +741,11 @@ export function makeServer<
 
             try {
               let execArgs: ExecutionArgs;
-              const maybeExecArgsOrErrors = await onSubscribe?.(ctx, message);
+              const maybeExecArgsOrErrors = await onSubscribe?.(
+                ctx,
+                message.id,
+                message.payload,
+              );
               if (maybeExecArgsOrErrors) {
                 if (areGraphQLErrors(maybeExecArgsOrErrors))
                   return id in ctx.subscriptions
@@ -833,7 +820,7 @@ export function makeServer<
 
               const maybeResult = await onOperation?.(
                 ctx,
-                message,
+                message.id,
                 execArgs,
                 operationResult,
               );
diff --git a/tests/server.test.ts b/tests/server.test.ts
@@ -1629,13 +1629,13 @@ describe.concurrent('Subscribe', () => {
     };
 
     const { url } = await startTServer({
-      onSubscribe: (_ctx, msg) => {
+      onSubscribe: (_ctx, _id, payload) => {
         // search using `SubscriptionPayload.query` as QueryID
         // check the client example below for better understanding
-        const hit = queriesStore[msg.payload.query as string]!;
+        const hit = queriesStore[payload.query as string]!;
         return {
           ...hit,
-          variableValues: msg.payload.variables, // use the variables from the client
+          variableValues: payload.variables, // use the variables from the client
         };
       },
     });
