Merge pull request #213 from aave/cesare/aave-3193-sdk-fix-issue-with-gql-errors-blocking-transaction-hooks

fix: issue where GQL error blocks React transaction hooks

Author: cesarenaldi

.changeset/hip-views-cough.md

@@ -0,0 +1,7 @@
+---
+"@aave/client": patch
+"@aave/react": patch
+"@aave/core": patch
+---
+
+**fix:** issue with GQL errors suchas Bad User Input or Bad Request blocking indefintely transaction hooks.

packages/client/src/testing.ts

@@ -1,6 +1,6 @@
 /// <reference path="../../../vite-env.d.ts" />
 
-import { GraphQLErrorCode, UnexpectedError } from '@aave/core';
+import { UnexpectedError } from '@aave/core';
 import { encodeHubId, encodeSpokeId } from '@aave/graphql';
 import {
   type BigDecimal,
@@ -236,30 +236,6 @@ export function fundErc20Address(
   );
 }
 
-const messages: Record<GraphQLErrorCode, string> = {
-  [GraphQLErrorCode.UNAUTHENTICATED]:
-    "Unauthenticated - Authentication is required to access '<operation>'",
-  [GraphQLErrorCode.FORBIDDEN]:
-    "Forbidden - You are not authorized to access '<operation>'",
-  [GraphQLErrorCode.INTERNAL_SERVER_ERROR]:
-    'Internal server error - Please try again later',
-  [GraphQLErrorCode.BAD_USER_INPUT]:
-    'Bad user input - Please check the input and try again',
-  [GraphQLErrorCode.BAD_REQUEST]:
-    'Bad request - Please check the request and try again',
-};
-
-export function createGraphQLErrorObject(code: GraphQLErrorCode) {
-  return {
-    message: messages[code],
-    locations: [],
-    path: [],
-    extensions: {
-      code: code,
-    },
-  };
-}
-
 export function wait(ms: number) {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }

packages/core/package.json

@@ -15,6 +15,9 @@
     ".": {
       "import": "./dist/index.js",
       "require": "./dist/index.cjs"
+    },
+    "./testing": {
+      "default": "./dist/testing.js"
     }
   },
   "typesVersions": {
@@ -24,6 +27,9 @@
       ],
       "require": [
         "./dist/index.d.cts"
+      ],
+      "testing": [
+        "./dist/testing.d.ts"
       ]
     }
   },

packages/core/src/GqlClient.test.ts

@@ -1,4 +1,4 @@
-import { assertOk, ResultAsync } from '@aave/types';
+import { assertErr, assertOk, ResultAsync } from '@aave/types';
 import { gql, stringifyDocument } from '@urql/core';
 import * as msw from 'msw';
 import { setupServer } from 'msw/node';
@@ -11,9 +11,10 @@ import {
   expect,
   it,
 } from 'vitest';
-
 import type { Context } from './context';
+import { GraphQLErrorCode } from './errors';
 import { GqlClient } from './GqlClient';
+import { createGraphQLErrorObject } from './testing';
 import { delay } from './utils';
 
 // see: https://mswjs.io/docs/graphql/mocking-responses/query-batching
@@ -153,6 +154,26 @@ describe(`Given an instance of the ${GqlClient.name}`, () => {
     });
   });
 
