bsm / redislock

Simplified distributed locking implementation using Redis
Other
1.45k stars 157 forks source link

Should here return `context.Cause(ctx)` or the error warp with `context.Cause(ctx)` when `ctx.Done()` ? #63

Closed spike014 closed 1 year ago

spike014 commented 1 year ago

https://github.com/bsm/redislock/blob/9348b3363067d988a6814942d0e6f308bcd0533f/redislock.go#L88-L93

Should here return context.Cause(ctx) or the error warp with context.Cause(ctx) when ctx.Done() ?

Because ctx may contain the CancelCause which inside the user's parent context generated by context.WithCancelCause.

I will contribute a PR later. If there is any problem, please let me know.

spike014 commented 1 year ago

context.Cause(ctx) and WithCancelCause is new after Go1.20.

I think the issue should return to :

Should here return ctx.Err() or the error warp with ctx.Err() when ctx.Done() ?

The ctx may with timeout setting. I think here may let user know why he/she didn't get the lock when the ctx's deadline passes or canceled by someone. Then the error can be deal with the way like this:

if err == context.DeadlineExceeded {
    log.Println("context timeout error", err)
    // or do something else
    // Timeout~
} else if err == context.Canceled {
    // Oh! Somebody cancaled this context. That is NOT the error from `bsm/redislock`.
}
dim commented 1 year ago

Will continue in #64, thanks for pointing this out