rogierschouten / async-lock

Lock on asynchronous code for Nodejs
MIT License
394 stars 50 forks source link

Recognise async lock errors #59

Open nebarf opened 1 year ago

nebarf commented 1 year ago

Hi @rogierschouten! Thanks for keeping to work on this library, really appreciate :pray:

Correct me if I'm wrong, currently the only way to distinguish between async-lock errors and any error that's thrown by the wrapped async function is by using the error message:

function someAsyncFn() {}

lock.acquire(key, someAsyncFn, opts)
    .catch(function(err) {
        const isAsyncLockTimeoutError = err instanceof Error && err.message.includes('async-lock timed out')
    if (isAsyncLockTimeoutError) {
            // handle async-lock timeout error
        }
    });

IMHO it would be great if async-lock could deal with its own error classes to model anything can go wrong on the library side.

class AsyncLockError extends Error {
    constructor(message) {
        super(message);
    }
}

class AsyncLockTimeoutError extends AsyncLockError {
    constructor(queueKey) {
        super(`Async lock timeout out in queue ${async-lock timed out in queue}`);
        this.name = 'AsyncLockTimeoutError';
    }
}

This mean on the user-land side it would be much cleaner handling any async-lock error:

function someAsyncFn() {}

lock.acquire(key, someAsyncFn, opts)
    // Handle different kind of async-lock errors
    .catch(function(err) {
        if (err instanceof AsyncLockTimeoutError) {
            // do something...
        }
        if (err instanceof AsyncLockMaxOccTimeError) {
            // do something...
        }
    });
function someAsyncFn() {}

lock.acquire(key, someAsyncFn, opts)
    // Catch any kind of async-lock errors
    .catch(function(err) {
        if (err instanceof AsyncLockError) {
            // async-lock error
        } else {
            // Any error thrown by `someAsyncFn`
        }
    });

What do you think about that? I can find some time to work on this if it can help.

rogierschouten commented 9 months ago

Would be great, I'll merge a PR. Sorry for late reply, this year's been rough