fix error display for websocket fetchers (#2535)

* don't join multiple errors in ws-fetcher

* make `formatError` typings more generic

* fix async interable execution

* add e2e test suite for errors

* add changeset

* fix error assertion for older graphql versions

.changeset/thirty-grapes-pay.md

@@ -0,0 +1,5 @@
+---
+'@graphiql/toolkit': patch
+---
+
+Fix error formatting for websocket requests and make error formatting more generic in general

packages/graphiql-react/src/execution.tsx

@@ -302,35 +302,27 @@ export function ExecutionContextProvider(props: ExecutionContextProviderProps) {
             }),
           );
         } else if (isAsyncIterable(value)) {
-          (async () => {
-            try {
-              for await (const result of value) {
-                handleResponse(result);
-              }
-              setIsFetching(false);
-              setSubscription(null);
-            } catch (error) {
-              setIsFetching(false);
-              setResponse(
-                formatError(
-                  error instanceof Error ? error : new Error(`${error}`),
-                ),
-              );
-              setSubscription(null);
-            }
-          })();
-
           setSubscription({
             unsubscribe: () => value[Symbol.asyncIterator]().return?.(),
           });
+
+          try {
+            for await (const result of value) {
+              handleResponse(result);
+            }
+            setIsFetching(false);
+            setSubscription(null);
+          } catch (error) {
+            setIsFetching(false);
+            setResponse(formatError(error));
+            setSubscription(null);
+          }
         } else {
           handleResponse(value);
         }
       } catch (error) {
         setIsFetching(false);
-        setResponse(
-          formatError(error instanceof Error ? error : new Error(`${error}`)),
-        );
+        setResponse(formatError(error));
         setSubscription(null);
       }
     },

packages/graphiql-react/src/schema.tsx

