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

Return pure redis.ErrNil, so 'nil return' can be detected. #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kakaLQY
Copy link

@kakaLQY kakaLQY commented Jan 22, 2016

redis.ErrNil can be easily detected and a timeout is got, which is different from other errors.

@dvirsky
Copy link
Member

dvirsky commented Jan 22, 2016

why can't you detect nil the usual way? formatting the way I do is the recommended way in go.

@kakaLQY
Copy link
Author

kakaLQY commented Jan 22, 2016

An example:

if jobs, err := client.GetMulti(10, 20*time.Second, queues...); err != nil && err != redis.ErrNil {
    log.Println(err)
}

If jobs is nil, there may be an error. But if the error is redis.ErrNil, I will know this is timeout and not other errors. Then I can start this observation again and it is not considered as a real error. If the error is not this, I must record it.

@dvirsky
Copy link
Member

dvirsky commented Jan 22, 2016

I'd rather handle this case internally and not trust redigo errors, I might change the internal client, I don't want to commit to keeping redigo under the hood.
Also, why is a timeout ErrNil?

@kakaLQY
Copy link
Author

kakaLQY commented Jan 22, 2016

You can jump in this line.

vals, err := redis.Values(c.conn.Do("GETJOB", args...))

And the err returned here for a timeout is redis.ErrNil.

If you don't want to keep redigo, you can also define an error type such as disque.ErrNil like redigo does. And then return it if there is a timeout.

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.

3 participants