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

More granular error codes #43

Closed
fider opened this issue Sep 24, 2018 · 7 comments
Closed

More granular error codes #43

fider opened this issue Sep 24, 2018 · 7 comments

Comments

@fider
Copy link

fider commented Sep 24, 2018

Hello.
Briefly - how would I detect situation where I cannot get lock because of all Redis instanes gone (eg. db connection error on all instances).

More details:

try {
    lock = await redlock.lock("x", 1000);
} catch (err) {
    // Db connection problem (what I want to log and response with appropriate error code/message)
    //   OR
    // Alredy locked by other instance of my app and I want to ignore it (eg. I will respond with 202 code to signal that task already in progress)
}

Is there a way I can distinguish between type of errors, eg:

  1. Unable to acquire/extend lock because of db connection error.
  2. Unable to acquire lock because resource already locked.
  3. Unable to extend lock because it already expired.

Is it possible that you will introduce LockError.code property and expose (export) some LOCK_ERROR_CODE_... consts?

Regards,
Wojtek

@mike-marcacci
Copy link
Owner

Hi @fider! This is a perfectly reasonable request, but I think we’ll have to refine your suggested API a bit.

Since redlock is highly available it can receive errors (or failures) on a minority of attempts and still acquire the lock. It’s possible that we reach a quorum and return the acquired lock before all attempts have finished, and the subsequent ones could potentially fail.

The other case is that a majority of attempts fail, which can happen from a combination of failures (lock is already acquired) and errors. Again, we can be certain of failure before we actually receive all the responses.

Perhaps both the Lock and LockError should have a field which is Promise that resolves with the results of each attempt, once they are all complete? I realize this might be a bit cumbersome for the single-redis use case.

@mike-marcacci
Copy link
Owner

Another complication is that of retries...

@fider
Copy link
Author

fider commented Oct 1, 2018 via email

@mike-marcacci mike-marcacci changed the title Error codes (question / feture request) More granular error codes Oct 24, 2018
@germansokolov13
Copy link

I am doing something wrong or there is a fundumental problem with Redlock api. It's somewhat hard to properly use it for optimistic locking. I assume that among many parallel requests one should acquire a lock and others should just give up right away because I only want some computation to be done exactly once. Redlock throws an exception whenever it fails to acquire a log. But I actually indend to sometimes fail acquiring the lock. This is not an error to handle. As such maybe it should not have been implemented through an exception. Exceptions are for errors, not for normal flow of algorithm.

@mike-marcacci
Copy link
Owner

Hi @germansokolov13 - are you actually experiencing a thrown exception? In normal usage the API either rejects a Promise or calls the callback with an error as the first argument; if you are experiencing a thrown exception something isn't working correctly.

@germansokolov13
Copy link

@mike-marcacci In JavaScript when you use async/await syntax rejecting a promise translates into catch clause making it behave like an exception. So if you await a lock then it will go to catch. I thought rejecting a promise is semantically the same thing as throwing an exception.

@mike-marcacci mike-marcacci mentioned this issue Dec 12, 2020
9 tasks
@mike-marcacci
Copy link
Owner

The newest version has all the relevant information attached to an error. Accessing all of this information is still a bit complex, because we can sometime know that something has errored before all parallel requests have completed. See the tests for an example of how to inspect these:

t.fail(`${error.message}
---
${(await Promise.all(error.attempts))
.map(
(s, i) =>
`ATTEMPT ${i}: ${formatWithOptions(
{ colors: true },
{
membershipSize: s.membershipSize,
quorumSize: s.quorumSize,
votesForSize: s.votesFor.size,
votesAgainstSize: s.votesAgainst.size,
votesAgainstError: s.votesAgainst.values(),
}
)}`
)
.join("\n\n")}
`);

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

No branches or pull requests

3 participants