Repository: urql-graphql/urql
Pull request #3307 (2023-07-10 16:03:21)

feat(auth): Handle refreshAuth rejections gracefully
## Summary

This adds a `catch(errorQueue)` case to `refreshAuth` and replaces `finally(flushQueue)` with `then(flushQueue)`. The rejection case will issue `OperationResult`s for all `Operation`s in the queue and will pass the authentication error that `refreshAuth` rejected with on as the network error.

## Set of changes

- Add error handling to `refreshAuth` rejections



Patch:
diff --git a/.changeset/five-penguins-unite.md b/.changeset/five-penguins-unite.md
new file mode 100644
index 00000000..a0334989
--- /dev/null
+++ b/.changeset/five-penguins-unite.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-persisted': patch
+---
+
+Fix `persistedExchange` ignoring teardowns in its initial operation mapping. Since the hash function is promisified, which defers any persisted operation, it needs to respect teardowns.
diff --git a/.changeset/light-kangaroos-smash.md b/.changeset/light-kangaroos-smash.md
new file mode 100644
index 00000000..f3434c7d
--- /dev/null
+++ b/.changeset/light-kangaroos-smash.md
@@ -0,0 +1,5 @@
+---
+'@urql/exchange-auth': patch
+---
+
+Handle `refreshAuth` rejections and pass the resulting error on to `OperationResult`s on the authentication queue.
diff --git a/exchanges/auth/src/authExchange.ts b/exchanges/auth/src/authExchange.ts
index 13e575c4..ba94f485 100644
--- a/exchanges/auth/src/authExchange.ts
+++ b/exchanges/auth/src/authExchange.ts
@@ -13,6 +13,7 @@ import {
 import {
   createRequest,
   makeOperation,
+  makeErrorResult,
   Operation,
   OperationContext,
   OperationResult,
@@ -202,17 +203,26 @@ export function authExchange(
   return ({ client, forward }) => {
     const bypassQueue = new Set<OperationInstance | undefined>();
     const retries = makeSubject<Operation>();
+    const errors = makeSubject<OperationResult>();
 
     let retryQueue = new Map<number, Operation>();
 
-    function flushQueue(_config?: AuthConfig | undefined) {
-      if (_config) config = _config;
+    function flushQueue() {
       authPromise = undefined;
       const queue = retryQueue;
       retryQueue = new Map();
       queue.forEach(retries.next);
     }
 
+    function errorQueue(error: Error) {
+      authPromise = undefined;
+      const queue = retryQueue;
+      retryQueue = new Map();
+      queue.forEach(operation => {
+        errors.next(makeErrorResult(operation, error));
+      });
+    }
+
     let authPromise: Promise<void> | void;
     let config: AuthConfig | null = null;
 
@@ -270,7 +280,10 @@ export function authExchange(
             },
           })
         )
-        .then(flushQueue);
+        .then((_config: AuthConfig) => {
+          if (_config) config = _config;
+          flushQueue();
+        });
 
       function refreshAuth(operation: Operation) {
         // add to retry queue to try again later
@@ -281,7 +294,7 @@ export function authExchange(
 
         // check that another operation isn't already doing refresh
         if (config && !authPromise) {
-          authPromise = config.refreshAuth().finally(flushQueue);
+          authPromise = config.refreshAuth().then(flushQueue).catch(errorQueue);
         }
       }
 
@@ -341,26 +354,29 @@ export function authExchange(
 
       const result$ = pipe(opsWithAuth$, forward);
 
-      return pipe(
-        result$,
-        filter(result => {
-          if (
-            !bypassQueue.has(result.operation.context._instance) &&
-            result.error &&
-            didAuthError(result) &&
-            !result.operation.context.authAttempt
-          ) {
-            refreshAuth(result.operation);
-            return false;
-          }
+      return merge([
+        errors.source,
+        pipe(
+          result$,
+          filter(result => {
+            if (
+              !bypassQueue.has(result.operation.context._instance) &&
+              result.error &&
+              didAuthError(result) &&
+              !result.operation.context.authAttempt
+            ) {
+              refreshAuth(result.operation);
+              return false;
+            }
 
-          if (bypassQueue.has(result.operation.context._instance)) {
-            bypassQueue.delete(result.operation.context._instance);
-          }
+            if (bypassQueue.has(result.operation.context._instance)) {
+              bypassQueue.delete(result.operation.context._instance);
+            }
 
-          return true;
-        })
-      );
+            return true;
+          })
+        ),
+      ]);
     };
   };
 }
diff --git a/exchanges/persisted/src/persistedExchange.ts b/exchanges/persisted/src/persistedExchange.ts
index 013a81b5..a2628d88 100644
--- a/exchanges/persisted/src/persistedExchange.ts
+++ b/exchanges/persisted/src/persistedExchange.ts
@@ -5,6 +5,7 @@ import {
   filter,
   merge,
   mergeMap,
+  takeUntil,
   pipe,
 } from 'wonka';
 
@@ -135,6 +136,36 @@ export const persistedExchange =
       ((enableForMutation && operation.kind === 'mutation') ||
         operation.kind === 'query');
 
+    const getPersistedOperation = async (operation: Operation) => {
+      const persistedOperation = makeOperation(operation.kind, operation, {
+        ...operation.context,
+        persistAttempt: true,
+      });
+
+      const sha256Hash = await hashFn(
+        stringifyDocument(operation.query),
+        operation.query
+      );
+      if (sha256Hash) {
+        persistedOperation.extensions = {
+          ...persistedOperation.extensions,
+          persistedQuery: {
+            version: 1,
+            sha256Hash,
+          },
+        };
+        if (
+          persistedOperation.kind === 'query' &&
+          preferGetForPersistedQueries
+        ) {
+          persistedOperation.context.preferGetMethod =
+            preferGetForPersistedQueries;
+        }
+      }
+
+      return persistedOperation;
+    };
+
     return operations$ => {
       const retries = makeSubject<Operation>();
 
@@ -146,36 +177,18 @@ export const persistedExchange =
       const persistedOps$ = pipe(
         operations$,
         filter(operationFilter),
-        map(async operation => {
-          const persistedOperation = makeOperation(operation.kind, operation, {
-            ...operation.context,
-            persistAttempt: true,
-          });
-
-          const sha256Hash = await hashFn(
-            stringifyDocument(operation.query),
-            operation.query
+        mergeMap(operation => {
+          const persistedOperation$ = getPersistedOperation(operation);
+          return pipe(
+            fromPromise(persistedOperation$),
+            takeUntil(
+              pipe(
+                operations$,
+                filter(op => op.kind === 'teardown' && op.key === operation.key)
+              )
+            )
           );
-          if (sha256Hash) {
-            persistedOperation.extensions = {
-              ...persistedOperation.extensions,
-              persistedQuery: {
-                version: 1,
-                sha256Hash,
-              },
-            };
-            if (
-              persistedOperation.kind === 'query' &&
-              preferGetForPersistedQueries
-            ) {
-              persistedOperation.context.preferGetMethod =
-                preferGetForPersistedQueries;
-            }
-          }
-
-          return persistedOperation;
-        }),
-        mergeMap(fromPromise)
+        })
       );
 
       return pipe(


Test patch:
diff --git a/exchanges/auth/src/authExchange.test.ts b/exchanges/auth/src/authExchange.test.ts
index b9045a87..e4c8a818 100644
--- a/exchanges/auth/src/authExchange.test.ts
+++ b/exchanges/auth/src/authExchange.test.ts
@@ -442,3 +442,37 @@ it('does not infinitely retry authentication when an operation did error', async
     'final-token'
   );
 });
+
+it('passes on failing refreshAuth() errors to results', async () => {
+  const { exchangeArgs, result } = makeExchangeArgs();
+
+  const didAuthError = vi.fn().mockReturnValue(true);
+  const willAuthError = vi.fn().mockReturnValue(true);
+
+  const res = await pipe(
+    fromValue(queryOperation),
+    authExchange(async utils => {
+      const token = 'initial-token';
+      return {
+        addAuthToOperation(operation) {
+          return utils.appendHeaders(operation, {
+            Authorization: token,
+          });
+        },
+        didAuthError,
+        willAuthError,
+        async refreshAuth() {
+          throw new Error('test');
+        },
+      };
+    })(exchangeArgs),
+    take(1),
+    toPromise
+  );
+
+  expect(result).toHaveBeenCalledTimes(0);
+  expect(didAuthError).toHaveBeenCalledTimes(0);
+  expect(willAuthError).toHaveBeenCalledTimes(1);
+
+  expect(res.error).toMatchInlineSnapshot('[CombinedError: [Network] test]');
+});