fix(cluster) do not inadvertently log error messages

[thibaultcha]

The post_local API of the worker-events library can return false, nil when polling is skipped because we are posting from an event
handler, effectively nullifying the purpose of the first return value of
post_local.

My suggestion to lua-resty-worker-events would be to replace the
following line in post_local:

return _M.poll()

with a wrapper:

local ok, err = _M.poll()
if err then
    return nil, err
end

return true -- the event *was* posted

In the meantime, this commit changes the return values check to rely on
err ~= nil, thus avoiding such inadvertent error messages to show up.

Fix #2872

[Tieske]

I think for code readability it would be better to replace

if not ok then

with an explicit nil check:

if ok == nil then   -- `false` means it is successfully posted, but not yet dealt with.

also because you do not have to rely on the second return value.

[thibaultcha]

I think the level of readability is about the same one way or the other. It is a common paradigm in our codebase to check for errors with the second return value already (unfortunately). I think that ideally, this should be updated in lua-resty-worker-events, because we are not calling poll() here, but post_event. As a user of this API, I wish that the return value tells me whether or not the event was posted, and I do not wish to have it superseded by internal-specifics.

[Tieske]

btw, when implementing this:

local ok, err = _M.poll()
if err then
    return nil, err
end

return true -- the event *was* posted

You would loose the distinction between posted+handled and posted+unhandled. This is an important distinction.

(this discussion should probably be done in the worker-events repo)

[thibaultcha]

Yes, I know. I would disagree that this distinction is important to make though, as highlighted here:

As a user of this API, I wish that the return value tells me whether or not the event was posted, and I do not wish to have it superseded by internal-specifics.