@@ -224,7 +224,7 @@ export function SchemaContextProvider(props: SchemaContextProviderProps) {
           setSchema(newSchema);
           onSchemaChange?.(newSchema);
         } catch (error) {
-          setFetchError(formatError(error as Error));
+          setFetchError(formatError(error));
         }
       })
       .catch(error => {

packages/graphiql-toolkit/src/create-fetcher/lib.ts

@@ -1,4 +1,4 @@
-import { DocumentNode, visit, GraphQLError } from 'graphql';
+import { DocumentNode, visit } from 'graphql';
 import { meros } from 'meros';
 import {
   Client,
@@ -115,20 +115,14 @@ export const createWebsocketsFetcherFromClient = (wsClient: Client) => (
     wsClient!.subscribe(graphQLParams, {
       ...sink,
       error: err => {
-        if (err instanceof Error) {
-          sink.error(err);
-        } else if (err instanceof CloseEvent) {
+        if (err instanceof CloseEvent) {
           sink.error(
             new Error(
               `Socket closed with event ${err.code} ${err.reason || ''}`.trim(),
             ),
           );
         } else {
-          sink.error(
-            new Error(
-              (err as GraphQLError[]).map(({ message }) => message).join(', '),
-            ),
-          );
+          sink.error(err);
         }
       },
     }),

packages/graphiql-toolkit/src/format/index.ts

@@ -1,46 +1,30 @@
-import { GraphQLError, GraphQLFormattedError } from 'graphql';
-
 function stringify(obj: unknown): string {
   return JSON.stringify(obj, null, 2);
 }
 
-const formatSingleError = (error: Error): Error => ({
-  ...error,
-  // Raise these details even if they're non-enumerable
-  message: error.message,
-  stack: error.stack,
-});
-
-type InputError = Error | GraphQLError | string;
+function formatSingleError(error: Error): Error {
+  return {
+    ...error,
+    // Raise these details even if they're non-enumerable
+    message: error.message,
+    stack: error.stack,
+  };
+}
 
-function handleSingleError(
-  error: InputError,
-): GraphQLFormattedError | Error | string {
-  if (error instanceof GraphQLError) {
-    return error.toString();
-  }
+function handleSingleError(error: unknown) {
   if (error instanceof Error) {
     return formatSingleError(error);
   }
   return error;
 }
 
-type GenericError =
-  | Error
-  | readonly Error[]
-  | string
-  | readonly string[]
-  | GraphQLError
-  | readonly GraphQLError[];
-
-export function formatError(error: GenericError): string {
+export function formatError(error: unknown): string {
   if (Array.isArray(error)) {
     return stringify({
-      errors: error.map((e: InputError) => handleSingleError(e)),
+      errors: error.map(e => handleSingleError(e)),
     });
   }
-  // @ts-ignore
-  return stringify({ errors: handleSingleError(error) });
+  return stringify({ errors: [handleSingleError(error)] });
 }
 
 export function formatResult(result: any): string {

packages/graphiql/cypress/integration/errors.spec.ts

@@ -0,0 +1,66 @@
+import { version } from 'graphql';
+
+describe('Errors', () => {
+  it('Should show an error when the HTTP request fails', () => {
+    cy.visit('/?http-error=true');
+    cy.assertQueryResult({
+      errors: [
+        {
+          /**
+           * The exact error message can differ depending on the browser and
+           * its JSON parser. This is the error you get in Electron (which
+           * we use to run the tests headless), the error in the latest Chrome
+           * version is different!
+           */
+          message: 'Unexpected token B in JSON at position 0',
+          stack: 'SyntaxError: Unexpected token B in JSON at position 0',
+        },
+      ],
+    });
+  });
+  it('Should show an error when introspection fails', () => {
+    cy.visit('/?graphql-error=true');
+    cy.assertQueryResult({
+      errors: [{ message: 'Something unexpected happened...' }],
+    });
+  });
+  it('Should show an error when the schema is invalid', () => {
+    cy.visit('/?bad=true');
+    /**
+     * We can't use `cy.assertQueryResult` here because the stack contains line
+     * and column numbers of the `graphiql.min.js` bundle which are not stable.
+     */
+    cy.get('section.result-window').should(element => {
+      expect(element.get(0).innerText).to.contain(
+        version.startsWith('16.')
+          ? 'Names must only contain [_a-zA-Z0-9] but \\"<img src=x onerror=alert(document.domain)>\\" does not.'
+          : 'Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but \\"<img src=x onerror=alert(document.domain)>\\" does not.',
+      );
+    });
+  });
+  it('Should show an error when sending an invalid query', () => {
+    cy.visitWithOp({ query: '{thisDoesNotExist}' });
+    cy.clickExecuteQuery();
+    cy.assertQueryResult({
+      errors: [
+        {
+          message: 'Cannot query field "thisDoesNotExist" on type "Test".',
+          locations: [{ line: 1, column: 2 }],
+        },
+      ],
+    });
+  });
+  it('Should show an error when sending an invalid subscription', () => {
+    cy.visitWithOp({ query: 'subscription {thisDoesNotExist}' });
+    cy.clickExecuteQuery();
+    cy.assertQueryResult({
+      errors: [
+        {
+          message:
+            'Cannot query field "thisDoesNotExist" on type "SubscriptionType".',
+          locations: [{ line: 1, column: 15 }],
+        },
+      ],
+    });
+  });
+});

packages/graphiql/cypress/integration/graphqlWs.spec.ts

@@ -12,11 +12,10 @@ describe('IncrementalDelivery support via fetcher', () => {
     };
 
     it('Expects a subscription to resolve', () => {
-      cy.assertQueryResult(
-        { query: testSubscription, variables: { delay: 0 } },
-        mockSubscriptionSuccess,
-        1200,
-      );
+      cy.visitWithOp({ query: testSubscription, variables: { delay: 0 } });
+      cy.clickExecuteQuery();
+      cy.wait(1200);
+      cy.assertQueryResult(mockSubscriptionSuccess);
     });
   });
 });

packages/graphiql/cypress/integration/incrementalDelivery.spec.ts

@@ -59,20 +59,16 @@ describeOrSkip('IncrementalDelivery support via fetcher', () => {
       const delay = 100;
       const timeout = mockStreamSuccess.data.streamable.length * (delay * 1.5);
 
-      cy.visit(`/?query=${testStreamQuery}`);
-      cy.assertQueryResult(
-        { query: testStreamQuery, variables: { delay } },
-        mockStreamSuccess,
-        timeout,
-      );
+      cy.visitWithOp({ query: testStreamQuery, variables: { delay } });
+      cy.clickExecuteQuery();
+      cy.wait(timeout);
+      cy.assertQueryResult(mockStreamSuccess);
     });
 
     it('Expects a quick stream to resolve in a single increment', () => {
-      cy.visit(`/?query=${testStreamQuery}`);
-      cy.assertQueryResult(
-        { query: testStreamQuery, variables: { delay: 0 } },
-        mockStreamSuccess,
-      );
+      cy.visitWithOp({ query: testStreamQuery, variables: { delay: 0 } });
+      cy.clickExecuteQuery();
+      cy.assertQueryResult(mockStreamSuccess);
     });
   });
 
@@ -92,21 +88,19 @@ describeOrSkip('IncrementalDelivery support via fetcher', () => {
         }
       `;
 
-      cy.visit(`/?query=${testQuery}`);
-      cy.assertQueryResult(
-        { query: testQuery, variables: { delay } },
-        {
-          data: {
-            deferrable: {
-              normalString: 'Nice',
-              deferredString:
-                'Oops, this took 1 seconds longer than I thought it would!',
-            },
+      cy.visitWithOp({ query: testQuery, variables: { delay } });
+      cy.clickExecuteQuery();
+      cy.wait(timeout);
+      cy.assertQueryResult({
+        data: {
+          deferrable: {
+            normalString: 'Nice',
+            deferredString:
+              'Oops, this took 1 seconds longer than I thought it would!',
           },
-          hasNext: false,
         },
-        timeout,
-      );
+        hasNext: false,
+      });
     });
 
     it('Expects to merge types when members arrive at different times', () => {
@@ -142,38 +136,36 @@ describeOrSkip('IncrementalDelivery support via fetcher', () => {
         }
       `;
 
-      cy.visit(`/?query=${testQuery}`);
-      cy.assertQueryResult(
-        { query: testQuery, variables: { delay } },
-        {
-          data: {
-            person: {
-              name: 'Mark',
-              friends: [
-                {
-                  name: 'James',
-                  age: 1000,
-                },
-                {
-                  name: 'Mary',
-                  age: 1000,
-                },
-                {
-                  name: 'John',
-                  age: 1000,
-                },
-                {
-                  name: 'Patrica',
-                  age: 1000,
-                },
-              ],
-              age: 1000,
-            },
+      cy.visitWithOp({ query: testQuery, variables: { delay } });
+      cy.clickExecuteQuery();
+      cy.wait(timeout);
+      cy.assertQueryResult({
+        data: {
+          person: {
+            name: 'Mark',
+            friends: [
+              {
+                name: 'James',
+                age: 1000,
+              },
+              {
+                name: 'Mary',
+                age: 1000,
+              },
+              {
+                name: 'John',
+                age: 1000,
+              },
+              {
+                name: 'Patrica',
+                age: 1000,
+              },
+            ],
+            age: 1000,
           },
-          hasNext: false,
         },
-        timeout,
-      );
+        hasNext: false,
+      });
     });
   });
 });

packages/graphiql/cypress/integration/init.spec.ts

@@ -43,7 +43,9 @@ describe('GraphiQL On Initialization', () => {
   });
 
   it('Executes a GraphQL query over HTTP that has the expected result', () => {
-    cy.assertQueryResult({ query: testQuery }, mockSuccess);
+    cy.visitWithOp({ query: testQuery });
+    cy.clickExecuteQuery();
+    cy.assertQueryResult(mockSuccess);
   });
   it('Shows the expected error when the schema is invalid', () => {
     cy.visit(`/?bad=true`);