Helper syntax for handling errors?

[ashtuchkin]

I really like how the await/defer structure helps writing async code. Lets go even further!

Currently, async error handling in node.js is a pain.

# Sample Express code with basic error handling
app.get '/', (req, res, next) ->
    db.readUsersList (err, users) ->
        if err? then return next(err)
        db.readArticles (err, articles) ->
            if err? then return next(err)
            res.render 'index', {users, articles}

# Sample Async code with basic error handling
readAndProcessFile = (filename, callback) ->
    fs.readFile filename, 'utf8', (err, text) ->
        if err? then return callback(err)  # Docs suggest to throw err, but its meaningless in async code. 
        process text, (err, result) ->
            if err? then return callback(err)
            callback null, result

As you can see, there are lots of if err? then return callback(err) everywhere, that's not good. Note that this is a very frequent pattern in asynchronous code.

I usually write a helper function to make the code a lot cleaner, something along the lines of:

# Error-handling helper. Use like this:
# asyncFunction params, errTo next, (results) ->
errTo = (next, callback) ->
    (err, params...) ->
        if err? return next(err)
        callback params...

# Sample Express code
app.get '/', (req, res, next) ->
    db.readUsersList errTo next, (users) ->  # Note no 'err' parameter.
        db.readArticles errTo next, (articles) ->
            res.render 'index', {users, articles}

So what can we do in IcedCoffeeScript? Lets see what can be done right now.

# Current way of error handling: same as in vanilla coffeescript, although a little cleaner because of lack of indentation.
app.get '/', (req, res, next) ->
    await db.readUsersList defer(err, users)
    if err? then return next(err)

    await db.readArticles defer(err, articles)
    if err? then return next(err)

    res.render 'index', {users, articles}

# Harder case: parallel fetching.
# I even dont know how to handle individual errors here.
app.get '/', (req, res, next) ->
    await 
        db.readUsersList defer(err, users)
        db.readArticles defer(err, articles)

    if err? then return next(err) # Not effective: waits for all deferreds to be fulfilled, also one error might overwrite the other.

    res.render 'index', {users, articles}

What's really lacking here is the semantics of try/catch clause, but with asynchronous twist and on a function level only (dont need to go up the stack). This is clearly hard to do in vanilla CoffeeScript, but I think might be doable in Iced.

Requirements:

• The syntax should be easy to understand. Use familiar constructs.
• Opt-in behavior. Should be easy to mix with the usual way of error handling.
• Should help programmer in dealing with serial and parallel flows. I.e. stop iteration when error is first encountered.
• Should look nice both in one-line form and block form of 'await'.

# Variant 1. Similar to my errTo helper
readAndProcess = (filename, cb) ->
    await fs.readFile filename, 'utf8', defer(~>cb, text)  # We can use any symbol instead of ~>. 
    await
        asyncOperation1 text, defer(~>cb, result1)
        asyncOperation2 text, defer(~>cb, result2)
    cb null, result1, result2

# Variant 2. Try/catch inspired
readAndProcess = (filename, cb) ->
    await fs.readFile filename, 'utf8', defer text catch cb

    await
        asyncOperation1 text, defer(result1)
        asyncOperation2 text, defer(result2)
    catch cb

    # Also, we can provide an error handler in-place:
    await
        asyncOperation1 text, defer(result1)
    catch (err) ->
        console.log err
        # Todo: do we allow return to the normal workflow?

    cb null, result1, result2

# Variant 3. Auto_cb automatically does this for the whole function
readAndProcess = (filename, auto_cb) ->
    await fs.readFile filename, 'utf8', defer(text) 
    await
        asyncOperation1 text, defer(result1)
        asyncOperation2 text, defer(result2)
    return {result1, result2}

What do you think? Is it doable for more generic case (ifs, whiles, etc.)?

[andrusha]

:+1: we need something like this, probably in the same fashion as Promises work

[vjpr]

+1

[maxtaco]

To answer your above question, you can handle errors from parallel calls as so:

app.get '/', (req, res, next) ->
    await 
        db.readUsersList defer e1, users
        db.readArticles defer e2, articles

    return next e if (e = e1 or e2)?
    res.render 'index', {users, articles}

For n errors, similarly, you can write a little (and generic) helper class. Or, you can use the "connectors" included in icedlib and demoed in this test case. The idea of connectors is to take a defer-generated callback, and to derive from it another callback, which can implement something like "accumulate all errors into a single error or null of there were no errors."

In general, I favor a library approach to this type of problem, rather than a language approach.

[ashtuchkin]

Arguably this is exactly the same type of problem that IcedCoffeeScript is trying to solve, compared to CoffeeScript ;) Unfortunately Jeremy prefers library approach too.

On the other hand, one of the most popular alternatives to using IcedCoffeeScript, async.js, gives errors first class handling, which I come to appreciate because most of my code includes ability to return error (not sure about yours).

No, seriously, how can you live without a helper for errors? Its so easy to forget writing another return next err if err?, and its so cluttering the logic!

[maxtaco]

If writing control flow as library calls (i.e. async.js) is your style, then certainly there's some library-like solution for handling errors in IcedCoffeeScript that suits your needs, and is achievable without a huge API (like async.js has).

As for me, my approach is usually of this form.

[maxtaco]

And BTW, I think Variant #1 (errTo above) is doable as a clean and simple library, you just should be careful about double-calling cb in the case of multiple errors in the parallel case.

[maxtaco]

Writing of ErrorShortCircuiter is left to the reader, but you can look at the Rendezvous class for inspiration..

# Variant 1. Similar to my errTo helper
readAndProcess = (filename, cb) ->
    esc = new ErrorShortCircuiter cb
    await fs.readFile filename, 'utf8', esc.defer res
    await
        asyncOperation1 text, esc.defer result1
        asyncOperation2 text, esc.defer result2
    cb null, result1, result2

[ashtuchkin]

That's an interesting approach, thank you!

A couple of things I'm worrying about:

• I believe the result1 and result2 will not be bound to variables.
• Will there be any memory leaks if the defer function is not called at all?

[maxtaco]

You'd plug into the standard defer-assignment-function machinery to get result1 and result2 bound to variables. Rendezvous currently does this already.

Good Q about the memleak. Honestly, I don't know. I thought V8 did a variation of mark-and-sweep (rather than reference-counting) so it should eventually come out with the answer.

I'm working on a little gist, give me a few minutes, I'll show you what I mean...

[maxtaco]

Here's the idea, I haven't tried it since it would require a small patch to IcedCoffeeScript.

https://gist.github.com/maxtaco/5609503

[ashtuchkin]

Yes, this would've nailed it. Although I'd prefer a function instead of a class:

deferErrTo = (cb) ->
    # Return the alternative defer function
    (arg) ->
        normal_cb = arg.context.defer arg
        (err, everything_else...) =>
            if err?
                if cb?
                    tmp = cb; cb = null; tmp(err)
            else 
                normal_cb everything_else...

Usage:

app.get '/', (req, res, next) ->
    deferErr = deferErrTo next
    await db.getUsers deferErr users
    res.render 'users', {users}

or (if you don't need parallel execution)

app.get '/', (req, res, next) ->
    await db.getUsers deferErrTo(next)(users)
    res.render 'users', {users}

[maxtaco]

Ok, great, I think we're in business. I pushed out a very small change in IcedCoffeeScript 1.6.2c (93e7f0d0ec8c847bac31b0e22863cf6bcc52f5ed) to make this possible. It's to expose __iced_deferrals as arg.context, which means you can use all of the regular defer mechanism in the success case, and your custom code in the failure case.

I like the idea of having failure-specific code in 3rd party libraries, since error-signalling conventions vary.

See the gist again for an updated version that works with a little test case.

Your code can work too, but I would just double-check that cb() is only called once in the case of two or more errors that happen in parallel.

[maxtaco]

Oh, and I take it back, your code won't work, because you need to give IcedCoffeeScript something of the form foo.defer x,y,z. Otherwise you won't get the compiler to rephase your foo.defer x,y,z as a series of assignments. In other words, defer is a language keyword that instructs the compile to output "call-by-reference"-type semantics (see iced --print or the web sandbox for more specifics).

[ashtuchkin]

Wow, I appreciate your efforts, thank you!

Though I don't really like the approach where I need to instantiate a class in every function :(

But I think I have one more solution, with no classes:

app.get '/', (req, res, next) ->
    await db.getUserById userId, errTo next, defer user

    # or, bound:
    noErr = errTo.bind(null, next)
    await db.getUserById userId, noErr defer user

This can work even for parallel errors if the function that is returned from defer call will contain a reference to corresponding Deferrals or Rendezvous object. Example implementation:

errTo = (errCallback, callback) ->
    (err, args...) ->
        if not err?
            return callback(args...)

        if not callback.__iced_context?.errorReturned    # Deferrals or Rendezvous object is exposed as __iced_context object.
            callback.__iced_context?.errorReturned = true
            errCallback(err)

Do you think you can implement this small change too? I would then update my library https://github.com/ashtuchkin/errTo to work with this case.

And, we'll still need to check about memory leaks.

[maxtaco]

Just so I understand, why can't this work?

errTo = (errCallback, callback) ->
    (err, args...) ->
        if not err?
            callback(args...)
        else if not errCallback?.__errToUsed
            errCallback.__errToUsed = true
            errCallback(err)

[ashtuchkin]

Interesting, I haven't thought about it) seems this can work too. If I wont find any unwanted side effects, I'll implement it this way (although, I believe exposing the context object on callback could still be useful for extensibility, much in the same way you did in last commits).

Sent from my iPhone

On 20.05.2013, at 7:35, Maxwell Krohn notifications@github.com wrote:

Just so I understand, why can't this work?

errTo = (errCallback, callback) -> (err, args...) -> if not err? callback(args...) else if not errCallback?.errToUsed errCallback.errToUsed = true errCallback(err) — Reply to this email directly or view it on GitHub.

[maxtaco]

...And one more proposal is the "curried" form, which is similar to your "bound" form above....

errTo = (errCallback) -> (callback) ->
    (err, args...) ->
        if not err?
            callback(args...)
        else if not errCallback?.__errToUsed
            errCallback.__errToUsed = true
            errCallback(err)

app.get '/', (req, res, next) ->
    noErr = errTo next
    await 
       db.getUserById userId, noErr defer user
       db.getKittenByPicId picId, noErr defer kitten
    await db.storeBoth user, kitten, noErr defer()
    next()

[maxtaco]

Good luck, and thanks for your feedback.

Let me think a bit more about that context object addition you wanted before I go ahead with it...

[ashtuchkin]

Okay, I've published a new version of errTo module with IcedCoffeeScript support. The syntax is as following:

errTo = require 'errto'

app.get '/', (req, res, next) ->
    await db.getUserById req.userId, errTo next, defer user  # Notice, errTo is outside defer.
    res.render 'index', {user}

app.get '/posts/:postId', (req, res, next) ->
    noErr = errTo.bind(null, next) # errTo can be bound in the beginning, using standard JS construct.
    await db.getPostById req.param('postId'), noErr defer post

    await
        # Notice these 2 requests will be run in parallel and if at least one of them fails (returns error)
        # then the whole block fails. But if both fail, then only the first error is kept.
        db.getPostComments post._id, errTo next, defer comments
        db.getPostText post._id, errTo next, defer text

    render 'post', {comments, text}

I think you can close the issue, thank you!

[vjpr]

@maxtaco Would it be possible to allow access to the arguments object from the original scope of the await call.

The idea is to allow a default error callback being set to the last argument from the last non-Iced function call.

This would make @ashtuchkin's errTo lib a bit slimmer preventing the need for errTo next, fn and noErr = errTo.bind(null, next).

At the moment the only way is to traverse up the call stack and look for the first non-Iced-generated function, by repeatedly calling fn.caller.arguments.

Since Iced is already saving a bit of information for writing nice stack traces, I don't see this as being that difficult to implement. The first call to await would check/set a variable to store the arguments.

I think this makes sense for Iced to implement because something like this would be possible with normal callbacks, by just calling _.last(arguments)(). Another approach would be to store the arguments object for the preceding callback. This would make error handling very easy and configurable with helper methods for e.g:

failOnError = (onErr, onSuccess) -> if e then (e) -> _.last($icedArguments)(e); ...
proceedOnError = (onErr, onSuccess) -> (e, r) -> onSuccess(r)
await foo failOnError defer foo
await bar proceedOnError defer bar

More here: https://github.com/ashtuchkin/errTo/issues/3