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

Add new feature to mutex and semaphore - cancelUnlockWaiters: Cancel pending unlocks. #82

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alveifbklsiu259
Copy link

@alveifbklsiu259 alveifbklsiu259 commented Jun 16, 2024

Hello @DirtyHairy , I added a new method cancelUnlockWaiters to mutex and semaphore. When this method is called, all pending unlock waiters will be cancelled, i.e. calls to waitForUnlock will be rejected with the error E_UNLOCKWAITERS_CANCELED.

A scenario where cancelUnlockWaiters would be useful is when a background reauthorization failed. When the reauthorization failed, there's no need to run other pending requests.

This idea came to me when I was implementing reauthorization with RTK Query, there exists a function to cancel all pending locks, however, no corresponding function exists to cancel pending unlocks, so I thought I might as well just make a pull request for such functionality.

Here's a simplified example of using cancelUnlockWaiters:

const baseQueryWithReauth: BaseQueryFn<
    string | FetchArgs,
    unknown,
    FetchBaseQueryError
> = async (args, baseQueryApi, extraOptions) => {

    try {
        // We shouldn't lock the mutex here, it'll make requests wait for each other.
        await mutex.waitForUnlock();

        // Attempt the initial request
        const result = await baseQuery(args, baseQueryApi, extraOptions);

        if (result.error?.status === 401) { // Reauth is needed.

            // This check prevents blindly putting the reauth request in the queue.
            if (!mutex.isLocked()) {

                // Lock the mutex.
                const release = await mutex.acquire();

                try {
                    const refreshResult = await baseQuery({ url: '/auth/refresh' }, baseQueryApi, extraOptions);

                    if (refreshResult.data) { // Reauth succeeds
                        // Save the new AT to the Redux state and retry the initial request
                        baseQueryApi.dispatch(setAccessToken(refreshResult.data as APIPayload.AccessToken));

                        return (await retryInitialRequest(args, baseQueryApi, extraOptions));
                    } else { // Reauth fails
                        mutex.cancelUnlockWaiters();
                        baseQueryApi.dispatch(authApiSlice.endpoints.logout.initiate(null, { track: false }))
                    };
                } finally {
                    release();
                };
            } else { // There's already a pending reauth request.
                await mutex.waitForUnlock();
                return (await retryInitialRequest(args, baseQueryApi, extraOptions));
            }
        };
        // Success / non-401-Error
        return result;
    } catch (e) {
        if (e === E_UNLOCKWAITERS_CANCELED) {
            // ...
        };
    }
};

If you want to include this example in the readme.md, feel free to do so.

About customizing the error for cancelUnlockWaiters, we could take the approach of passing an object to Mutex or Semaphore for both cancelError and unlockCancelError, but that might cause breaking change, so I choose the approach of passing custom unlockCancelError as another argument to Mutex or Semaphore. This means that if a user wants to keep the default cancelError but customize the unlockCancelError, they have to implicitly import E_CANCELED and pass it to Mutex or Semaphore.

import {E_CANCELED} from 'async-mutex';

const semaphore = new Semaphore(2, E_CANCELED, new Error('fancy custom error'));

Thank you for this great library!

@alveifbklsiu259
Copy link
Author

Hi @DirtyHairy, I'm not sure if you received my mail, but I'd appreciate it if you could review the pull request when you have a chance.

Thanks!

@DirtyHairy
Copy link
Owner

Hi @alveifbklsiu259 !

Yeah, I got your mail, thank you for your PR. However, I am currently rather swamped, but I plan to dedicate some time to the currently open PRs and to a new release later this month.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants