API change: defer everything to the callback

[txdv]

Makes it easier for use for the user to use the library, because the user now has only to do error checking in one place instead of two.

Discussion: #829

[saghul]

I don't like the idea of deferring bogus parameter errors to the callback.

Ben's original proposal (which I like too) contained that last bit about bad arguments returning EINVAL on debug builds and triggering an assert on release builds. I find that very useful. Moreover, What do you plan to do with functions such as uv_timer_start, which cannot get a NULL callback? Return EINVAL, of course.

You said you wanted write an RFC about error handling, I'd rather see that first before jumping into writing any code.

[huxingyi]

+1 for no return value. (Some apis have no callback, then could add a callback)
It will ease the user level coding and reduce bugs which was caused by forgetting to process the return value.

[txdv]

The entire discussion is here #829. We should talk about this in bnoordhuis issue and use this pull request to comment on the code strictly. So I'll post your questions there and then add my answers.

[piscisaureus]

I agree that errors should be deferred to the callback as much as possible.

In principle I agree with @bnoordhuis's suggestion that parameter errors should either cause crashes or be reported synchronously. In node, for example, we'll throw if the user doesn't specify a callback, or if the user calls fs.open without specifying a filename. It's simply impossible to interpret what the user might have wanted, and if there's no callback node can't even defer the callback.

In libuv this type of error typically causes a crash. That's because we can't verify that the user specified a valid loop pointer to uv_tcp_init(), or whether a callback pointer is valid or not. In these situations crashing is the only thing we can do.

However @txdv is right about the fact that if parameter verification is taken a step further, it becomes an extremely fine line. Some examples:

• attempt to open a file but the filename isn't well-formed
• attempt to write to a shutdown()-ed socket
• attempt to write to a closing socket
• attempt to uv_connect() a stream handle that's identified to be a server (and not a connection)
• attempt to write an empty buffer

In my opinion, all of these errors would ideally deferred.

This is easy enough to define for functions like uv_write() that use the "request-callback" pattern.
But libuv currently doesn't always do that. Functions like uv_read_start(stream, cb) and uv_listen(stream, cb) put the handle in a certain "state" and then fire callbacks repeatedly.
To define what should happen here is a bit more difficult - e.g. should uv_read_start() on a server handle cause a one-time invocation of the read callback, or do we report the failure synchronously?

Note I think that these APIs will be replaced by "request-callback" like APIs in the future.

[txdv]

Yours pointed out should result in EINVAL too? And you think they should be deferred?

You forgot ENOMEM. everyone forgets ENOMEM, but that one is easy to defer.

As for for uv_listen and uv_read_start, YES, they will become request callback APIs.

[txdv]

no resolution

[saghul]

I've kept this one on the back of my head, and the more I thought of it, the messier it becomes. IMHO, we just cannot defer everything to the callback because the close callback has to be last one to be called, so if the user calls uv_write after that it needs to fail synchronously. I couldn't think of a way around this.

Also, we have functions like uv_try_write which don't take a callback but work similarly to uv_write, so making one report some errors inline and the other defer them could be seen as an inconsistency.

So, my current state of mind is that I find trouble wording how the behavior should be, without having tons of exceptions.

I think, however, that a refined version of what we have could work better and could potentially be easier to explain: "if the function returns 0 it's guaranteed that the callback will be called, if it returned < 0 then the callback won't be called.". Then, we'd only defer errors to the callback under special circumstances:

• platform differences, as in the bind / listen EADDRINUSE (IIRC)
• in case we try to perform a blocking operation (as in uv_write)
• ?

Even if we won't use the code in this PR, I'll open an issue about this on libuv/libuv in order to further investigate this.

[txdv]

pipe_connect is deffering EINVAL has currently inconsistent API

[saghul]

We are currently inconsistent in a few places, yes, that's why we need to
find the best way forward.
On Nov 18, 2014 4:46 PM, "Andrius Bentkus" [email protected] wrote:

pipe_connect is deffering EINVAL has currently inconsistent API

—
Reply to this email directly or view it on GitHub
#1538 (comment).