+  describe('When executing a query that fails with GraphQL errors', () => {
+    beforeAll(() => {
+      server.use(
+        api.query(TestQuery, () => {
+          return msw.HttpResponse.json({
+            errors: [createGraphQLErrorObject(GraphQLErrorCode.BAD_REQUEST)],
+          });
+        }),
+      );
+    });
+
+    it('Then it should fail with an `UnexpectedError`', async () => {
+      const client = new GqlClient(context);
+
+      const result = await client.query(TestQuery, { id: 1 });
+
+      assertErr(result);
+    });
+  });
+
   describe('When batching concurrent queries', () => {
     it('Then it should limit batching to a maximum of 10 queries', async () => {
       const client = new GqlClient(context);

packages/core/src/GqlClient.ts

@@ -21,7 +21,6 @@ import {
   extractOperationName,
   isActiveQueryOperation,
   isTeardownOperation,
-  takeValue,
 } from './utils';
 
 /**
@@ -99,7 +98,7 @@ export class GqlClient {
     const query = this.resolver.replaceFrom(document);
     return this.resultFrom(
       this.urql.query(query, variables, { batch, requestPolicy }),
-    ).map(takeValue);
+    );
   }
 
   /**
@@ -113,9 +112,7 @@ export class GqlClient {
     document: TypedDocumentNode<StandardData<TValue>, TVariables>,
     variables: TVariables,
   ): ResultAsync<TValue, UnexpectedError> {
-    return this.resultFrom(this.urql.mutation(document, variables)).map(
-      takeValue,
-    );
+    return this.resultFrom(this.urql.mutation(document, variables));
   }
 
   protected async refreshWhere(
@@ -199,17 +196,24 @@ export class GqlClient {
         );
   }
 
-  private resultFrom<TData, TVariables extends AnyVariables>(
-    source: OperationResultSource<OperationResult<TData, TVariables>>,
-  ): ResultAsync<OperationResult<TData, TVariables>, UnexpectedError> {
+  private resultFrom<TValue, TVariables extends AnyVariables>(
+    source: OperationResultSource<
+      OperationResult<StandardData<TValue>, TVariables>
+    >,
+  ): ResultAsync<TValue, UnexpectedError> {
     return ResultAsync.fromPromise(source.toPromise(), (err: unknown) => {
       this.logger.error(err);
       return UnexpectedError.from(err);
     }).andThen((result) => {
       if (result.error?.networkError) {
         return errAsync(UnexpectedError.from(result.error.networkError));
       }
-      return okAsync(result);
+
+      if (result.data) {
+        return okAsync(result.data.value);
+      }
+
+      return errAsync(UnexpectedError.from(result.error));
     });
   }
 }

packages/core/src/testing.ts

@@ -0,0 +1,25 @@
+import { GraphQLErrorCode } from './errors';
+
+const messages: Record<GraphQLErrorCode, string> = {
+  [GraphQLErrorCode.UNAUTHENTICATED]:
+    "Unauthenticated - Authentication is required to access '<operation>'",
+  [GraphQLErrorCode.FORBIDDEN]:
+    "Forbidden - You are not authorized to access '<operation>'",
+  [GraphQLErrorCode.INTERNAL_SERVER_ERROR]:
+    'Internal server error - Please try again later',
+  [GraphQLErrorCode.BAD_USER_INPUT]:
+    'Bad user input - Please check the input and try again',
+  [GraphQLErrorCode.BAD_REQUEST]:
+    'Bad request - Please check the request and try again',
+};
+
+export function createGraphQLErrorObject(code: GraphQLErrorCode) {
+  return {
+    message: messages[code],
+    locations: [],
+    path: [],
+    extensions: {
+      code: code,
+    },
+  };
+}

packages/core/src/utils.ts

@@ -1,7 +1,5 @@
-import { invariant } from '@aave/types';
-import type { AnyVariables, Operation, OperationResult } from '@urql/core';
+import type { Operation } from '@urql/core';
 import { Kind, type OperationDefinitionNode } from 'graphql';
-import type { StandardData } from './types';
 
 /**
  * @internal
@@ -10,17 +8,6 @@ export function delay(ms: number): Promise<void> {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }
 
-/**
- * @internal
- */
-export function takeValue<T>({
-  data,
-  error,
-}: OperationResult<StandardData<T> | undefined, AnyVariables>): T {
-  invariant(data, `Expected a value, got: ${error?.message}`);
-  return data.value;
-}
-
 /**
  * @internal
  */

packages/core/tsup.config.ts

@@ -1,17 +1,34 @@
 /* eslint-disable import/no-default-export */
 import { defineConfig } from 'tsup';
 
-export default defineConfig(() => ({
-  entry: ['src/index.ts'],
-  outDir: 'dist',
-  splitting: false,
-  sourcemap: true,
-  treeshake: true,
-  clean: true,
-  tsconfig: 'tsconfig.build.json',
-  bundle: true,
-  minify: true,
-  dts: true,
-  platform: 'neutral',
-  format: ['esm', 'cjs'],
-}));
+export default defineConfig(() => [
+  {
+    entry: ['src/index.ts'],
+    outDir: 'dist',
+    splitting: false,
+    sourcemap: true,
+    treeshake: true,
+    clean: true,
+    tsconfig: 'tsconfig.build.json',
+    bundle: true,
+    minify: true,
+    dts: true,
+    platform: 'neutral',
+    format: ['esm', 'cjs'],
+  },
+  // ESM only
+  {
+    entry: ['src/testing.ts'],
+    outDir: 'dist',
+    splitting: false,
+    sourcemap: true,
+    treeshake: true,
+    clean: true,
+    tsconfig: 'tsconfig.build.json',
+    bundle: true,
+    minify: true,
+    dts: true,
+    platform: 'neutral',
+    format: ['esm'],
+  },
+]);

packages/react/src/helpers/reads.test.ts

@@ -1,4 +1,5 @@
-import { UnexpectedError } from '@aave/client';
+import { GraphQLErrorCode, UnexpectedError } from '@aave/client';
+import { createGraphQLErrorObject } from '@aave/core/testing';
 import { act } from '@testing-library/react';
 import { graphql, HttpResponse } from 'msw';
 import { setupServer } from 'msw/node';
@@ -66,9 +67,7 @@ describe(`Given the '${useSuspendableQuery.name}' hook`, () => {
       server.use(
         graphql.query(AnyQuery, () => {
           return HttpResponse.json({
-            errors: [
-              { message: 'Test error', extensions: { code: 'TEST_ERROR' } },
-            ],
+            errors: [createGraphQLErrorObject(GraphQLErrorCode.BAD_REQUEST)],
           });
         }),
       );

packages/react/src/helpers/tasks.test.ts

@@ -115,8 +115,27 @@ describe(`Given the '${useAsyncTask.name}' hook`, () => {
       });
     });
 
+    describe('When the task throws an error synchronously', () => {
+      it('Then it should revert to the previous state and rethrow the error', async () => {
+        const { result } = renderHook(() =>
+          useAsyncTask((_: string) => {
+            throw new Error('test error');
+          }, []),
+        );
+        const state = result.current[1];
+
+        await act(async () => {
+          await expect(async () => {
+            await result.current[0]('test');
+          }).rejects.toThrow(Error);
+        });
+
+        expect(result.current[1]).toMatchObject(state);
+      });
+    });
+
     describe('When the task fails', () => {
-      it('Then it should return the state in line with type of `AsyncTaskFailed`', async () => {
+      it('Then it should return the state in line with type of `AsyncTaskError`', async () => {
         const { result } = renderHook(() =>
           useAsyncTask((_: string) => errAsync(new Error('test error')), []),
         );
@@ -150,7 +169,7 @@ describe(`Given the '${useAsyncTask.name}' hook`, () => {
     });
   });
 
-  describe('And the hook is executed once with an error', () => {
+  describe('And a previous execution failed', () => {
     describe('When the hook is executed again', () => {
       it('Then it should return the state in line with type of `AsyncTaskLoading`', async () => {
         const { result } = renderHook(() =>
@@ -235,7 +254,7 @@ describe(`Given the '${useAsyncTask.name}' hook`, () => {
     });
 
     describe('When the task fails', () => {
-      it('Then it should return the state in line with type of `AsyncTaskFailed`', async () => {
+      it('Then it should return the state in line with type of `AsyncTaskError`', async () => {
         const { result } = renderHook(() =>
           useAsyncTask((input: string) => {
             if (input === 'one') return okAsync(input);