Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(auth): Fix willAuthError not being called on waiting operations #3017

Merged
merged 3 commits into from
Mar 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/stupid-cobras-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@urql/exchange-auth': patch
---

Fix `willAuthError` not being called for operations that are waiting on the authentication state to update. This can actually lead to a common issue where operations that came in during the authentication initialization (on startup) will never have `willAuthError` called on them. This can cause an easy mistake where the initial authentication state is never checked to be valid.
50 changes: 50 additions & 0 deletions exchanges/auth/src/authExchange.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,8 @@ it('adds the same token to subsequent operations', async () => {
toPromise
);

await new Promise(resolve => setTimeout(resolve));

next(queryOperation);

next(
Expand Down Expand Up @@ -333,3 +335,51 @@ it('triggers authentication when an operation will error', async () => {
'final-token'
);
});

it('calls willAuthError on queued operations', async () => {
const { exchangeArgs, result, operations } = makeExchangeArgs();
const { source, next } = makeSubject<any>();

let initialAuthResolve: ((_?: any) => void) | undefined;

const willAuthError = vi.fn().mockReturnValue(false);

pipe(
source,
authExchange<{ token: string }>({
async getAuth() {
await new Promise(resolve => {
initialAuthResolve = resolve;
});

return { token: 'token' };
},
willAuthError,
didAuthError: () => false,
addAuthToOperation: ({ authState, operation }) => {
return withAuthHeader(operation, authState?.token);
},
})(exchangeArgs),
publish
);

await Promise.resolve();

next(queryOperation);
expect(result).toHaveBeenCalledTimes(0);
expect(willAuthError).toHaveBeenCalledTimes(0);

expect(initialAuthResolve).toBeDefined();
initialAuthResolve!();

await new Promise(resolve => setTimeout(resolve));

expect(willAuthError).toHaveBeenCalledTimes(1);
expect(result).toHaveBeenCalledTimes(1);

expect(operations.length).toBe(1);
expect(operations[0]).toHaveProperty(
'context.fetchOptions.headers.Authorization',
'token'
);
});
75 changes: 32 additions & 43 deletions exchanges/auth/src/authExchange.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
import {
Source,
pipe,
map,
mergeMap,
fromPromise,
fromValue,
filter,
onStart,
empty,
take,
makeSubject,
toPromise,
merge,
share,
takeUntil,
} from 'wonka';

import {
Expand Down Expand Up @@ -72,7 +68,9 @@ export function authExchange<T>({
willAuthError,
}: AuthConfig<T>): Exchange {
return ({ client, forward }) => {
const bypassQueue: WeakSet<Operation> = new WeakSet();
const retryQueue: Map<number, Operation> = new Map();

const {
source: retrySource$,
next: retryOperation,
Expand All @@ -94,7 +92,10 @@ export function authExchange<T>({

return pipe(
result$,
onStart(() => retryOperation(operation)),
onStart(() => {
bypassQueue.add(operation);
retryOperation(operation);
}),
filter(result => result.operation.key === operation.key),
take(1),
toPromise
Expand Down Expand Up @@ -142,43 +143,31 @@ export function authExchange<T>({
);

const opsWithAuth$ = pipe(
merge([
retrySource$,
pipe(
pendingOps$,
mergeMap(operation => {
if (retryQueue.has(operation.key)) {
return empty;
}

if (
!authPromise &&
willAuthError &&
willAuthError({ operation, authState })
) {
refreshAuth(operation);
return empty;
} else if (!authPromise) {
return fromValue(addAuthAttemptToOperation(operation, false));
}

const teardown$ = pipe(
sharedOps$,
filter(op => {
return op.kind === 'teardown' && op.key === operation.key;
})
);

return pipe(
fromPromise(authPromise),
map(() => addAuthAttemptToOperation(operation, false)),
takeUntil(teardown$)
);
})
),
]),
map(operation => addAuthToOperation({ operation, authState }))
);
merge([retrySource$, pendingOps$]),
map(operation => {
if (bypassQueue.has(operation)) {
return operation;
} else if (authPromise) {
operation = addAuthAttemptToOperation(operation, false);
retryQueue.set(
operation.key,
addAuthAttemptToOperation(operation, false)
);
return null;
} else if (
!operation.context.authAttempt &&
willAuthError &&
willAuthError({ operation, authState })
) {
refreshAuth(operation);
return null;
}

operation = addAuthAttemptToOperation(operation, false);
return addAuthToOperation({ operation, authState });
}),
filter(Boolean)
) as Source<Operation>;

const result$ = pipe(merge([opsWithAuth$, teardownOps$]), forward, share);

Expand Down